Adds FormControl's "Auto-Wiring" to ActionMenu Component #4372
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@primer/react': minor | ||
| --- | ||
|
|
||
| ActionMenu: Automatically wires the ActionMenu component to the accessibility and validation provided by the FormControl component it's nested within. If ActionMenu isn't nested within FormControl, nothing changes. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,7 @@ import {useId} from '../hooks/useId' | |
| import type {MandateProps} from '../utils/types' | ||
| import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic' | ||
| import {Tooltip} from '../TooltipV2/Tooltip' | ||
| import {useFormControlForwardedProps} from '../FormControl' | ||
|
|
||
| export type MenuContextProps = Pick< | ||
| AnchoredOverlayProps, | ||
|
|
@@ -43,10 +44,12 @@ const Menu: React.FC<React.PropsWithChildren<ActionMenuProps>> = ({ | |
| open, | ||
| onOpenChange, | ||
| children, | ||
| ...externalInputProps | ||
| }: ActionMenuProps) => { | ||
| const [combinedOpenState, setCombinedOpenState] = useProvidedStateOrCreate(open, onOpenChange, false) | ||
| const onOpen = React.useCallback(() => setCombinedOpenState(true), [setCombinedOpenState]) | ||
| const onClose = React.useCallback(() => setCombinedOpenState(false), [setCombinedOpenState]) | ||
| const inputProps = useFormControlForwardedProps(externalInputProps) | ||
|
|
||
| const menuButtonChild = React.Children.toArray(children).find( | ||
| child => React.isValidElement<ActionMenuButtonProps>(child) && (child.type === MenuButton || child.type === Anchor), | ||
|
|
@@ -69,7 +72,7 @@ const Menu: React.FC<React.PropsWithChildren<ActionMenuProps>> = ({ | |
| renderAnchor = anchorProps => { | ||
| // We need to attach the anchor props to the tooltip trigger (ActionMenu.Button's grandchild) not the tooltip itself. | ||
| const triggerButton = React.cloneElement(anchorChildren, {...anchorProps}) | ||
| return React.cloneElement(child, {children: triggerButton, ref: anchorRef, ...inputProps}) | ||
|
There was a problem hiding this comment. Followed the pattern suggested in this slack thread: https://github.slack.com/archives/CSGAVNZ19/p1709843761424779?thread_ts=1709836721.563139&cid=CSGAVNZ19 |
||
| } | ||
| } | ||
| return null | ||
|
|
@@ -84,15 +87,15 @@ const Menu: React.FC<React.PropsWithChildren<ActionMenuProps>> = ({ | |
| // We need to attach the anchor props to the tooltip trigger not the tooltip itself. | ||
| const tooltipTriggerEl = React.cloneElement(tooltipTrigger, {...anchorProps}) | ||
| const tooltip = React.cloneElement(anchorChildren, {children: tooltipTriggerEl}) | ||
| return React.cloneElement(child, {children: tooltip, ref: anchorRef, ...inputProps}) | ||
| } | ||
| } | ||
| } else { | ||
| renderAnchor = anchorProps => React.cloneElement(child, {...anchorProps, ...inputProps}) | ||
| } | ||
| return null | ||
| } else if (child.type === MenuButton) { | ||
| renderAnchor = anchorProps => React.cloneElement(child, {...anchorProps, ...inputProps}) | ||
| return null | ||
| } else { | ||
| return child | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,20 +22,6 @@ exports[`Token components AvatarToken renders all sizes 1`] = ` | |
| border-width: 0; | ||
| } | ||
|
|
||
|
There was a problem hiding this comment. I updated snapshots as detailed in the contributor docs: https://github.com/primer/react/blob/cb54f4298700d369e616aaefb2e7df4830231736/contributor-docs/testing.md#snapshots Not sure why these classes had to be rearranged as it's not clear to me how |
||
| .c0 { | ||
| -webkit-align-items: center; | ||
| -webkit-box-align: center; | ||
|
|
@@ -155,6 +141,20 @@ exports[`Token components AvatarToken renders all sizes 1`] = ` | |
| bottom: 0; | ||
| } | ||
|
|
||
| .c3 { | ||
| display: inline-block; | ||
| overflow: hidden; | ||
| line-height: 1; | ||
| vertical-align: middle; | ||
| border-radius: 50%; | ||
| box-shadow: 0 0 0 1px var(--avatar-borderColor,var(--color-avatar-border,rgba(31,35,40,0.15))); | ||
| height: var(--avatar-size); | ||
| width: var(--avatar-size); | ||
| --avatar-size: 16px; | ||
| width: 100%; | ||
| height: 100%; | ||
| } | ||
|
|
||
| .c2 { | ||
| --spacing: calc(4px * 2); | ||
| display: block; | ||
|
|
@@ -248,20 +248,6 @@ exports[`Token components AvatarToken renders all sizes 2`] = ` | |
| border-width: 0; | ||
| } | ||
|
|
||
| .c0 { | ||
| -webkit-align-items: center; | ||
| -webkit-box-align: center; | ||
|
|
@@ -381,6 +367,20 @@ exports[`Token components AvatarToken renders all sizes 2`] = ` | |
| bottom: 0; | ||
| } | ||
|
|
||
| .c3 { | ||
| display: inline-block; | ||
| overflow: hidden; | ||
| line-height: 1; | ||
| vertical-align: middle; | ||
| border-radius: 50%; | ||
| box-shadow: 0 0 0 1px var(--avatar-borderColor,var(--color-avatar-border,rgba(31,35,40,0.15))); | ||
| height: var(--avatar-size); | ||
| width: var(--avatar-size); | ||
| --avatar-size: 20px; | ||
| width: 100%; | ||
| height: 100%; | ||
| } | ||
|
|
||
| .c2 { | ||
| --spacing: calc(4px * 2); | ||
| display: block; | ||
|
|
@@ -474,20 +474,6 @@ exports[`Token components AvatarToken renders all sizes 3`] = ` | |
| border-width: 0; | ||
| } | ||
|
|
||
| .c0 { | ||
| -webkit-align-items: center; | ||
| -webkit-box-align: center; | ||
|
|
@@ -607,6 +593,20 @@ exports[`Token components AvatarToken renders all sizes 3`] = ` | |
| bottom: 0; | ||
| } | ||
|
|
||
| .c3 { | ||
| display: inline-block; | ||
| overflow: hidden; | ||
| line-height: 1; | ||
| vertical-align: middle; | ||
| border-radius: 50%; | ||
| box-shadow: 0 0 0 1px var(--avatar-borderColor,var(--color-avatar-border,rgba(31,35,40,0.15))); | ||
| height: var(--avatar-size); | ||
| width: var(--avatar-size); | ||
| --avatar-size: 24px; | ||
| width: 100%; | ||
| height: 100%; | ||
| } | ||
|
|
||
| .c2 { | ||
| --spacing: calc(4px * 2); | ||
| display: block; | ||
|
|
@@ -700,20 +700,6 @@ exports[`Token components AvatarToken renders all sizes 4`] = ` | |
| border-width: 0; | ||
| } | ||
|
|
||
| .c0 { | ||
| -webkit-align-items: center; | ||
| -webkit-box-align: center; | ||
|
|
@@ -835,6 +821,20 @@ exports[`Token components AvatarToken renders all sizes 4`] = ` | |
| bottom: 0; | ||
| } | ||
|
|
||
| .c3 { | ||
| display: inline-block; | ||
| overflow: hidden; | ||
| line-height: 1; | ||
| vertical-align: middle; | ||
| border-radius: 50%; | ||
| box-shadow: 0 0 0 1px var(--avatar-borderColor,var(--color-avatar-border,rgba(31,35,40,0.15))); | ||
| height: var(--avatar-size); | ||
| width: var(--avatar-size); | ||
| --avatar-size: 32px; | ||
| width: 100%; | ||
| height: 100%; | ||
| } | ||
|
|
||
| .c2 { | ||
| --spacing: calc(4px * 2); | ||
| display: block; | ||
|
|
@@ -915,20 +915,6 @@ exports[`Token components AvatarToken renders isSelected 1`] = ` | |
| margin-right: 4px; | ||
| } | ||
|
|
||
| .c0 { | ||
| -webkit-align-items: center; | ||
| -webkit-box-align: center; | ||
|
|
@@ -998,6 +984,20 @@ exports[`Token components AvatarToken renders isSelected 1`] = ` | |
| bottom: 0; | ||
| } | ||
|
|
||
| .c3 { | ||
| display: inline-block; | ||
| overflow: hidden; | ||
| line-height: 1; | ||
| vertical-align: middle; | ||
| border-radius: 50%; | ||
| box-shadow: 0 0 0 1px var(--avatar-borderColor,var(--color-avatar-border,rgba(31,35,40,0.15))); | ||
| height: var(--avatar-size); | ||
| width: var(--avatar-size); | ||
| --avatar-size: 20px; | ||
| width: 100%; | ||
| height: 100%; | ||
| } | ||
|
|
||
| .c2 { | ||
| --spacing: calc(4px * 2); | ||
| display: block; | ||
|
|
@@ -1056,20 +1056,6 @@ exports[`Token components AvatarToken renders with a remove button 1`] = ` | |
| border-width: 0; | ||
| } | ||
|
|
||
| .c0 { | ||
| -webkit-align-items: center; | ||
| -webkit-box-align: center; | ||
|
|
@@ -1189,6 +1175,20 @@ exports[`Token components AvatarToken renders with a remove button 1`] = ` | |
| bottom: 0; | ||
| } | ||
|
|
||
| .c3 { | ||
| display: inline-block; | ||
| overflow: hidden; | ||
| line-height: 1; | ||
| vertical-align: middle; | ||
| border-radius: 50%; | ||
| box-shadow: 0 0 0 1px var(--avatar-borderColor,var(--color-avatar-border,rgba(31,35,40,0.15))); | ||
| height: var(--avatar-size); | ||
| width: var(--avatar-size); | ||
| --avatar-size: 20px; | ||
| width: 100%; | ||
| height: 100%; | ||
| } | ||
|
|
||
| .c2 { | ||
| --spacing: calc(4px * 2); | ||
| display: block; | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,7 @@ import userEvent from '@testing-library/user-event' | |
| import {axe} from 'jest-axe' | ||
| import React from 'react' | ||
| import theme from '../theme' | ||
| import {ActionMenu, ActionList, BaseStyles, ThemeProvider, SSRProvider, Tooltip, Button, FormControl} from '..' | ||
| import {Tooltip as TooltipV2} from '../TooltipV2/Tooltip' | ||
| import {behavesAsComponent, checkExports} from '../utils/testing' | ||
| import {SingleSelect} from '../ActionMenu/ActionMenu.features.stories' | ||
|
|
@@ -77,6 +77,24 @@ function ExampleWithTooltipV2(actionMenuTrigger: React.ReactElement): JSX.Elemen | |
| ) | ||
| } | ||
|
|
||
| function ExampleWithForm(): JSX.Element { | ||
| return ( | ||
| <FormControl> | ||
| <FormControl.Label>Action Menu Label</FormControl.Label> | ||
| <Example /> | ||
| </FormControl> | ||
| ) | ||
| } | ||
|
|
||
| function ExampleWithTooltipWithForm(): JSX.Element { | ||
| return ( | ||
| <FormControl> | ||
| <FormControl.Label>Action Menu Label</FormControl.Label> | ||
| <ExampleWithTooltip /> | ||
| </FormControl> | ||
| ) | ||
| } | ||
|
|
||
|
There was a problem hiding this comment. I tried adding a test case for TooltipV2 function ExampleWithTooltipV2WithForm(): JSX.Element {
return (
<FormControl>
<FormControl.Label>Action Menu Label</FormControl.Label>
<ThemeProvider theme={theme}>
<SSRProvider>
<BaseStyles>
<ActionMenu>
<TooltipV2 text="Additional context about the menu button" direction="s">
<ActionMenu.Button>Toggle Menu</ActionMenu.Button>
</TooltipV2>
<ActionMenu.Overlay>
<ActionList>
<ActionList.Item>New file</ActionList.Item>
</ActionList>
</ActionMenu.Overlay>
</ActionMenu>
</BaseStyles>
</SSRProvider>
</ThemeProvider>
</FormControl>
)
} it('MenuButton wrapped by TooltipV2 within FormControl should be labelled by FormControl.Label', async () => {
const component = HTMLRender(<ExampleWithTooltipV2WithForm />)
const button = component.getByLabelText('Action Menu Label')
expect(button).toBeVisible()
const buttonByRole = component.getByRole('button')
expect(button.id).toBe(buttonByRole.id)
})But I got this error because the If I try to force the function ExampleWithTooltipV2WithForm(): JSX.Element {
return (
<FormControl>
<FormControl.Label>Action Menu Label</FormControl.Label>
<ThemeProvider theme={theme}>
<SSRProvider>
<BaseStyles>
<TooltipV2 text="Additional context about the menu button" direction="s">
<ActionMenu>
<ActionMenu.Button>Toggle Menu</ActionMenu.Button>
<ActionMenu.Overlay>
<ActionList>
<ActionList.Item>New file</ActionList.Item>
</ActionList>
</ActionMenu.Overlay>
</ActionMenu>
</TooltipV2>
</BaseStyles>
</SSRProvider>
</ThemeProvider>
</FormControl>
)
}I get this error since that's not how Either way I'm not concerned about not having a test including |
||
| describe('ActionMenu', () => { | ||
| behavesAsComponent({ | ||
| Component: ActionList, | ||
|
|
@@ -398,4 +416,20 @@ describe('ActionMenu', () => { | |
|
|
||
| expect(button.id).toBe(buttonId) | ||
| }) | ||
|
|
||
| it('MenuButton within FormControl should be labelled by FormControl.Label', async () => { | ||
| const component = HTMLRender(<ExampleWithForm />) | ||
| const button = component.getByLabelText('Action Menu Label') | ||
| expect(button).toBeVisible() | ||
| const buttonByRole = component.getByRole('button') | ||
| expect(button.id).toBe(buttonByRole.id) | ||
| }) | ||
|
|
||
| it('MenuButton wrapped by Tooltip within FormControl should be labelled by FormControl.Label', async () => { | ||
| const component = HTMLRender(<ExampleWithTooltipWithForm />) | ||
| const button = component.getByLabelText('Action Menu Label') | ||
| expect(button).toBeVisible() | ||
| const buttonByRole = component.getByRole('button') | ||
| expect(button.id).toBe(buttonByRole.id) | ||
| }) | ||
| }) | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using the hook recently created by this PR: #3632. This hook is also being used in the
InlineAutocompletedraft component:react/packages/react/src/drafts/InlineAutocomplete/InlineAutocomplete.tsx
Line 118 in cb54f42