Skip to content
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

Jump to bottom

fix(dialog): fade-out animation when lazy loaded on popover overlays #4937

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
+86 −19
Open

fix(dialog): fade-out animation when lazy loaded on popover overlays #4937

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
80eaeb0
fix(dialog): fade-out animation when lazy loaded on popover overlays
mizgaionutalexandru Nov 15, 2024
da8e185
fix(dialog): missing fade-out animations close dispatch
mizgaionutalexandru Nov 15, 2024
70414cf
chore: update storybook paragraph
mizgaionutalexandru Nov 15, 2024
bebd451
fix: underlay guard
mizgaionutalexandru Nov 18, 2024
c5a69aa
Merge branch 'main' into imizga/trigger-directive-transition
TarunAdobe Nov 18, 2024
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
29 changes: 26 additions & 3 deletions packages/dialog/src/DialogBase.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,18 +155,41 @@ export class DialogBase extends FocusVisiblePolyfillMixin(SpectrumElement) {
this.handleTransitionEvent(event);
}

private get hasTransitionDuration(): boolean {
const modal = this.shadowRoot.querySelector('.modal') as HTMLElement;

const modalTransitionDurations =
window.getComputedStyle(modal).transitionDuration;
for (const duration of modalTransitionDurations.split(','))
if (parseFloat(duration) > 0) return true;

const underlay = this.shadowRoot.querySelector(
'sp-underlay'
) as HTMLElement;

if (underlay) {
const underlayTransitionDurations =
window.getComputedStyle(underlay).transitionDuration;
for (const duration of underlayTransitionDurations.split(','))
if (parseFloat(duration) > 0) return true;
}

return false;
}

protected override update(changes: PropertyValues<this>): void {
if (changes.has('open') && changes.get('open') !== undefined) {
const hasTransitionDuration = this.hasTransitionDuration;
this.animating = true;
this.transitionPromise = new Promise((res) => {
this.resolveTransitionPromise = () => {
this.animating = false;
if (!this.open && hasTransitionDuration)
this.dispatchClosed();
res();
};
});
if (!this.open) {
this.dispatchClosed();
}
if (!this.open && !hasTransitionDuration) this.dispatchClosed();
}
super.update(changes);
}
Expand Down
76 changes: 60 additions & 16 deletions packages/dialog/stories/dialog-base.stories.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,33 +10,31 @@ governing permissions and limitations under the License.
*/

import { html, TemplateResult } from '@spectrum-web-components/base';
import '@spectrum-web-components/button/sp-button.js';
import '@spectrum-web-components/checkbox/sp-checkbox.js';
import '@spectrum-web-components/dialog/sp-dialog-base.js';
import '@spectrum-web-components/dialog/sp-dialog.js';
import '@spectrum-web-components/button/sp-button.js';
import { trigger } from '@spectrum-web-components/overlay';
import '@spectrum-web-components/overlay/sp-overlay.js';
import '@spectrum-web-components/checkbox/sp-checkbox.js';
import { alertDestructive } from './dialog.stories.js';
import { portrait } from './images.js';
import { disabledButtonDecorator } from './index.js';

const withOverlayDecorator = (story: () => TemplateResult): TemplateResult => {
Copy link
Contributor

Choose a reason for hiding this comment

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

This decorator needs to be initialized. This will create problems in VRT tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe I'm missing something here but I'm not sure what you mean by initializing it. I extracted the function as it was previously defined for all the stories from this file, to be able to add it only where needed.
Is this related to the VRTs saying that this PR removes the disabledButton VRT (link)?

Copy link
Contributor

Choose a reason for hiding this comment

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

I see you are trying to add withOverlayDecorator decorator with every story! Instead of adding this as story.decorator = [withOverlayDecorator] you can initialise this as

export default { 
....
decorators:[withOverlayDecorator()]
}

But this is not something that will trigger the issue! In the mean time I see some flaky tests have emerged and a redundant hash is there. Can you please updated the hash and surface up the new baseline for slotted and re-trigger the runs.

return html`
<sp-button variant="primary" id="trigger">Toggle Dialog</sp-button>
<sp-overlay type="modal" trigger="trigger@click" open>
${story()}
</sp-overlay>
`;
};

export default {
title: 'Dialog Base',
component: 'sp-dialog-base',
decorators: [
(story: () => TemplateResult): TemplateResult => {
return html`
<sp-button variant="primary" id="trigger">
Toggle Dialog
</sp-button>
<sp-overlay type="modal" trigger="trigger@click" open>
${story()}
</sp-overlay>
`;
},
],
};

export const Slotted = (): TemplateResult => html`
export const slotted = (): TemplateResult => html`
<sp-dialog-base
underlay
@click=${(event: Event) => {
Expand All @@ -50,6 +48,7 @@ export const Slotted = (): TemplateResult => html`
${alertDestructive()}
</sp-dialog-base>
`;
slotted.decorators = [withOverlayDecorator];

export const disabledButton = (): TemplateResult => {
return html`
Expand Down Expand Up @@ -116,7 +115,7 @@ export const disabledButton = (): TemplateResult => {
`;
};

disabledButton.decorators = [disabledButtonDecorator];
disabledButton.decorators = [withOverlayDecorator, disabledButtonDecorator];

export const notAgain = (): TemplateResult => html`
<sp-dialog-base
Expand All @@ -143,6 +142,7 @@ export const notAgain = (): TemplateResult => html`
</sp-dialog>
</sp-dialog-base>
`;
notAgain.decorators = [withOverlayDecorator];

export const moreCustom = (): TemplateResult => html`
<sp-dialog-base
Expand Down Expand Up @@ -189,6 +189,7 @@ export const moreCustom = (): TemplateResult => html`
</div>
</sp-dialog-base>
`;
moreCustom.decorators = [withOverlayDecorator];

export const fullyCustom = (): TemplateResult => html`
<sp-dialog-base
Expand Down Expand Up @@ -222,3 +223,46 @@ export const fullyCustom = (): TemplateResult => html`
</div>
</sp-dialog-base>
`;
fullyCustom.decorators = [withOverlayDecorator];

export const lazyLoaded = (): TemplateResult => {
const template = (): TemplateResult => html`
<sp-dialog-base
underlay
@click=${(event: Event) => {
if ((event.target as HTMLElement).localName === 'sp-button') {
(event.target as HTMLElement).dispatchEvent(
new Event('close', { bubbles: true, composed: true })
);
}
}}
>
<sp-dialog size="m">
<h2 slot="heading">This is a heading</h2>
<p>
The click on the "OK" button should close the overlay with
the correct animation (duration).
</p>
<sp-button variant="secondary" treatment="fill" slot="button">
Ok
</sp-button>
</sp-dialog>
</sp-dialog-base>
`;

return html`
<sp-button
variant="primary"
${trigger(template, {
open: false,
triggerInteraction: 'click',
})}
>
Open dialog
</sp-button>
`;
};

lazyLoaded.swc_vrt = {
skip: true,
};
Loading