-
Notifications
You must be signed in to change notification settings - Fork 45
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
Add Modal
component
#631
Add Modal
component
#631
Conversation
π¦ Changeset detectedLatest commit: 9129e59 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The latest updates on your projects. Learn more about Vercel for Git βοΈ
|
9eb657e
to
70281b4
Compare
0a9387d
to
5e19579
Compare
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.
@alex-ju I did a first pass on the code and added a few comments. more than happy to discuss them in a call/chat
173f551
to
9a7a6bf
Compare
9a7a6bf
to
e2694e4
Compare
e2694e4
to
850af9e
Compare
850af9e
to
34e1375
Compare
This should prevent the scroll from being displayed on the `hds-modal__body` element if the content does not overflow.
Updated to test that any additional attribute passed is rendered β not just the `class`
We do this so that viewers don't accidentally remove an example from the showcase while checking out the examples.
If the callback function returns `false` we keep the modal dialog open
To facilitate the implementation of dismiss actions
Co-Authored-By: Melanie Sumner <[email protected]>
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.
Added a couple of super minor comments.
It's good to go! π’
packages/components/tests/dummy/app/templates/components/modal.hbs
Outdated
Show resolved
Hide resolved
Switch from checking the callback function return to checking a `isDismissDisabled` property before dismissing a modal dialog
e6d88d5
to
9129e59
Compare
π Summary
Add
modalDialog
Modal
componentπ οΈ Detailed description
Raising this early for feedback on the direction of travelIn this modal dialog implementation, we try to leverage the
<dialog>
element as much as possible and kept the codebase to a minimum.π Preview
Hds::Modal
componentFunctionality
id
and setaria-labelledby
to the title)role="dialog"
aria-modal="true"
) [dialog]ESC
key the modal dialog is closed [dialog]ember-focus-trap
orember-click-outside
)ember-focus-trap
)Variations
size
variation (small/medium/large)typecolor
variation (neutral/warning/critical)Documentation
πΈ Screenshots
The current implementation of
![dialog-polyfill on Safari14](https://user-images.githubusercontent.com/788096/198996666-7c48696e-2aa8-4040-bd0e-0d0665b0c225.png)
Hds::Modal
component in Safari 14 (tested via BrowserStack) which does not have native support for<dialog>
. In this instances, where there is no native support, we use Google Chrome's polyfill.π External links
Jira issue
π How to review
π Review commit-by-commit (for the 'evolution' of the component and a more digestible review)
π Review by files changed (for an overall review)
Reviewer's checklist:
π¬ Please consider using conventional comments when reviewing this PR.