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

feat: Add ToggleButtonGroup component to RAC and S2 #7264

Merged
merged 15 commits into from
Nov 6, 2024

Conversation

devongovett
Copy link
Member

This adds a new ToggleButtonGroup component to React Aria Components, along with corresponding hooks and S2 components. Similar to CheckboxGroup and RadioGroup, this component coordinates the selection state between multiple ToggleButton children, supporting both single and multiple selection. It also implements arrow key navigation in both horizontal and vertical orientations.

ActionGroup in S2 is split into two components: ActionButtonGroup (which takes ActionButtons as children), and ToggleButtonGroup (which takes ToggleButtons as children). This also swaps SegmentedControl to using ToggleButtonGroup instead of RadioGroup, which enables tooltips to work properly and makes keyboard navigation not affect selection until the user presses Enter.

Some API questions are noted inline.

@rspbot
Copy link

rspbot commented Oct 25, 2024

@@ -24,6 +24,7 @@
"dependencies": {
"@react-aria/focus": "^3.18.4",
"@react-aria/interactions": "^3.22.4",
"@react-aria/toolbar": "3.0.0-beta.10",
Copy link
Member Author

Choose a reason for hiding this comment

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

Not super ideal that the button package now depends on toolbar but I didn't want to make a whole new package for this...

borderRadius: 'lg',
width: 'full'
borderRadius: 'default',
width: 'fit'
Copy link
Member Author

Choose a reason for hiding this comment

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

SegmentedControl now fits its contents by default rather than filling its parent

Copy link
Member

Choose a reason for hiding this comment

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

Did spectrum say anything about this in the meeting?

Copy link
Member Author

Choose a reason for hiding this comment

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

They were ok with the change to fit the content width by default. I added an isJustified prop to SementedControl to allow the previous behavior as an option, which also matches the same option in ActionButtonGroup and ToggleButtonGroup.

* Manages state for a group of toggles.
* It supports both single and multiple selected items.
*/
export function useToggleGroupState(props: ToggleGroupProps): ToggleGroupState {
Copy link
Member Author

Choose a reason for hiding this comment

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

This is called useToggleGroupState rather than useToggleButtonGroupState to make it non-button specific. Not sure what else we'd use it for yet though. Does this make sense?

Copy link
Member

Choose a reason for hiding this comment

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

makes sense to me

packages/@react-types/button/src/index.d.ts Outdated Show resolved Hide resolved
@rspbot
Copy link

rspbot commented Oct 25, 2024

@rspbot
Copy link

rspbot commented Oct 26, 2024

@rspbot
Copy link

rspbot commented Oct 28, 2024

Copy link
Member

@snowystinger snowystinger left a comment

Choose a reason for hiding this comment

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

Given the number of API hesitations we have, should we start with UNSTABLE prefixes?

I'll keep reviewing tomorrow

@rspbot
Copy link

rspbot commented Oct 28, 2024

@rspbot
Copy link

rspbot commented Nov 1, 2024

@rspbot
Copy link

rspbot commented Nov 1, 2024

LFDanLu
LFDanLu previously approved these changes Nov 1, 2024
Copy link
Member

@LFDanLu LFDanLu left a comment

Choose a reason for hiding this comment

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

Approving for testing, just some small questions and comments but otherwise the behavior looks good to me

packages/@react-aria/button/src/useToggleButtonGroup.ts Outdated Show resolved Hide resolved
packages/@react-stately/toggle/src/useToggleGroupState.ts Outdated Show resolved Hide resolved
* Manages state for a group of toggles.
* It supports both single and multiple selected items.
*/
export function useToggleGroupState(props: ToggleGroupProps): ToggleGroupState {
Copy link
Member

Choose a reason for hiding this comment

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

makes sense to me

snowystinger
snowystinger previously approved these changes Nov 5, 2024
@devongovett devongovett dismissed stale reviews from snowystinger and LFDanLu via e70f360 November 6, 2024 17:14
@rspbot
Copy link

rspbot commented Nov 6, 2024

@rspbot
Copy link

rspbot commented Nov 6, 2024

## API Changes

react-aria-components

/react-aria-components:ToggleButton

 ToggleButton {
   aria-controls?: string
   aria-describedby?: string
   aria-details?: string
   aria-expanded?: boolean | 'true' | 'false'
   aria-haspopup?: boolean | 'menu' | 'listbox' | 'tree' | 'grid' | 'dialog' | 'true' | 'false'
   aria-label?: string
   aria-labelledby?: string
   aria-pressed?: boolean | 'true' | 'false' | 'mixed'
   autoFocus?: boolean
   children?: ReactNode | ((ToggleButtonRenderProps & {
     defaultChildren: ReactNode | undefined
 })) => ReactNode
   className?: string | ((ToggleButtonRenderProps & {
     defaultClassName: string | undefined
 })) => string
   defaultSelected?: boolean
   excludeFromTabOrder?: boolean
-  id?: string
+  id?: Key
   isDisabled?: boolean
   isSelected?: boolean
   onBlur?: (FocusEvent<Target>) => void
   onChange?: (boolean) => void
   onFocusChange?: (boolean) => void
   onHoverChange?: (boolean) => void
   onHoverEnd?: (HoverEvent) => void
   onHoverStart?: (HoverEvent) => void
   onKeyDown?: (KeyboardEvent) => void
   onKeyUp?: (KeyboardEvent) => void
   onPress?: (PressEvent) => void
   onPressChange?: (boolean) => void
   onPressEnd?: (PressEvent) => void
   onPressStart?: (PressEvent) => void
   onPressUp?: (PressEvent) => void
   preventFocusOnPress?: boolean
   slot?: string | null
   style?: CSSProperties | ((ToggleButtonRenderProps & {
     defaultStyle: CSSProperties
 })) => CSSProperties | undefined
   type?: 'button' | 'submit' | 'reset' = 'button'
 }

/react-aria-components:ToggleButtonProps

 ToggleButtonProps {
   aria-controls?: string
   aria-describedby?: string
   aria-details?: string
   aria-expanded?: boolean | 'true' | 'false'
   aria-haspopup?: boolean | 'menu' | 'listbox' | 'tree' | 'grid' | 'dialog' | 'true' | 'false'
   aria-label?: string
   aria-labelledby?: string
   aria-pressed?: boolean | 'true' | 'false' | 'mixed'
   autoFocus?: boolean
   children?: ReactNode | ((ToggleButtonRenderProps & {
     defaultChildren: ReactNode | undefined
 })) => ReactNode
   className?: string | ((ToggleButtonRenderProps & {
     defaultClassName: string | undefined
 })) => string
   defaultSelected?: boolean
   excludeFromTabOrder?: boolean
-  id?: string
+  id?: Key
   isDisabled?: boolean
   isSelected?: boolean
   onBlur?: (FocusEvent<Target>) => void
   onChange?: (boolean) => void
   onFocusChange?: (boolean) => void
   onHoverChange?: (boolean) => void
   onHoverEnd?: (HoverEvent) => void
   onHoverStart?: (HoverEvent) => void
   onKeyDown?: (KeyboardEvent) => void
   onKeyUp?: (KeyboardEvent) => void
   onPress?: (PressEvent) => void
   onPressChange?: (boolean) => void
   onPressEnd?: (PressEvent) => void
   onPressStart?: (PressEvent) => void
   onPressUp?: (PressEvent) => void
   preventFocusOnPress?: boolean
   slot?: string | null
   style?: CSSProperties | ((ToggleButtonRenderProps & {
     defaultStyle: CSSProperties
 })) => CSSProperties | undefined
   type?: 'button' | 'submit' | 'reset' = 'button'
 }

/react-aria-components:ToggleButtonGroup

+ToggleButtonGroup {
+  aria-describedby?: string
+  aria-details?: string
+  aria-label?: string
+  aria-labelledby?: string
+  children?: ReactNode | ((ToggleButtonGroupRenderProps & {
+    defaultChildren: ReactNode | undefined
+})) => ReactNode
+  className?: string | ((ToggleButtonGroupRenderProps & {
+    defaultClassName: string | undefined
+})) => string
+  defaultSelectedKeys?: Iterable<Key>
+  disallowEmptySelection?: boolean
+  isDisabled?: boolean
+  onSelectionChange?: (Set<Key>) => void
+  orientation?: Orientation = 'horizontal'
+  selectedKeys?: Iterable<Key>
+  selectionMode?: 'single' | 'multiple'
+  slot?: string | null
+  style?: CSSProperties | ((ToggleButtonGroupRenderProps & {
+    defaultStyle: CSSProperties
+})) => CSSProperties | undefined
+}

/react-aria-components:ToggleButtonGroupContext

+ToggleButtonGroupContext {
+  UNTYPED
+}

/react-aria-components:ToggleGroupStateContext

+ToggleGroupStateContext {
+  UNTYPED
+}

/react-aria-components:ToggleButtonGroupProps

+ToggleButtonGroupProps {
+  aria-describedby?: string
+  aria-details?: string
+  aria-label?: string
+  aria-labelledby?: string
+  children?: ReactNode | ((ToggleButtonGroupRenderProps & {
+    defaultChildren: ReactNode | undefined
+})) => ReactNode
+  className?: string | ((ToggleButtonGroupRenderProps & {
+    defaultClassName: string | undefined
+})) => string
+  defaultSelectedKeys?: Iterable<Key>
+  disallowEmptySelection?: boolean
+  isDisabled?: boolean
+  onSelectionChange?: (Set<Key>) => void
+  orientation?: Orientation = 'horizontal'
+  selectedKeys?: Iterable<Key>
+  selectionMode?: 'single' | 'multiple'
+  slot?: string | null
+  style?: CSSProperties | ((ToggleButtonGroupRenderProps & {
+    defaultStyle: CSSProperties
+})) => CSSProperties | undefined
+}

/react-aria-components:ToggleButtonGroupRenderProps

+ToggleButtonGroupRenderProps {
+  isDisabled: boolean
+  state: ToggleGroupState
+}

@react-aria/button

/@react-aria/button:useToggleButtonGroup

+useToggleButtonGroup {
+  props: AriaToggleButtonGroupProps
+  state: ToggleGroupState
+  ref: RefObject<HTMLElement | null>
+  returnVal: undefined
+}

/@react-aria/button:useToggleButtonGroupItem

+useToggleButtonGroupItem {
+  props: AriaToggleButtonGroupItemOptions<ElementType>
+  state: ToggleGroupState
+  ref: RefObject<any>
+  returnVal: undefined
+}

/@react-aria/button:AriaToggleButtonGroupProps

+AriaToggleButtonGroupProps {
+  aria-describedby?: string
+  aria-details?: string
+  aria-label?: string
+  aria-labelledby?: string
+  defaultSelectedKeys?: Iterable<Key>
+  disallowEmptySelection?: boolean
+  isDisabled?: boolean
+  onSelectionChange?: (Set<Key>) => void
+  orientation?: Orientation = 'horizontal'
+  selectedKeys?: Iterable<Key>
+  selectionMode?: 'single' | 'multiple'
+}

/@react-aria/button:ToggleButtonGroupAria

+ToggleButtonGroupAria {
+  groupProps: DOMAttributes
+}

/@react-aria/button:AriaToggleButtonGroupItemProps

+AriaToggleButtonGroupItemProps <E extends ElementType = 'button'> {
+  aria-controls?: string
+  aria-describedby?: string
+  aria-details?: string
+  aria-expanded?: boolean | 'true' | 'false'
+  aria-haspopup?: boolean | 'menu' | 'listbox' | 'tree' | 'grid' | 'dialog' | 'true' | 'false'
+  aria-label?: string
+  aria-labelledby?: string
+  aria-pressed?: boolean | 'true' | 'false' | 'mixed'
+  autoFocus?: boolean
+  children?: ReactNode
+  elementType?: ElementType | JSXElementConstructor<any> = 'button'
+  excludeFromTabOrder?: boolean
+  id: Key
+  isDisabled?: boolean
+  onBlur?: (FocusEvent<Target>) => void
+  onFocus?: (FocusEvent<Target>) => void
+  onFocusChange?: (boolean) => void
+  onKeyDown?: (KeyboardEvent) => void
+  onKeyUp?: (KeyboardEvent) => void
+  onPress?: (PressEvent) => void
+  onPressChange?: (boolean) => void
+  onPressEnd?: (PressEvent) => void
+  onPressStart?: (PressEvent) => void
+  onPressUp?: (PressEvent) => void
+  preventFocusOnPress?: boolean
+  type?: 'button' | 'submit' | 'reset' = 'button'
+}

@react-aria/toolbar

/@react-aria/toolbar:useToolbar

 useToolbar {
   props: AriaToolbarProps
-  ref: RefObject<HTMLDivElement | null>
+  ref: RefObject<HTMLElement | null>
   returnVal: undefined
 }

@react-spectrum/s2

/@react-spectrum/s2:SegmentedControl

 SegmentedControl {
   UNSAFE_className?: string
   UNSAFE_style?: CSSProperties
   aria-describedby?: string
   aria-details?: string
   aria-label?: string
   aria-labelledby?: string
   children: ReactNode
-  defaultSelectedKey?: string
+  defaultSelectedKey?: Key
   isDisabled?: boolean
-  onSelectionChange?: (string) => void
-  selectedKey?: string | null
+  isJustified?: boolean
+  onSelectionChange?: (Key) => void
+  selectedKey?: Key | null
   slot?: string | null
   styles?: StylesProp
 }

/@react-spectrum/s2:SegmentedControlItem

 SegmentedControlItem {
   UNSAFE_className?: string
   UNSAFE_style?: CSSProperties
   aria-describedby?: string
   aria-details?: string
   aria-label?: string
   aria-labelledby?: string
   children: ReactNode
-  id: string
+  id: Key
   isDisabled?: boolean
   styles?: StylesProp
 }

/@react-spectrum/s2:ToggleButton

 ToggleButton {
   UNSAFE_className?: string
   UNSAFE_style?: CSSProperties
   aria-controls?: string
   aria-describedby?: string
   aria-details?: string
   aria-expanded?: boolean | 'true' | 'false'
   aria-haspopup?: boolean | 'menu' | 'listbox' | 'tree' | 'grid' | 'dialog' | 'true' | 'false'
   aria-label?: string
   aria-labelledby?: string
   aria-pressed?: boolean | 'true' | 'false' | 'mixed'
   autoFocus?: boolean
   children?: ReactNode
   defaultSelected?: boolean
   excludeFromTabOrder?: boolean
-  id?: string
+  id?: Key
   isDisabled?: boolean
   isEmphasized?: boolean
   isQuiet?: boolean
   isSelected?: boolean
   onChange?: (boolean) => void
   onFocus?: (FocusEvent<Target>) => void
   onFocusChange?: (boolean) => void
   onKeyDown?: (KeyboardEvent) => void
   onKeyUp?: (KeyboardEvent) => void
   onPress?: (PressEvent) => void
   onPressChange?: (boolean) => void
   onPressEnd?: (PressEvent) => void
   onPressStart?: (PressEvent) => void
   onPressUp?: (PressEvent) => void
   preventFocusOnPress?: boolean
   size?: 'XS' | 'S' | 'M' | 'L' | 'XL' = 'M'
   slot?: string | null
   staticColor?: 'black' | 'white'
   styles?: StylesProp
   type?: 'button' | 'submit' | 'reset' = 'button'
 }

/@react-spectrum/s2:SegmentedControlProps

 SegmentedControlProps {
   UNSAFE_className?: string
   UNSAFE_style?: CSSProperties
   aria-describedby?: string
   aria-details?: string
   aria-label?: string
   aria-labelledby?: string
   children: ReactNode
-  defaultSelectedKey?: string
+  defaultSelectedKey?: Key
   isDisabled?: boolean
-  onSelectionChange?: (string) => void
-  selectedKey?: string | null
+  isJustified?: boolean
+  onSelectionChange?: (Key) => void
+  selectedKey?: Key | null
   slot?: string | null
   styles?: StylesProp
 }

/@react-spectrum/s2:SegmentedControlItemProps

 SegmentedControlItemProps {
   UNSAFE_className?: string
   UNSAFE_style?: CSSProperties
   aria-describedby?: string
   aria-details?: string
   aria-label?: string
   aria-labelledby?: string
   children: ReactNode
-  id: string
+  id: Key
   isDisabled?: boolean
   styles?: StylesProp
 }

/@react-spectrum/s2:ToggleButtonProps

 ToggleButtonProps {
   UNSAFE_className?: string
   UNSAFE_style?: CSSProperties
   aria-controls?: string
   aria-describedby?: string
   aria-details?: string
   aria-expanded?: boolean | 'true' | 'false'
   aria-haspopup?: boolean | 'menu' | 'listbox' | 'tree' | 'grid' | 'dialog' | 'true' | 'false'
   aria-label?: string
   aria-labelledby?: string
   aria-pressed?: boolean | 'true' | 'false' | 'mixed'
   autoFocus?: boolean
   children?: ReactNode
   defaultSelected?: boolean
   excludeFromTabOrder?: boolean
-  id?: string
+  id?: Key
   isDisabled?: boolean
   isEmphasized?: boolean
   isQuiet?: boolean
   isSelected?: boolean
   onChange?: (boolean) => void
   onFocus?: (FocusEvent<Target>) => void
   onFocusChange?: (boolean) => void
   onKeyDown?: (KeyboardEvent) => void
   onKeyUp?: (KeyboardEvent) => void
   onPress?: (PressEvent) => void
   onPressChange?: (boolean) => void
   onPressEnd?: (PressEvent) => void
   onPressStart?: (PressEvent) => void
   onPressUp?: (PressEvent) => void
   preventFocusOnPress?: boolean
   size?: 'XS' | 'S' | 'M' | 'L' | 'XL' = 'M'
   slot?: string | null
   staticColor?: 'black' | 'white'
   styles?: StylesProp
   type?: 'button' | 'submit' | 'reset' = 'button'
 }

/@react-spectrum/s2:ActionButtonGroup

+ActionButtonGroup {
+  UNSAFE_className?: string
+  UNSAFE_style?: CSSProperties
+  children: ReactNode
+  density?: 'compact' | 'regular' = "regular"
+  isDisabled?: boolean
+  isJustified?: boolean
+  isQuiet?: boolean
+  orientation?: 'horizontal' | 'vertical' = 'horizontal'
+  size?: 'XS' | 'S' | 'M' | 'L' | 'XL' = "M"
+  slot?: string | null
+  staticColor?: 'white' | 'black'
+  styles?: StylesPropWithHeight
+}

/@react-spectrum/s2:ActionButtonGroupContext

+ActionButtonGroupContext {
+  UNTYPED
+}

/@react-spectrum/s2:ToggleButtonGroup

+ToggleButtonGroup {
+  UNSAFE_className?: string
+  UNSAFE_style?: CSSProperties
+  aria-describedby?: string
+  aria-details?: string
+  aria-label?: string
+  aria-labelledby?: string
+  children: ReactNode
+  defaultSelectedKeys?: Iterable<Key>
+  density?: 'compact' | 'regular' = "regular"
+  disallowEmptySelection?: boolean
+  isDisabled?: boolean
+  isEmphasized?: boolean
+  isJustified?: boolean
+  isQuiet?: boolean
+  onSelectionChange?: (Set<Key>) => void
+  orientation?: 'horizontal' | 'vertical' = 'horizontal'
+  selectedKeys?: Iterable<Key>
+  selectionMode?: 'single' | 'multiple'
+  size?: 'XS' | 'S' | 'M' | 'L' | 'XL' = "M"
+  slot?: string | null
+  staticColor?: 'white' | 'black'
+  styles?: StylesPropWithHeight
+}

/@react-spectrum/s2:ToggleButtonGroupContext

+ToggleButtonGroupContext {
+  UNTYPED
+}

/@react-spectrum/s2:ActionButtonGroupProps

+ActionButtonGroupProps {
+  UNSAFE_className?: string
+  UNSAFE_style?: CSSProperties
+  children: ReactNode
+  density?: 'compact' | 'regular' = "regular"
+  isDisabled?: boolean
+  isJustified?: boolean
+  isQuiet?: boolean
+  orientation?: 'horizontal' | 'vertical' = 'horizontal'
+  size?: 'XS' | 'S' | 'M' | 'L' | 'XL' = "M"
+  slot?: string | null
+  staticColor?: 'white' | 'black'
+  styles?: StylesPropWithHeight
+}

/@react-spectrum/s2:ToggleButtonGroupProps

+ToggleButtonGroupProps {
+  UNSAFE_className?: string
+  UNSAFE_style?: CSSProperties
+  aria-describedby?: string
+  aria-details?: string
+  aria-label?: string
+  aria-labelledby?: string
+  children: ReactNode
+  defaultSelectedKeys?: Iterable<Key>
+  density?: 'compact' | 'regular' = "regular"
+  disallowEmptySelection?: boolean
+  isDisabled?: boolean
+  isEmphasized?: boolean
+  isJustified?: boolean
+  isQuiet?: boolean
+  onSelectionChange?: (Set<Key>) => void
+  orientation?: 'horizontal' | 'vertical' = 'horizontal'
+  selectedKeys?: Iterable<Key>
+  selectionMode?: 'single' | 'multiple'
+  size?: 'XS' | 'S' | 'M' | 'L' | 'XL' = "M"
+  slot?: string | null
+  staticColor?: 'white' | 'black'
+  styles?: StylesPropWithHeight
+}

@react-stately/toggle

/@react-stately/toggle:useToggleGroupState

+useToggleGroupState {
+  props: ToggleGroupProps
+  returnVal: undefined
+}

/@react-stately/toggle:ToggleGroupProps

+ToggleGroupProps {
+  defaultSelectedKeys?: Iterable<Key>
+  disallowEmptySelection?: boolean
+  isDisabled?: boolean
+  onSelectionChange?: (Set<Key>) => void
+  selectedKeys?: Iterable<Key>
+  selectionMode?: 'single' | 'multiple'
+}

/@react-stately/toggle:ToggleGroupState

+ToggleGroupState {
+  isDisabled: boolean
+  selectedKeys: Set<Key>
+  selectionMode: 'single' | 'multiple'
+  setSelected: (Key, boolean) => void
+  setSelectedKeys: (Set<Key>) => void
+  toggleKey: (Key) => void
+}

Copy link
Member

@reidbarber reidbarber left a comment

Choose a reason for hiding this comment

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

LGTM

@devongovett devongovett added this pull request to the merge queue Nov 6, 2024
Merged via the queue into main with commit c703323 Nov 6, 2024
30 checks passed
@devongovett devongovett deleted the toggle-button-group branch November 6, 2024 18:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants