fix: S2 Selectbox Updates #8748

DPandyan · 2025-08-20T16:42:35Z

Closes

things to look out for that we discussed when we tested selectbox:

selectbox should have cursor: 'default'. right now it has a pointer cursor
the focus ring should be a halo since it conflicts with the selection state. i think all you need to do is remove the outlineOffset
we're just going to omit the showCheckbox and have checkboxes always rendered. we can change this later
when the user presses down on a select box, the select box should scale down (which is what we call press scaling). search pressScale and example of how to use it in S2 should come up. we want it to look similar to how CardView does when you press down on a Card
clean up the stories a little bit since some of the various states can be achieved by using the controls (like the Disabled one for example). some of stories might actually be better in chromatic where we take snapshots of the components. lemme know if you need help with getting chromatic to work but that can be follow-up to this new PR if it's easier to separate out
spread props onto ListBoxItem/ListBox
delete gutterWidth/numColumns

✅ Pull Request Checklist:

Included link to corresponding React Spectrum GitHub Issue.
Added/updated unit tests and storybook for this change (for new code or code which already has tests).
Filled out test instructions.
Updated documentation (if it already exists for this component).
Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

Run the SelectBoxGroup.test.tsx file and observe the SelectBox s2 component with yarn start:s2

🧢 Your Project:

DPandyan · 2025-08-20T16:43:08Z

I still need to take a closer look at the design tokens and the tests but many of the issues have been addressed.

rspbot · 2025-08-20T16:48:45Z

Build successful! 🎉

rspbot · 2025-08-20T17:24:31Z

Build successful! 🎉

packages/@react-spectrum/s2/stories/SelectBoxGroup.stories.tsx

snowystinger · 2025-08-21T00:42:49Z

packages/@react-spectrum/s2/stories/SelectBoxGroup.stories.tsx

+  render: (args) => {
+    const {selectionMode, orientation, isDisabled} = args as any;
+    return (
+      <SelectBoxGroup selectionMode={selectionMode} orientation={orientation} isDisabled={isDisabled} UNSAFE_style={{gridTemplateColumns: 'repeat(2, 1fr)'}}>


you can just spread args onto here once you have the types fixed

we shouldn't use UNSAFE_style in our documentation, I'd just get rid of it, and if you need to constrain the area so it doesn't go across the entire screen, just add a wrapping div with an appropriate width. Check other storybooks for examples

snowystinger · 2025-08-21T00:43:16Z

packages/@react-spectrum/s2/stories/SelectBoxGroup.stories.tsx

-        {/* Enabled items with dynamic illustrations */}
+        }}
+        {...args}
+        UNSAFE_style={{gridTemplateColumns: 'repeat(4, 1fr)'}}>


snowystinger · 2025-08-21T00:43:43Z

packages/@react-spectrum/s2/stories/SelectBoxGroup.stories.tsx

@@ -313,7 +220,7 @@ function InteractiveExamplesStory() {
 }

 export const InteractiveExamples: Story = {
-  render: () => <InteractiveExamplesStory />
+  render: (args) => <InteractiveExamplesStory {...args} />
 };

 export const AllSlotCombinations: Story = {


this is a better story for chromatic

snowystinger · 2025-08-21T00:44:24Z

packages/@react-spectrum/s2/test/SelectBoxGroup.test.tsx

+    jest.spyOn(window.HTMLElement.prototype, 'clientWidth', 'get').mockImplementation(() => 100);
+    jest.spyOn(window.HTMLElement.prototype, 'clientHeight', 'get').mockImplementation(() => 100);
+    jest.spyOn(window.HTMLElement.prototype, 'scrollHeight', 'get').mockImplementation(() => 50);


why do we need to mock all these? this is usually for virtualised components, which this isn't

snowystinger · 2025-08-21T00:46:58Z

packages/@react-spectrum/s2/test/SelectBoxGroup.test.tsx

      const options = screen.getAllByRole('option');
      const option1 = options.find(option => option.textContent?.includes('Option 1'))!;

-      await userEvent.click(option1);
+      await user.click(option1);


We can probably make use of the test-util ListBoxTester in order to do interactions and avoid things like

const options = screen.getAllByRole('option'); const option1 = options.find(option => option.textContent?.includes('Option 1'))!;

https://react-spectrum.adobe.com/react-aria/ListBox.html#testing

These three lines would be something like

toggleOptionSelection({option: 1})

Then it's easier to write tests which are agnostic of keyboard or mouse, so you can run both on the same series of tests

reidbarber

Looks great!

packages/@react-spectrum/s2/src/SelectBoxGroup.tsx

reidbarber · 2025-08-21T15:06:54Z

packages/@react-spectrum/s2/src/SelectBoxGroup.tsx

-        gridTemplateColumns: `repeat(${numColumns}, 1fr)`,
-        ...UNSAFE_style
-      }}>
+      {...otherProps}


selectionBehavior="replace" doesn't seem to be working as expected. Probably not something we need to address right now. It seems like something that wouldn't be used with this component anyway.

Also shouldSelectOnPressUp doesn't seem to work as expected. I think this is one we could omit from the types, I don't think it makes sense to have here. A few more aren't working as well, like shouldFocusWrap. Definitely not something we need now though, I would be in favor of omitting them.

Just confirmed with the team that we can omit these props for now.

…styling changes.

rspbot · 2025-08-21T20:02:01Z

Build successful! 🎉

rspbot · 2025-08-21T20:02:03Z

## API Changes

@react-spectrum/s2

/@react-spectrum/s2:SelectBoxGroup

 SelectBoxGroup <T> {
   UNSAFE_className?: UnsafeClassName
   UNSAFE_style?: CSSProperties
   aria-describedby?: string
   aria-details?: string
   aria-label?: string
   aria-labelledby?: string
   autoFocus?: boolean | FocusStrategy
   children: ReactNode
   className?: ClassNameOrFunction<ListBoxRenderProps>
-  defaultSelectedKeys?: Selection
+  defaultSelectedKeys?: 'all' | Iterable<Key>
+  dependencies?: ReadonlyArray<any>
   disabledKeys?: Iterable<Key>
   disallowEmptySelection?: boolean
+  dragAndDropHooks?: DragAndDropHooks<NoInfer<T>>
   escapeKeyBehavior?: 'clearSelection' | 'none' = 'clearSelection'
-  gutterWidth?: 'default' | 'compact' | 'spacious' = 'default'
   id?: string
   isDisabled?: boolean
-  numColumns?: number = 2
+  items?: Iterable<T>
+  layout?: 'stack' | 'grid' = 'stack'
   onAction?: (Key) => void
   onBlur?: (FocusEvent<Target>) => void
   onFocus?: (FocusEvent<Target>) => void
   onFocusChange?: (boolean) => void
   onSelectionChange?: (Selection) => void
   orientation?: Orientation = 'vertical'
-  selectedKeys?: Selection
+  renderEmptyState?: (ListBoxRenderProps) => ReactNode
+  selectedKeys?: 'all' | Iterable<Key>
   selectionBehavior?: SelectionBehavior = "toggle"
   selectionMode?: 'single' | 'multiple' = 'single'
   shouldFocusOnHover?: boolean
   shouldFocusWrap?: boolean
   shouldSelectOnPressUp?: boolean
-  showCheckbox?: boolean = false
   slot?: string | null
   style?: StyleOrFunction<ListBoxRenderProps>
   styles?: StylesProp
 }

/@react-spectrum/s2:SelectBoxGroupProps

 SelectBoxGroupProps <T> {
   UNSAFE_className?: UnsafeClassName
   UNSAFE_style?: CSSProperties
   aria-describedby?: string
   aria-details?: string
   aria-label?: string
   aria-labelledby?: string
   autoFocus?: boolean | FocusStrategy
   children: ReactNode
   className?: ClassNameOrFunction<ListBoxRenderProps>
-  defaultSelectedKeys?: Selection
+  defaultSelectedKeys?: 'all' | Iterable<Key>
+  dependencies?: ReadonlyArray<any>
   disabledKeys?: Iterable<Key>
   disallowEmptySelection?: boolean
+  dragAndDropHooks?: DragAndDropHooks<NoInfer<T>>
   escapeKeyBehavior?: 'clearSelection' | 'none' = 'clearSelection'
-  gutterWidth?: 'default' | 'compact' | 'spacious' = 'default'
   id?: string
   isDisabled?: boolean
-  numColumns?: number = 2
+  items?: Iterable<T>
+  layout?: 'stack' | 'grid' = 'stack'
   onAction?: (Key) => void
   onBlur?: (FocusEvent<Target>) => void
   onFocus?: (FocusEvent<Target>) => void
   onFocusChange?: (boolean) => void
   onSelectionChange?: (Selection) => void
   orientation?: Orientation = 'vertical'
-  selectedKeys?: Selection
+  renderEmptyState?: (ListBoxRenderProps) => ReactNode
+  selectedKeys?: 'all' | Iterable<Key>
   selectionBehavior?: SelectionBehavior = "toggle"
   selectionMode?: 'single' | 'multiple' = 'single'
   shouldFocusOnHover?: boolean
   shouldFocusWrap?: boolean
   shouldSelectOnPressUp?: boolean
-  showCheckbox?: boolean = false
   slot?: string | null
   style?: StyleOrFunction<ListBoxRenderProps>
   styles?: StylesProp
 }

reidbarber

LGTM

s2 selectbox changes

80ecb57

DPandyan changed the title ~~S2 Selectbox Updates~~ WIP: S2 Selectbox Updates Aug 20, 2025

Merge branch 'main' into s2-selectbox-updates

0812adc

yihuiliao changed the title ~~WIP: S2 Selectbox Updates~~ fix: S2 Selectbox Updates Aug 20, 2025

yihuiliao added the release label Aug 20, 2025

snowystinger reviewed Aug 21, 2025

View reviewed changes

This comment was marked as spam.

Sign in to view

reidbarber reviewed Aug 21, 2025

View reviewed changes

addressed gh comments, removed interactivestory for chromatic, small …

c949a10

…styling changes.

reidbarber approved these changes Aug 21, 2025

View reviewed changes

devongovett approved these changes Aug 21, 2025

View reviewed changes

snowystinger approved these changes Aug 21, 2025

View reviewed changes

LFDanLu added this pull request to the merge queue Aug 21, 2025

Merged via the queue into main with commit 6b56454 Aug 21, 2025
32 checks passed

LFDanLu deleted the s2-selectbox-updates branch August 21, 2025 20:30

fix: S2 Selectbox Updates #8748

fix: S2 Selectbox Updates #8748

Conversation

DPandyan commented Aug 20, 2025 • edited by yihuiliao Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Pull Request Checklist:

📝 Test Instructions:

🧢 Your Project:

Uh oh!

DPandyan commented Aug 20, 2025

Uh oh!

rspbot commented Aug 20, 2025

Uh oh!

rspbot commented Aug 20, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as spam.

Uh oh!

reidbarber left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rspbot commented Aug 21, 2025

Uh oh!

rspbot commented Aug 21, 2025

@react-spectrum/s2

/@react-spectrum/s2:SelectBoxGroup

/@react-spectrum/s2:SelectBoxGroupProps

Uh oh!

reidbarber left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

DPandyan commented Aug 20, 2025 •

edited by yihuiliao

Loading