[SolidusAdmin] Remove inaccessible details/summary element

[mamhoff]

Summary

The variant search on the order#show page utilized a details/summary HTML element in order to get open/close functionality. However, Axe flags this as inaccessible, because the summary element does not have a label. This removes the summary element in favor of a simple div, and changes the controller to rely on a hidden class instead.

This should fix our currently failing builds.

[kennyadsl]

@mamhoff wow, that's new to me. Can you link any resource supporting the fact that the summery element is not accessible? Maybe there's some workaround for that so we can use the native HTML element?

[mamhoff]

The error we get from Axe references this: https://dequeuniversity.com/rules/axe/4.10/summary-name

It's not that the element is not accessible, it's that the summary (which is hidden, and never clicked in this instance) does not have a label. IMO I don't think this is a case for the summary element semantically, as there's not summary that a user could actually click. In this instance we just used the open/close functionality to get a hidden or not hidden element...

[tvdeyen]

I am confused as well. Could we add an aria-label?

[benjaminwil]

I agree that <details> doesn’t make semantic sense here, but I think interactable things that users click should be buttons, not <div>s. My understanding is that using <div>s as buttons has its own accessibility problems

[mamhoff]

But - there is no button. The previous version used the details thing as the container for search results, and just plain hides the summary element, in order to then programmatically show or hide the results container. This is a div :)

[mamhoff]

This is a screenshot of the old HTML structure:

This is the new HTML structure:

You'll see there is no button. The result list for the variant search is hidden by default, but when there's a search term in the search form, it is programmatically unhidden. The whole thing does not have an option for the user to manually hide the search results, instead the user is required to click outside the div to hide the results.

[tvdeyen]

Ok. thank you very much for giving this context. I now wholeheartedly agree with you that this must be a div. one could argue for a ul li structure or dt dl, datalist, whatnot. but in the end it does not matter that much and the immediate issue is resolved. we can rework the variant select anytime, but not in this PR

[tvdeyen]

For future reference: This is the ideal candidate for a datalist element

[tvdeyen]

Installer Test is unrelated to this change. Since this blocks all open PRs right now, merging.

[github-actions]

💚 All backports created successfully

Status	Branch	Result
✅	v4.3

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

[MadelineCollier]

Thanks for this @mamhoff !