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

Add dialog role to editor find widget #172979

Merged
merged 8 commits into from
Jun 6, 2023

Conversation

jjaeggli
Copy link
Contributor

@jjaeggli jjaeggli commented Feb 1, 2023

This addresses adding semantic labels to the non-modal "Find" dialog. See issue #172861 The biggest issue this addresses is that the "Close" button had no programmatic relationship with the dialog and therefore it was unclear that the control was associated with the dialog from a screen reader perspective. Adding the dialog role provides grouping for all controls within the dialog. Additionally, this moves the close button out of the find find-part element. The reason for this is that find-part represents a semantic group of controls which are associated with the find input. The close button relates to the dialog, but not to the associated form group, therefore needs to be outside of this element. For issues related to programmatic grouping of input controls see #167885.

find-dialog-layout

This uses absolute positioning of the close control to be consistent with other cases where the controls are positioned outside of the usual flow. Not using flex hurts me just a little bit, but this maintains pixel perfect layout when compared to the previous version. Tab order of the control is also unchanged. This changes the screen reader output as tested in the following screen readers:

VoiceOver

this change

Find slash replace find, dialog, with 8 items, Find, edit text

<tab to close button>

Close escape, button, group

<tab to exit dialog, shift tab into dialog>

Find slash replace, dialog, with 8 items, close escape, button, group, main

orig

Find, edit text

<tab to close button>

Close escape, button, group

<tab to exit dialog, shift tab into dialog>

Close escape, button, group, main

NVDA

this change

Find slash replace dialog, Find edit multiline blank

<tab to close button>

Close, escape, button

<tab to exit dialog, shift tab into dialog>

Main landmark, Find slash replace dialog, close, escape, button

orig

Find edit multiline, find, blank

<tab to close button>

Close, escape, button

<tab to exit dialog, shift tab into dialog>

Main landmark, close, escape, button

JAWS

this change

Find slash Replace dialog, Find edit Contains text, type in text

<tab to close button>

Close left paren escape right paren button, to activate press enter

<tab to exit dialog, shift tab into dialog>

Main region, large dot ts, Find slash replace dialog, Close left paren escape right paren button, to activate press enter

orig

Find edit Contains text, type in text

<tab to close button>

Close left paren escape right paren button, to activate press enter

<tab to exit dialog, shift tab into dialog>

Main region, large dot ts, Close left paren escape right paren button, to activate press enter

@hediet hediet assigned andreamah and unassigned hediet Feb 2, 2023
@isidorn isidorn assigned rebornix and unassigned andreamah Feb 3, 2023
@rebornix rebornix added this to the March 2023 milestone Feb 7, 2023
@rebornix
Copy link
Member

rebornix commented Feb 7, 2023

Thanks for the contribution, it looks great to me. @meganrogge is now driving our accessibility story in editor along with @isidorn , let's put together a plan item for March, in case I don't get a chance to review and test this PR.

1 similar comment
@rebornix
Copy link
Member

rebornix commented Feb 7, 2023

Thanks for the contribution, it looks great to me. @meganrogge is now driving our accessibility story in editor along with @isidorn , let's put together a plan item for March, in case I don't get a chance to review and test this PR.

@rebornix rebornix modified the milestones: March 2023, April 2023 Mar 22, 2023
@rebornix rebornix modified the milestones: April 2023, On Deck Apr 26, 2023
@jjaeggli
Copy link
Contributor Author

jjaeggli commented May 8, 2023

Hi @meganrogge and @rebornix, it's been a while so I merged and verified that this PR is still producing the same behavior shown above. There did not appear to be any conflicts. Thanks!

@isidorn
Copy link
Contributor

isidorn commented May 10, 2023

Adding this to June milestone so we make sure to take this into account for next milestone planning @meganrogge
Thanks for understanding.

@isidorn isidorn modified the milestones: On Deck, June 2023 May 10, 2023
@isidorn isidorn added the accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues label May 10, 2023
@rebornix
Copy link
Member

rebornix commented Jun 6, 2023

Tested locally and this is working great. Thank you @jjaeggli

@rebornix rebornix enabled auto-merge (squash) June 6, 2023 19:02
@rebornix rebornix merged commit 56f11d9 into microsoft:main Jun 6, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants