search

jquery / jquery-ui

The official jQuery user interface library.
https://jqueryui.com
Other
11.25k stars 5.33k forks source link

Dialog: Allow to put the dialog title in a heading element (h1-h6) #2271

Closed rpkoller closed 4 days ago

rpkoller commented 1 month ago

With https://github.com/jquery/jquery-ui/issues/2246 the element in the background of the dialog are getting removed from the AOM. The problem the title is just wrapped in a span which isn't semantic. The title is properly announced when dialog is opened (tested on drupal) but if the content of that modal doesnt contain any heading elements if you open up the rotor in for example voiceover the headings section wont be available at all. this might be potentially confusing since you are unable to get the context the dialog is about. there are no headings available and with VO-shift-i you only get announced the title and stats of the hidden page not the dialog modal. technically you would have to step through the modal with the voiceover cursor since the title isnt focusable or close the modal and reopen it again to get it reannounced.

which element to use instead of the generic span is not specified in the wcag, see this comment for context https://github.com/w3c/wcag/discussions/2722#discussioncomment-3842484 . according to the comment an h1 or any other heading level element is perfectly fine. the aria apg example uses an h2 for example: https://www.w3.org/WAI/ARIA/apg/patterns/dialog-modal/examples/dialog/ while in a discussion on the a11y slack https://web-a11y.slack.com/archives/C042TSFGN/p1708471859401949 the recommendation by a few folks to go with an h1. with the modal dialog open only the content of the modal is accessible the overall heading structure of the page is irrelevant. and james scholes emphasized that the modal title should be h1. there is no reason to use an h2 since all it does is to take away one heading level from the modal content (james is a screen reader user).

i "think" i've raised the aspect of the html element wrapping the title before in the context of the linked issue or the corresponding PR but unable to find the detail. but since it is still debatable which heading level to pick as well as the aspect of backwards compatability i wonder if it would be feasible to make the html element wrapping the title configurable? so adding somthing like titleElement to the options and set it to "span" per default that way it would be a none breaking change. the user would be able to to change the element proactively via the options?

mgol commented 1 month ago

Thanks for the report. I'd rather avoid adding more options as then we need to maintain them indefinitely. Since h1 seems to be OK per your description, I'd try that.

It'd be good to get that into 1.14.0 just in case someone considers this a compatibility break.

Would you like to submit a PR? I think we'd just need updating the wrapping span to a h1 in the uiDialogTitle definition and maybe add a simple test checking that the title is wrapped in a h1.

mgol commented 1 month ago

I've just realized we cannot do this for all dialogs as non-modal ones don't get the aria-modal attribute set so out-of-dialog headings are still available. This would have to be conditional on the aria-modal attribute - but the modal option that sets it can be set during runtime and you cannot change tagName of an existing element.

I guess adding an option like uiDialogTitleTagName would be the safest and also non-breaking.

I'll move this to the 1.14.1 milestone since I want to get the stable 1.14.0 out ASAP.

mgol commented 1 month ago

@fnagel what do you think?

rpkoller commented 1 month ago

I've just realized we cannot do this for all dialogs as non-modal ones don't get the aria-modal attribute set so out-of-dialog headings are still available. This would have to be conditional on the aria-modal attribute - but the modal option that sets it can be set during runtime and you cannot change tagName of an existing element.

I guess adding an option like uiDialogTitleTagName would be the safest and also non-breaking.

I'll move this to the 1.14.1 milestone since I want to get the stable 1.14.0 out ASAP.

uhhh you are right. and another downside i've just realized for changing the title wrapper from a span to an h1 is the heading hierarchy. in case the dialog modal is using more heading elements within the dialog. having an h1 might lead to a second h1 or a gap in between heading level. or if an h2 would have been picked instead of the h1. you can never know the content users will apply in their dialogs. so yeah adding an option would be the safest and none breaking approach. i could create an initial PR (going with uiDialogTitleTagName for the option name)