-
Notifications
You must be signed in to change notification settings - Fork 12
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.
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
feat(dialog): improve api with modal attribute (VIV-1657) #1853
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1853 +/- ##
===========================================
Coverage 100.00% 100.00%
===========================================
Files 123 342 +219
Lines 1562 6211 +4649
Branches 108 781 +673
===========================================
+ Hits 1562 6211 +4649
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@@ -64,18 +55,12 @@ function renderDismissButton(buttonTag: string) { | |||
} | |||
|
|||
function handleEscapeKey(dialog: Dialog, event: Event) { | |||
if ((event as KeyboardEvent).key === 'Escape' && dialog.modal) { | |||
if ((event as KeyboardEvent).key === 'Escape' && dialog._openedAsModal) { | |||
dialog.open = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we stop the propagation of the event here to stop it affecting other components that are listening for an escape key event?
For example, if the modal markup was inside an open side drawer component for some reason, pressing escape would close the side drawer as well as the modal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@TaylorJ76 Yes, but that would not really be related to the modal
feature.
Also, we need to do it for all other components.
I created a new ticket for it: https://jira.vonage.com/browse/VIV-1950
modal
attributeopen
eventv-model
for open