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

fix(ui-kit): dialog closing and a11y #346

Merged
merged 3 commits into from
Jul 30, 2023
Merged

fix(ui-kit): dialog closing and a11y #346

merged 3 commits into from
Jul 30, 2023

Conversation

harlan-zw
Copy link
Contributor

@harlan-zw harlan-zw commented Jul 27, 2023

Seems like the introduction of the focus trap on NDialog stopped the dialog closing when clicking on the backdrop.

Peek.2023-07-27.14-46.webm

(I'm clicking on the backdrop in the video)

To fix it we implement the same logic that the DrawerRight component uses. We also fix some i18n issues and have better styling for dark mode.

@antfu
Copy link
Member

antfu commented Jul 27, 2023

/cc @arashsheyda I guess it's related to your PR?

@arashsheyda
Copy link
Member

arashsheyda commented Jul 27, 2023

This PR solving another issue I think, but I guess we can fix that issue in here too.

the problem in other PR is described like:

  • while a drawer has auto-close and it's open, and there is a Dialog and user clicks on the Dialog, it will close the Dialog and the Drawer becuase it has auto-close.
example video
bandicam.2023-07-27.20-42-25-281.mp4

I though a solution can be by adding a class that Dialog has(e.g. ignore-click-outside) to the ignore list of the Drawer

or what about using the dialog html element and passing that to ignore list ?

@harlan-zw
Copy link
Contributor Author

harlan-zw commented Jul 27, 2023

Is there a branch I can use to test it like that video?

Could do ignore [role="dialog"], have pushed that up to test. Although maybe this PR isn't the best spot for the fix

@arashsheyda
Copy link
Member

Is there a branch I can use to test it like that video?

Could do ignore [role="dialog"], have pushed that up to test. Although maybe this PR isn't the best spot for the fix

There's no branch but I test it locally and it worked perfect, thanks.

@antfu
Copy link
Member

antfu commented Jul 28, 2023

You mean a11y right?

@harlan-zw harlan-zw changed the title fix(ui-kit): dialog closing and i18n fix(ui-kit): dialog closing and a11y Jul 28, 2023
@harlan-zw
Copy link
Contributor Author

You mean a11y right?

Yes.. too much time working with i18n on the sitemap module 🤦‍♂️

@antfu antfu merged commit 95bd0a1 into main Jul 30, 2023
@antfu antfu deleted the fix/ui-kit-dialog-closing branch July 30, 2023 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants