Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds FormControl's "Auto-Wiring" to SelectPanel v2 Component #4389

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
0f4cec5
autowires SelectPanelv2 to FormControl
JeffreyKozik Mar 13, 2024
06b048a
added changeset
JeffreyKozik Mar 13, 2024
5e46bce
Update afraid-beds-lick.md
siddharthkp Mar 14, 2024
d1df0dd
Button is aria-labelledby by 2 different components now
JeffreyKozik Mar 19, 2024
3d69e09
using querySelector instead of VisuallyHidden approach
JeffreyKozik Mar 19, 2024
969c97c
Merge branch 'main' into jeffreykozik/autolinking_selectpanelv2_to_fo…
JeffreyKozik Mar 19, 2024
4d7b5d7
Added visually hidden punctuation
JeffreyKozik Mar 20, 2024
616ad2b
removed selectPanelButtonId
JeffreyKozik Mar 22, 2024
d26d79a
hiding visuallyhidden text from screen readers as well
JeffreyKozik Mar 22, 2024
d6b08a2
use ariaLabel instead of ariaLabelledby
JeffreyKozik Mar 23, 2024
6ea27c7
added a 2nd test for complex button case
JeffreyKozik Mar 23, 2024
006a61b
updated tests
JeffreyKozik Mar 23, 2024
e8aa873
Merge branch 'main' into jeffreykozik/autolinking_selectpanelv2_to_fo…
JeffreyKozik Mar 26, 2024
672aaab
using aria-labelledby to reference itself, updated tests
JeffreyKozik Mar 26, 2024
c3e1594
flipped order within aria-label
JeffreyKozik Mar 26, 2024
71ea3b5
updated conditional, added negative test
JeffreyKozik Mar 26, 2024
a3db265
Merge branch 'main' into jeffreykozik/autolinking_selectpanelv2_to_fo…
JeffreyKozik Apr 17, 2024
e6d93e1
removed aria-labelledby
JeffreyKozik Apr 17, 2024
38d9748
Merge branch 'main' into jeffreykozik/autolinking_selectpanelv2_to_fo…
JeffreyKozik Apr 19, 2024
856e957
test
JeffreyKozik Apr 19, 2024
eacfa7c
fix
JeffreyKozik Apr 19, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/afraid-beds-lick.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/react': minor
---

Automatically wires the SelectPanel v2 component to the accessibility and validation provided by the FormControl component it's nested within. If SelectPanel v2 isn't nested within FormControl, nothing changes.
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React from 'react'
import {SelectPanel} from './SelectPanel'
import {ActionList, ActionMenu, Avatar, Box, Button, Text, Octicon, Flash} from '../../index'
import {ActionList, ActionMenu, Avatar, Box, Button, Text, Octicon, Flash, FormControl} from '../../index'
import {Dialog} from '../../drafts'
import {
ArrowRightIcon,
Expand Down Expand Up @@ -846,6 +846,38 @@ export const NestedSelection = () => {
)
}

export const WithinForm = () => {
const [selectedTag, setSelectedTag] = React.useState<string>()

const onSubmit = () => {
if (!selectedTag) return
data.ref = selectedTag // pretending to persist changes
}

const itemsToShow = data.tags

return (
<>
<h1>Within Form</h1>

<FormControl>
<FormControl.Label>SelectPanel within FormControl</FormControl.Label>
<SelectPanel title="Choose a tag" selectionVariant="instant" onSubmit={onSubmit}>
<SelectPanel.Button leadingVisual={TagIcon}>{selectedTag || 'Choose a tag'}</SelectPanel.Button>

<ActionList>
{itemsToShow.map(tag => (
<ActionList.Item key={tag.id} onSelect={() => setSelectedTag(tag.id)} selected={selectedTag === tag.id}>
{tag.name}
</ActionList.Item>
))}
</ActionList>
</SelectPanel>
</FormControl>
</>
)
}

// ----- Suspense implementation details ----

const cache = new Map()
Expand Down
19 changes: 18 additions & 1 deletion packages/react/src/drafts/SelectPanel2/SelectPanel.test.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React from 'react'
import {ThemeProvider, ActionList} from '../../'
import {ThemeProvider, ActionList, FormControl} from '../../'
import type {RenderResult} from '@testing-library/react'
import {render} from '@testing-library/react'
import type {UserEvent} from '@testing-library/user-event'
Expand Down Expand Up @@ -43,6 +43,15 @@ const Fixture = ({onSubmit, onCancel}: Pick<SelectPanelProps, 'onSubmit' | 'onCa
)
}

function SelectPanelWithForm(): JSX.Element {
return (
<FormControl>
<FormControl.Label>Select Panel Label</FormControl.Label>
<Fixture />
</FormControl>
)
}

describe('SelectPanel', () => {
it('renders Button by default', async () => {
const container = render(<Fixture />)
Expand Down Expand Up @@ -132,4 +141,12 @@ describe('SelectPanel', () => {
expect(mockOnCancel).toHaveBeenCalledTimes(1)
expect(mockOnSubmit).toHaveBeenCalledTimes(0)
})

it('SelectPanel within FormControl should be labelled by FormControl.Label', async () => {
const component = render(<SelectPanelWithForm />)
const button = component.getByLabelText('Select Panel Label')
expect(button).toBeVisible()
const buttonByRole = component.getByRole('button')
expect(button.id).toBe(buttonByRole.id)
})
})
18 changes: 16 additions & 2 deletions packages/react/src/drafts/SelectPanel2/SelectPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,20 @@ import {SearchIcon, XCircleFillIcon, XIcon, FilterRemoveIcon, AlertIcon, ArrowLe
import {FocusKeys} from '@primer/behaviors'

import type {ButtonProps, TextInputProps, ActionListProps, LinkProps, CheckboxProps} from '../../index'
import {Button, IconButton, Heading, Box, Tooltip, TextInput, Spinner, Text, Octicon, Link, Checkbox} from '../../index'
import {
Button,
IconButton,
Heading,
Box,
Tooltip,
TextInput,
Spinner,
Text,
Octicon,
Link,
Checkbox,
useFormControlForwardedProps,
Copy link
Contributor Author

@JeffreyKozik JeffreyKozik Mar 13, 2024

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 @iansan5653 in this PR: #3632. This hook is also being used in the InlineAutocomplete draft component:

const inputProps = useFormControlForwardedProps(externalInputProps)

} from '../../index'
import {ActionListContainerContext} from '../../ActionList/ActionListContainerContext'
import {useSlots} from '../../hooks/useSlots'
import {useProvidedRefOrCreate, useId, useAnchoredPosition} from '../../hooks'
Expand Down Expand Up @@ -315,7 +328,8 @@ const Panel: React.FC<SelectPanelProps> = ({
}

const SelectPanelButton = React.forwardRef<HTMLButtonElement, ButtonProps>((props, anchorRef) => {
return <Button ref={anchorRef} {...props} />
const inputProps = useFormControlForwardedProps(props)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return <Button ref={anchorRef} {...inputProps} />
})

const SelectPanelHeader: React.FC<React.PropsWithChildren & {onBack?: () => void}> = ({children, onBack, ...props}) => {
Expand Down
Loading