-
Notifications
You must be signed in to change notification settings - Fork 9
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 dialog component #416
base: main
Are you sure you want to change the base?
Add dialog component #416
Conversation
9162709
to
f96180c
Compare
} | ||
|
||
.button-close { | ||
top: -2rem; |
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.
Suggest adding a comment to explain the relation with icons. Wording could be better but here’s a draft:
top: -2rem; | |
// Offset by the size of the icon | |
top: -2rem; |
} | ||
|
||
.dialog-overlay, | ||
.dialog-box { |
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.
We will need forced-colors styles (borders) so the dialog is easier to understand in WHCM.
Perhaps something to raise with a11y-dialog
as well if this is an issue with the base component from the library?
Checklist
Will be updated based on team's inputs on naming
Description
Accessible dialog (modal) window based on a11y-dialog library:
dialog.show()
fadeIn
animationdialogId
(for instantiation and trigger),dialogTrigger
(triggers dialog opening) andcontent
(dialog content)ariaLabelledBy
(accessible dialog name), 'overlayClose' (closes the dialog on overlay click and defaults to true), anddialogBoxClass
(css class for styling)container
class for consistency with the existing layout, no other styles are added for maximum flexibility. Styles could be applied viadialogBoxClass
or directly in the dialog content - as demonstrated in the Story. Potentially, min/max-height can be hard-coded like this:Note: for some reason Tailwind classes like
max-h-*
don't work (with the exception ofmax-h-xl
), which affects mobile version look.Instructions to test
Check the storybook Dialog story, try to add 2 dialogs directly to the
index.html
to test multiple dialog instances on 1 pageTested in the following environments/browsers:
Operating System
Browser