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.

Sign up for GitHub

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

Jump to bottom

fix(NcPopover): add aria-modal to some popover-based dialog components #5309

Merged
merged 1 commit into from
Feb 29, 2024
Merged

fix(NcPopover): add aria-modal to some popover-based dialog components #5309

merged 1 commit into from
Feb 29, 2024

Conversation

ShGKme
Copy link
Contributor

@ShGKme ShGKme commented Feb 26, 2024 •
edited
Loading

☑️ Resolves

  • Our dialogs should be considered modal because they have focus-trap and click outside closes the dialog

🏁 Checklist

  • ⛑️ Tests are included or are not applicable
  • 📘 Component documentation has been extended, updated or is not applicable
  • 3️⃣ Backport to next requested with a Vue 3 upgrade

@ShGKme ShGKme added bug Something isn't working feature: popover Related to the popovermenu component feature: actions Related to the actions components accessibility Making sure we design for the widest range of people possible, including those who have disabilities feature: colorpicker Related to the colorpicker component feature: emoji picker Related to the emoji picker component labels Feb 26, 2024
@ShGKme ShGKme added this to the 8.7.2 milestone Feb 26, 2024
@ShGKme ShGKme self-assigned this Feb 26, 2024
@ShGKme ShGKme force-pushed the fix/nc-popover--add-aria-modal branch from 9da8c75 to f2aa153 Compare February 26, 2024 20:31
@ShGKme
fix(NcPopover): add aria-modal to some popover-based dialog components
dd7ba31 
Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
@ShGKme ShGKme force-pushed the fix/nc-popover--add-aria-modal branch from f2aa153 to dd7ba31 Compare February 29, 2024 08:29
@ShGKme ShGKme marked this pull request as ready for review February 29, 2024 08:32
JuliaKirschenheuter
JuliaKirschenheuter approved these changes Feb 29, 2024
View reviewed changes
Copy link
Contributor

@JuliaKirschenheuter JuliaKirschenheuter left a comment

Choose a reason for hiding this comment

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

works! ;)

@Antreesy Antreesy merged commit 77bf8b9 into master Feb 29, 2024
19 checks passed
@Antreesy Antreesy deleted the fix/nc-popover--add-aria-modal branch February 29, 2024 10:18
@Antreesy Antreesy mentioned this pull request Feb 29, 2024
Closed
@ShGKme
Copy link
Contributor Author

ShGKme commented Feb 29, 2024

/backport to next

@backportbot backportbot bot mentioned this pull request Feb 29, 2024
Merged
2 tasks
@ShGKme ShGKme mentioned this pull request Feb 29, 2024
Merged
@ShGKme ShGKme modified the milestones: 8.7.2, 8.8.0 Feb 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Antreesy Antreesy Antreesy approved these changes

@JuliaKirschenheuter JuliaKirschenheuter JuliaKirschenheuter approved these changes

@susnux susnux Awaiting requested review from susnux

@Pytal Pytal Awaiting requested review from Pytal

Assignees

@ShGKme ShGKme

Labels
accessibility Making sure we design for the widest range of people possible, including those who have disabilities bug Something isn't working feature: actions Related to the actions components feature: colorpicker Related to the colorpicker component feature: emoji picker Related to the emoji picker component feature: popover Related to the popovermenu component
Projects
None yet
Milestone
8.8.0
Development

Successfully merging this pull request may close these issues.
3 participants
@ShGKme @JuliaKirschenheuter @Antreesy