From 3571658723aa33e731a3a3b498d18a174113d4cd Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Thu, 11 Aug 2022 19:25:10 -0400 Subject: [PATCH] Make SegmentedControl uncontrolled by default (#2189) * makes SegmentedControl uncontrolled by default * corrects onChange in SegmentedControl prop table * adds changeset * fixes broken test * adds selected and defaultSelected props, adds controlled SegmentedControl example to docs * fixes docs 'Controlled' example --- .changeset/cuddly-experts-wash.md | 5 ++ .../content/{ => drafts}/SegmentedControl.mdx | 84 ++++++++++++------- .../SegmentedControl.test.tsx | 19 +++++ src/SegmentedControl/SegmentedControl.tsx | 34 ++++---- .../SegmentedControlButton.tsx | 4 +- .../SegmentedControlIconButton.tsx | 4 +- .../SegmentedControl.test.tsx.snap | 3 + 7 files changed, 105 insertions(+), 48 deletions(-) create mode 100644 .changeset/cuddly-experts-wash.md rename docs/content/{ => drafts}/SegmentedControl.mdx (70%) diff --git a/.changeset/cuddly-experts-wash.md b/.changeset/cuddly-experts-wash.md new file mode 100644 index 00000000000..4064a6d94f9 --- /dev/null +++ b/.changeset/cuddly-experts-wash.md @@ -0,0 +1,5 @@ +--- +'@primer/react': patch +--- + +Makes SegmentedControl uncontrolled by default. diff --git a/docs/content/SegmentedControl.mdx b/docs/content/drafts/SegmentedControl.mdx similarity index 70% rename from docs/content/SegmentedControl.mdx rename to docs/content/drafts/SegmentedControl.mdx index 62c4f3b712d..3ec51266a2a 100644 --- a/docs/content/SegmentedControl.mdx +++ b/docs/content/drafts/SegmentedControl.mdx @@ -4,25 +4,45 @@ status: Draft description: Use a segmented control to let users select an option from a short list and immediately apply the selection --- -Not implemented yet - ## Examples -### Simple +### Uncontrolled (default) ```jsx live drafts - Preview + Preview Raw Blame ``` +### Controlled + +```javascript noinline live drafts +const Controlled = () => { + const [selectedIndex, setSelectedIndex] = React.useState(0) + + const handleSegmentChange = selectedIndex => { + setSelectedIndex(selectedIndex) + } + + return ( + + Preview + Raw + Blame + + ) +} + +render(Controlled) +``` + ### With icons and labels ```jsx live drafts - + Preview Raw @@ -34,7 +54,7 @@ description: Use a segmented control to let users select an option from a short ```jsx live drafts - + @@ -44,7 +64,7 @@ description: Use a segmented control to let users select an option from a short ```jsx live drafts - + Preview Raw @@ -56,7 +76,7 @@ description: Use a segmented control to let users select an option from a short ```jsx live drafts - + Preview Raw @@ -68,7 +88,7 @@ description: Use a segmented control to let users select an option from a short ```jsx live drafts - Preview + Preview Raw Blame @@ -78,7 +98,7 @@ description: Use a segmented control to let users select an option from a short ```jsx live drafts - Preview + Preview Raw Blame @@ -97,7 +117,7 @@ description: Use a segmented control to let users select an option from a short - Preview + Preview Raw Blame @@ -110,7 +130,7 @@ description: Use a segmented control to let users select an option from a short File view - Preview + Preview Raw Blame @@ -122,23 +142,8 @@ description: Use a segmented control to let users select an option from a short ```jsx live drafts - Preview - Raw - Blame - -``` - -### With a selection change handler - -```jsx live drafts - { - alert(`Segment ${selectedIndex}`) - }} -> Preview - Raw + Raw Blame ``` @@ -166,7 +171,6 @@ description: Use a segmented control to let users select an option from a short name="onChange" type="(selectedIndex?: number) => void" description="The handler that gets called when a segment is selected" - required /> - + + @@ -202,7 +215,16 @@ description: Use a segmented control to let users select an option from a short description="The icon that represents the segmented control item" required /> - + + diff --git a/src/SegmentedControl/SegmentedControl.test.tsx b/src/SegmentedControl/SegmentedControl.test.tsx index 1129507e80e..f9e1df4962f 100644 --- a/src/SegmentedControl/SegmentedControl.test.tsx +++ b/src/SegmentedControl/SegmentedControl.test.tsx @@ -175,6 +175,25 @@ describe('SegmentedControl', () => { expect(handleChange).toHaveBeenCalledWith(1) }) + it('changes selection to the clicked segment even without onChange being passed', async () => { + const user = userEvent.setup() + const {getByText} = render( + + {segmentData.map(({label}) => ( + {label} + ))} + + ) + + const buttonToClick = getByText('Raw').closest('button') + + expect(buttonToClick?.getAttribute('aria-current')).toBe('false') + if (buttonToClick) { + await user.click(buttonToClick) + } + expect(buttonToClick?.getAttribute('aria-current')).toBe('true') + }) + it('calls segment button onClick if it is passed', async () => { const user = userEvent.setup() const handleClick = jest.fn() diff --git a/src/SegmentedControl/SegmentedControl.tsx b/src/SegmentedControl/SegmentedControl.tsx index 2a7e054336e..9d07be8b227 100644 --- a/src/SegmentedControl/SegmentedControl.tsx +++ b/src/SegmentedControl/SegmentedControl.tsx @@ -1,4 +1,4 @@ -import React, {useRef} from 'react' +import React, {useRef, useState} from 'react' import Button, {SegmentedControlButtonProps} from './SegmentedControlButton' import SegmentedControlIconButton, {SegmentedControlIconButtonProps} from './SegmentedControlIconButton' import {ActionList, ActionMenu, useTheme} from '..' @@ -51,6 +51,11 @@ const Root: React.FC> = ({ }) => { const segmentedControlContainerRef = useRef(null) const {theme} = useTheme() + const isUncontrolled = + onChange === undefined || + React.Children.toArray(children).some( + child => React.isValidElement(child) && child.props.defaultSelected !== undefined + ) const responsiveVariant = useResponsiveValue(variant, 'default') const isFullWidth = useResponsiveValue(fullWidth, false) const selectedSegments = React.Children.toArray(children).map( @@ -58,7 +63,9 @@ const Root: React.FC> = ({ React.isValidElement(child) && child.props.selected ) const hasSelectedButton = selectedSegments.some(isSelected => isSelected) - const selectedIndex = hasSelectedButton ? selectedSegments.indexOf(true) : 0 + const selectedIndexExternal = hasSelectedButton ? selectedSegments.indexOf(true) : 0 + const [selectedIndexInternalState, setSelectedIndexInternalState] = useState(selectedIndexExternal) + const selectedIndex = isUncontrolled ? selectedIndexInternalState : selectedIndexExternal const selectedChild = React.isValidElement( React.Children.toArray(children)[selectedIndex] ) @@ -108,18 +115,11 @@ const Root: React.FC> = ({ | React.KeyboardEvent) => { - onChange(index) - // TODO: figure out a way around the typecasting - child.props.onClick && child.props.onClick(event as React.MouseEvent) - } - : // TODO: figure out a way around the typecasting - (child.props.onClick as ( - event: React.MouseEvent | React.KeyboardEvent - ) => void) - } + onSelect={(event: React.MouseEvent | React.KeyboardEvent) => { + isUncontrolled && setSelectedIndexInternalState(index) + onChange && onChange(index) + child.props.onClick && child.props.onClick(event as React.MouseEvent) + }} > {ChildIcon && } {getChildText(child)} @@ -146,9 +146,13 @@ const Root: React.FC> = ({ onClick: onChange ? (event: React.MouseEvent) => { onChange(index) + isUncontrolled && setSelectedIndexInternalState(index) child.props.onClick && child.props.onClick(event) } - : child.props.onClick, + : (event: React.MouseEvent) => { + child.props.onClick && child.props.onClick(event) + isUncontrolled && setSelectedIndexInternalState(index) + }, selected: index === selectedIndex, sx: { '--separator-color': diff --git a/src/SegmentedControl/SegmentedControlButton.tsx b/src/SegmentedControl/SegmentedControlButton.tsx index 44ec9716a37..cc89d2e3f37 100644 --- a/src/SegmentedControl/SegmentedControlButton.tsx +++ b/src/SegmentedControl/SegmentedControlButton.tsx @@ -8,8 +8,10 @@ import {getSegmentedControlButtonStyles, getSegmentedControlListItemStyles} from export type SegmentedControlButtonProps = { /** The visible label rendered in the button */ children: string - /** Whether the segment is selected */ + /** Whether the segment is selected. This is used for controlled `SegmentedControls`, and needs to be updated using the `onChange` handler on `SegmentedControl`. */ selected?: boolean + /** Whether the segment is selected. This is used for uncontrolled `SegmentedControls` to pick one `SegmentedControlButton` that is selected on the initial render. */ + defaultSelected?: boolean /** The leading icon comes before item label */ leadingIcon?: React.FunctionComponent> } & SxProp & diff --git a/src/SegmentedControl/SegmentedControlIconButton.tsx b/src/SegmentedControl/SegmentedControlIconButton.tsx index 2fd0225c5fc..04cfec124e4 100644 --- a/src/SegmentedControl/SegmentedControlIconButton.tsx +++ b/src/SegmentedControl/SegmentedControlIconButton.tsx @@ -10,8 +10,10 @@ export type SegmentedControlIconButtonProps = { 'aria-label': string /** The icon that represents the segmented control item */ icon: React.FunctionComponent> - /** Whether the segment is selected */ + /** Whether the segment is selected. This is used for controlled SegmentedControls, and needs to be updated using the onChange handler on SegmentedControl. */ selected?: boolean + /** Whether the segment is selected. This is used for uncontrolled SegmentedControls to pick one SegmentedControlButton that is selected on the initial render. */ + defaultSelected?: boolean } & SxProp & HTMLAttributes diff --git a/src/SegmentedControl/__snapshots__/SegmentedControl.test.tsx.snap b/src/SegmentedControl/__snapshots__/SegmentedControl.test.tsx.snap index 0dd5f099183..0b0e0a14523 100644 --- a/src/SegmentedControl/__snapshots__/SegmentedControl.test.tsx.snap +++ b/src/SegmentedControl/__snapshots__/SegmentedControl.test.tsx.snap @@ -343,6 +343,7 @@ exports[`SegmentedControl renders consistently 1`] = `