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

Modal, Flyout - Remove close event listener on destroy #1887

Merged
merged 3 commits into from
Jan 16, 2024
Merged

Modal, Flyout - Remove close event listener on destroy #1887

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
229c0bf
Remove event listener on destroy for Modal
alex-ju Jan 15, 2024
a3cdf24
Remove event listener on destroy for Flyout
alex-ju Jan 15, 2024
2d9f359
Add changeset
alex-ju Jan 15, 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
6 changes: 6 additions & 0 deletions .changeset/fast-goats-approve.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@hashicorp/design-system-components": patch
---

Modal - Removed `close` event listener on destroy
Flyout - Removed `close` event listener on destroy
1 change: 1 addition & 0 deletions packages/components/addon/components/hds/flyout/index.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
...attributes
aria-labelledby={{this.id}}
{{did-insert this.didInsert}}
{{will-destroy this.willDestroyNode}}
{{focus-trap isActive=this.isOpen focusTrapOptions=(hash onDeactivate=this.onDismiss clickOutsideDeactivates=true)}}
>
{{yield (hash Header=(component "hds/flyout/header" id=this.id onDismiss=this.onDismiss))}}
Expand Down
27 changes: 20 additions & 7 deletions packages/components/addon/components/hds/flyout/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,14 @@ export default class HdsFlyoutIndexComponent extends Component {
return classes.join(' ');
}

@action registerOnCloseCallback() {
if (this.args.onClose && typeof this.args.onClose === 'function') {
this.args.onClose();
}

this.isOpen = false;
}

@action
didInsert(element) {
// Store references of `<dialog>` and `<body>` elements
Expand Down Expand Up @@ -94,20 +102,25 @@ export default class HdsFlyoutIndexComponent extends Component {
}

// Register "onClose" callback function to be called when a native 'close' event is dispatched
this.element.addEventListener('close', () => {
if (this.args.onClose && typeof this.args.onClose === 'function') {
this.args.onClose();
}

this.isOpen = false;
});
this.element.addEventListener('close', this.registerOnCloseCallback, true);

// If the flyout dialog is not already open
if (!this.element.open) {
this.open();
}
}

@action
willDestroyNode() {
if (this.element) {
this.element.removeEventListener(
'close',
this.registerOnCloseCallback,
true
);
}
}

@action
open() {
// Make flyout dialog visible using the native `showModal` method
Expand Down
1 change: 1 addition & 0 deletions packages/components/addon/components/hds/modal/index.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
...attributes
aria-labelledby={{this.id}}
{{did-insert this.didInsert}}
{{will-destroy this.willDestroyNode}}
{{focus-trap isActive=this.isOpen focusTrapOptions=(hash onDeactivate=this.onDismiss clickOutsideDeactivates=true)}}
>
{{yield (hash Header=(component "hds/modal/header" id=this.id onDismiss=this.onDismiss))}}
Expand Down
57 changes: 35 additions & 22 deletions packages/components/addon/components/hds/modal/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,29 @@ export default class HdsModalIndexComponent extends Component {
return classes.join(' ');
}

@action registerOnCloseCallback() {
if (
!this.isDismissDisabled &&
this.args.onClose &&
typeof this.args.onClose === 'function'
) {
this.args.onClose();
}

// If the dismissal of the modal is disabled, we keep the modal open/visible otherwise we mark it as closed
if (this.isDismissDisabled) {
// If, in a chain of events, the element is not attached to the DOM, the `showModal` would fail
// so we add this safeguard condition that checks for the `<dialog>` to have a parent
if (this.element.parentElement) {
// As there is no way to `preventDefault` on `close` events, we call the `showModal` function
// preserving the state of the modal dialog
this.element.showModal();
}
} else {
this.isOpen = false;
}
}

@action
didInsert(element) {
// Store references of `<dialog>` and `<body>` elements
Expand Down Expand Up @@ -120,35 +143,25 @@ export default class HdsModalIndexComponent extends Component {
}

// Register "onClose" callback function to be called when a native 'close' event is dispatched
this.element.addEventListener('close', () => {
if (
!this.isDismissDisabled &&
this.args.onClose &&
typeof this.args.onClose === 'function'
) {
this.args.onClose();
}

// If the dismissal of the modal is disabled, we keep the modal open/visible otherwise we mark it as closed
if (this.isDismissDisabled) {
// If, in a chain of events, the element is not attached to the DOM, the `showModal` would fail
// so we add this safeguard condition that checks for the `<dialog>` to have a parent
if (this.element.parentElement) {
// As there is no way to `preventDefault` on `close` events, we call the `showModal` function
// preserving the state of the modal dialog
this.element.showModal();
}
} else {
this.isOpen = false;
}
});
this.element.addEventListener('close', this.registerOnCloseCallback, true);

// If the modal dialog is not already open
if (!this.element.open) {
this.open();
}
}

@action
willDestroyNode() {
if (this.element) {
this.element.removeEventListener(
'close',
this.registerOnCloseCallback,
true
);
}
}

@action
open() {
// Make modal dialog visible using the native `showModal` method
Expand Down