Skip to content

refactor(modal): allow to disable dialog animation#1478

Merged
spike-rabbit merged 1 commit intomainfrom
refactor/modal-allow-disabling-animations
Feb 17, 2026
Merged

refactor(modal): allow to disable dialog animation#1478
spike-rabbit merged 1 commit intomainfrom
refactor/modal-allow-disabling-animations

Conversation

@spliffone
Copy link
Copy Markdown
Member

@spliffone spliffone commented Feb 9, 2026

Describe in detail what your merge request does and why. Add relevant
screenshots and reference related issues via Closes #XY or Related to #XY.


@spliffone spliffone requested a review from a team as a code owner February 9, 2026 14:12
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a global flag to disable modal animations, which is a good improvement for accessibility and performance on low-end devices. The implementation correctly uses a helper to check for the global setting.

However, the current implementation is incomplete. The global animation setting is only applied to the animation timings controlled by animationTime(), but not to the CSS-based fade animations or the backdrop animation logic.

I've added a couple of suggestions to refactor the logic into a reusable isAnimated property. Adopting this suggestion and using the new property consistently in showBackdrop() and the component's template (si-modal.component.html) will fix the issue and make the code cleaner.

Comment thread projects/element-ng/modal/si-modal.component.ts Outdated
Comment thread projects/element-ng/modal/si-modal.component.ts Outdated
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Feb 9, 2026

@spliffone spliffone force-pushed the refactor/modal-allow-disabling-animations branch from 466bd67 to e3fa53f Compare February 9, 2026 14:40
Comment thread projects/element-ng/modal/si-modal.component.ts
@spliffone spliffone force-pushed the refactor/modal-allow-disabling-animations branch from e3fa53f to d3a23e2 Compare February 17, 2026 09:17
@spliffone spliffone requested a review from a team as a code owner February 17, 2026 09:17
@spike-rabbit spike-rabbit enabled auto-merge (rebase) February 17, 2026 09:17
@spliffone spliffone force-pushed the refactor/modal-allow-disabling-animations branch from d3a23e2 to 7182e68 Compare February 17, 2026 09:40
@spike-rabbit spike-rabbit enabled auto-merge (rebase) February 17, 2026 09:51
@github-actions
Copy link
Copy Markdown

Code Coverage

@spike-rabbit spike-rabbit merged commit 64f2ca9 into main Feb 17, 2026
10 checks passed
@spike-rabbit spike-rabbit deleted the refactor/modal-allow-disabling-animations branch February 17, 2026 09:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants