SelectPanel anchors should have aria-haspopup="dialog"

[electron97]

Summary

• Currently the anchors of SelectPanel have aria-haspopup="true" which is incorrect.
• Since the value was set to aria-haspopup="true" by default in AnchoredOverlay.tsx.
• To overwrite that we need to add aria-haspopup="listbox" in SelectPanel.stories.tsx renderAnchor to fix the issue.

Closes #3629

Screenshots

Before Fix	After Fix

Merge checklist

• Added/updated tests
• Added/updated documentation
• Changes are SSR compatible
• Tested in Chrome
• Tested in Firefox
• Tested in Safari
• Tested in Edge

Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.

[changeset-bot]

🦋 Changeset detected

Latest commit: 505741e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@primer/react	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[joshblack]

Thanks so much for taking the time to open this PR! Just wanted to share this as a heads up, we're sync'ing up with the accessibility team here at GitHub to double-check the intended behavior for the upstream issue. We should be meeting on Friday this week and we'll figure out next steps from there. Hope this makes sense! Let me know if you have any questions.

[electron97]

Thanks so much for taking the time to open this PR! Just wanted to share this as a heads up, we're sync'ing up with the accessibility team here at GitHub to double-check the intended behavior for the upstream issue. We should be meeting on Friday this week and we'll figure out next steps from there. Hope this makes sense! Let me know if you have any questions.

Hey @joshblack ,

Thank you so much for taking your time and taking a look at the PR. This gives me motivation to contribute more to primer/react. Also I have seen your comment yesterday on the issue where you mentioned there will be discussion first regarding this which also made me think Did I understand completely why aria-haspopup equal to listbox and not true? I have worked on few accessibility issues myself in the past and then I started reading about it and I learned what aria-haspopup attribute is actually used for. Thanks again for taking your time and giving me the heads up will keep on contributing to primer/react.

Thanks!

[joshblack]

Thanks so much for your patience! Requesting a small change to dialog based on the feedback in the issue. Let me know if you have any questions!

[joshblack]

Suggested change

	<Button
	trailingAction={TriangleDownIcon}
	aria-labelledby={` ${ariaLabelledBy}`}
	{...anchorProps}
	aria-haspopup="listbox"
	>
	<Button
	trailingAction={TriangleDownIcon}
	aria-labelledby={` ${ariaLabelledBy}`}
	aria-haspopup="dialog"
	{...anchorProps}
	>

[electron97]

Hey @joshblack,

The order which you suggested will not work

aria-haspopup="dialog"
{...anchorProps}

because first it will take the value of aria-haspopup to dialog then it will overwrite the value with true from anchorProps. So to fix that we need first pass anchorProps and then dialog.

{...anchorProps}
aria-haspopup="dialog"

Please let me know if you have any questions.
I have updated the screenshots in the PR description with the current changes.

Also I found something while testing the changes locally that the same issue still exists in SelectPanel.features.stories.tsx. Please let me know if should we fix this issue in this PR as well or open a completely new issue.

Thanks!

[electron97]

hey @joshblack ✋ ,
It's been two months since this issue was raised so wanted to check with you once with the latest update.
Is this component now a legacy component since there is SelectPanel2 and that's why we are not working on fixing the a11y issue or do you think the solution that I came up with is not the best solution for this issue?
Thanks!

[joshblack]

Hey @electron97! So sorry about the delay, re-checking this and I think the changes look great 👍

For SelectPanel, there has been new work on the V2 version but I think this change totally makes sense as-is. Thanks so much for making this PR!