Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
spliffone wants to merge 1 commit into
base: main
Choose a base branch
from
Open

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

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions projects/element-ng/modal/si-modal.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
viewChild,
DOCUMENT
} from '@angular/core';
import { areAnimationsDisabled } from '@siemens/element-ng/common';

import { ModalRef } from './modalref';

Expand All @@ -26,7 +27,8 @@ import { ModalRef } from './modalref';
changeDetection: ChangeDetectionStrategy.OnPush
})
export class SiModalComponent implements OnInit, AfterViewInit, OnDestroy {
protected modalRef = inject(ModalRef<unknown, any>);
protected readonly modalRef = inject(ModalRef<unknown, any>);
protected readonly isAnimated = !areAnimationsDisabled() && this.modalRef.data.animated !== false;

protected dialogClass = this.modalRef.dialogClass ?? '';
protected titleId = this.modalRef.data?.ariaLabelledBy ?? '';
Expand Down Expand Up @@ -135,6 +137,6 @@ export class SiModalComponent implements OnInit, AfterViewInit, OnDestroy {
}

private animationTime(millis: number): number {
return this.modalRef?.data.animated !== false ? millis : 0;
return this.isAnimated ? millis : 0;
Copy link
Member

@spike-rabbit spike-rabbit Feb 9, 2026

Choose a reason for hiding this comment

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

I think you need still need to respect data.animated.

@dr-itz do you think we can deprecate the animated property with this change? Was there a specific use case why this was introduced?

Copy link
Member Author

@spliffone spliffone Feb 9, 2026

Choose a reason for hiding this comment

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

It is respected in line 31

Copy link
Contributor

@dr-itz dr-itz Feb 9, 2026

Choose a reason for hiding this comment

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

Oh I don't really remember, but I guess this was for ngx-bootstrap compatibility. So yeah, having this here feels kinda pointless

}
}
Loading