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

fix: pointer-events when content has CSS animations #173

Merged
merged 3 commits into from
Mar 5, 2020
Merged

fix: pointer-events when content has CSS animations #173

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
043693f
fix: pointer-events when content has CSS animations
Haprog Mar 3, 2020
51de212
fix: tests and more synchronous open/close behaviour
Haprog Mar 4, 2020
e17e42b
test: fix tests for IE11
Haprog Mar 4, 2020
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
50 changes: 26 additions & 24 deletions src/vaadin-overlay.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -578,7 +578,10 @@

_enqueueAnimation(type, callback) {
const handler = `__${type}Handler`;
const listener = () => {
const listener = event => {
if (event && event.target !== this) {
return;
}
Comment on lines +582 to +584
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This part is most of the fix. This makes overlay ignore any unrelated animationend events that might be triggered by anything inside the dialog (e.g. if using <vaadin-text-area> inside the dialog which triggers a CSS animation on init).

callback();
this.removeEventListener('animationend', listener);
delete this[handler];
Expand All @@ -599,15 +602,15 @@
this._flushAnimation('closing');
}
this._attachOverlay();
if (!this.modeless) {
this._enterModalState();
}
this.setAttribute('opening', '');

const finishOpening = () => {
this.removeAttribute('opening');
document.addEventListener('iron-overlay-canceled', this._boundIronOverlayCanceledListener);

if (!this.modeless) {
this._enterModalState();
}
this.removeAttribute('opening');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing the opening attribute was moved to be the very last thing in finishOpening() so we can use it (e.g. in MutationObserver) to detect when the opening has been completely finished (including possible animations).

Other stuff (the modal state update) was moved out of this method to be run earlier/synchronously as it doesn't need to be tied to the completion of the possible animation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Similar changes to finishClosing() below.

Copy link

Choose a reason for hiding this comment

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

In my opinion, this refactor is important, in the future, we can use the opening/closing to check something because we know the status state: opening/ opened or closing/closed.

This fix addressed the bug and refactored code better. Thank you for the great fix.

};

if (this._shouldAnimate()) {
Expand All @@ -629,30 +632,29 @@
this._flushAnimation('opening');
}
if (this._placeholder) {
this.setAttribute('closing', '');
this._exitModalState();

const finishClosing = () => {
this.shadowRoot.querySelector('[part="overlay"]').style.removeProperty('pointer-events');
if (this.restoreFocusOnClose && this.__restoreFocusNode) {
// If the activeElement is `<body>` or inside the overlay,
// we are allowed to restore the focus. In all the other
// cases focus might have been moved elsewhere by another
// component or by the user interaction (e.g. click on a
// button outside the overlay).
const activeElement = this._getActiveElement();

this._exitModalState();
if (activeElement === document.body || this._deepContains(activeElement)) {
this.__restoreFocusNode.focus();
}
this.__restoreFocusNode = null;
}

this.setAttribute('closing', '');

const finishClosing = () => {
document.removeEventListener('iron-overlay-canceled', this._boundIronOverlayCanceledListener);
this._detachOverlay();
this.shadowRoot.querySelector('[part="overlay"]').style.removeProperty('pointer-events');
this.removeAttribute('closing');

if (this.restoreFocusOnClose && this.__restoreFocusNode) {
// If the activeElement is `<body>` or inside the overlay,
// we are allowed to restore the focus. In all the other
// cases focus might have been moved elsewhere by another
// component or by the user interaction (e.g. click on a
// button outside the overlay).
const activeElement = this._getActiveElement();

if (activeElement === document.body || this._deepContains(activeElement)) {
this.__restoreFocusNode.focus();
}
this.__restoreFocusNode = null;
}
};

if (this._shouldAnimate()) {
Expand Down Expand Up @@ -715,7 +717,7 @@

// Disable pointer events in other attached overlays
OverlayElement.__attachedInstances.forEach(el => {
if (el !== this && !el.hasAttribute('opening') && !el.hasAttribute('closing')) {
if (el !== this) {
el.shadowRoot.querySelector('[part="overlay"]').style.pointerEvents = 'none';
}
Comment on lines 718 to 722
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was simplified a bit as this works like this now with the updated logic above. Now pointer-events: none might also be applied to closing or opening overlays here but that should be fine. Closing dialogs will remove it themselves after the closing animation finishes (in finishClosing()).

});
Expand Down
Loading