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.

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

Dialog tweaks #1421

Merged
merged 5 commits into from
Feb 1, 2019
Merged

Dialog tweaks #1421

merged 5 commits into from
Feb 1, 2019

Conversation

walaura
Copy link
Contributor

@walaura walaura commented Jan 31, 2019

Why are you doing this?

Implement some of @SiAdcock's suggestions (the easy high api layer impact ones) in the dialog component #1416

Changes

  • Don't handle browser focus if it supports <dialog>
  • Remove modal prop
  • Add new styled prop that adds a modal-like appearance (this was previously tangled together with modal-ness. Most appearances will be custom but this makes it more satisfying to implement. (instant dialogs!)
  • New default is for dialogs to not dismiss themselves when clicking out – to make implementers think about adding a close button (needed for keyboard users). the prop for this is now called blocking for simplicity
  • Got carried away writing stories for this!

Screenshots

screenshot 2019-01-31 at 10 17 14 am

screenshot 2019-01-31 at 10 17 09 am

@walaura walaura requested a review from SiAdcock January 31, 2019 10:19
@SiAdcock
Copy link
Contributor

New default is for dialogs to not dismiss themselves when clicking out – to make implementers think about adding a close button (needed for keyboard users)

Does the functionality exist to dismiss using the Escape key? I think keyboard users would expect it. A close button is better though!

Copy link
Contributor

@SiAdcock SiAdcock left a comment

Choose a reason for hiding this comment

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

Great changes! 👍

@walaura
Copy link
Contributor Author

walaura commented Jan 31, 2019

New default is for dialogs to not dismiss themselves when clicking out – to make implementers think about adding a close button (needed for keyboard users)

Does the functionality exist to dismiss using the Escape key? I think keyboard users would expect it. A close button is better though!

ah! good point. it's in for <dialog> not in the polyfill. Will add

@walaura walaura merged commit 4f806ec into master Feb 1, 2019
@walaura walaura deleted the lg-dialogs-rev-2 branch February 1, 2019 14:52
@prout-bot
Copy link

Seen on PROD (merged by @walaura 8 minutes and 26 seconds ago)

Sentry Release: support-client-side, support

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants