fix: Correct roles and add missing attributes
#1314
Conversation
…s ('dialog' or 'menu', rather than 'listbox').
🦋 Changeset detectedLatest commit: 3dbb0f8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/primer/primer-components/5DpWPkie8fgMqAkryzEkyJDJ3Mka |
smockle
requested review from
dgreif,
VanAnderson,
colebemis,
ichelsea and
khiga8
| @@ -7,7 +7,7 @@ export type DropdownButtonProps = ButtonProps | |||
|
|
|||
| export const DropdownButton = React.forwardRef<HTMLElement, React.PropsWithChildren<DropdownButtonProps>>( | |||
| ({children, ...props}: React.PropsWithChildren<DropdownButtonProps>, ref): JSX.Element => ( | |||
| <Button ref={ref} {...props}> | |||
| <Button ref={ref} type="button" {...props}> | |||
There was a problem hiding this comment.
If this is a standard html button, we shouldn't have to define the type should we?
There was a problem hiding this comment.
I think an HTML button defaults to type="submit" which is for form submission, but for javascript-y behavior, we'd want to specify type="button".
|
Would you mind linking the storybook or docs examples we should be examining that correspond to these changes? 😃 I am getting a little confused because we have so many dropdown type components |
|
@khiga8 the docs for these components aren't currently indexed in the docs, so it's tricky to track them down on your own. Here are the two main components affected by this change: https://primer-components-git-overlay-accessibility-primer.vercel.app/components/DropdownMenu |
|
As discussed in this week’s Primer React Planning meeting, it’s likely this PR will be closed and replaced, for a couple reasons:
|
Partial fix for https://github.com/github/primer/issues/217
<button>element (from<DropdownButton>) should havetype="button"<Overlay>(from<AnchoredOverlay>) should haverole="dialog"orrole="menu", notrole="listbox"<Item>s should have separate labels and descriptionsScreenshot
No visual change for
<Item descriptionVariant={"inline"} />mainbranchNo visual change for
<Item descriptionVariant={"block"} />mainbranchImproved
labelformenuitemnodemainbranchMerge checklist