Skip to content

Conversation

LFDanLu
Copy link
Member

@LFDanLu LFDanLu commented Oct 1, 2025

Closes

✅ 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:

🧢 Your Project:

RSP

By default, `Modal` includes a builtin `ModalOverlay`, which renders a backdrop over the page when a modal is open. This can be targeted using the `.react-aria-ModalOverlay` CSS selector. To customize the `ModalOverlay` with a different class name or other attributes, render a `ModalOverlay` and place a `Modal` inside.

The `--visual-viewport-height` CSS custom property will be set on the `ModalOverlay`, which you can use to set the height to account for the virtual keyboard on mobile.
The `--visual-viewport-height` and `--page-height` CSS custom property will be set on the `ModalOverlay`, which you can use to set the height to account for the virtual keyboard on mobile.
Copy link
Member Author

Choose a reason for hiding this comment

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

How much do we want to explain here? I debated whether or not to mention iOS 26 specifically (and thus explain the difference between the two properties here).

Copy link
Member

Choose a reason for hiding this comment

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

I think it would be useful to at least the explain the differences since it might not be immediately obvious

Copy link
Member Author

Choose a reason for hiding this comment

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

kk clarified the wording in the sentence and added a comment to the css example, hopefully that keeps it concise enough but distinguishes the two properties

@LFDanLu LFDanLu changed the title chore: Update docs for new --page-height var and small audit fixes chore: Update docs for new --page-height var and ComboBox actions Oct 1, 2025
@LFDanLu LFDanLu added the release label Oct 1, 2025
@rspbot
Copy link

rspbot commented Oct 1, 2025

@rspbot
Copy link

rspbot commented Oct 1, 2025

}
```

## Row actions
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
## Row actions
## Item actions

## Row actions

By default, interacting with an item in a ComboBox selects it and updates the input value. Alternatively, items can also trigger a custom action instead via providing a `onAction` prop to the ListBoxItem.
As shown in the example below, a "Create" action is added for the current input value. Note that interacting with said item does not update selection or the input value.
Copy link
Member

Choose a reason for hiding this comment

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

Can we copy the text I wrote for the new docs? This is a bit more verbose.

@LFDanLu LFDanLu changed the title chore: Update docs for new --page-height var and ComboBox actions docs: Update docs for new --page-height var and ComboBox actions Oct 1, 2025
@rspbot
Copy link

rspbot commented Oct 1, 2025


## Item actions

Use the `onAction` prop on a `<ListBoxItem>` to perform a custom action when the item is selected. This example adds a "Create" action for the current input value.
Copy link
Member

Choose a reason for hiding this comment

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

when the item is selected

isn't this a little misnomer? if it has an action, it doesn't get selected?

Copy link
Member Author

Choose a reason for hiding this comment

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

"activated" maybe? "triggered"?

Copy link
Member

Choose a reason for hiding this comment

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

or just "pressed"?

Copy link
Member

Choose a reason for hiding this comment

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

pressed is fine. you can update the new docs too if you want. fyi the same thing is in S2 and RAC docs.

@rspbot
Copy link

rspbot commented Oct 1, 2025

yihuiliao
yihuiliao previously approved these changes Oct 1, 2025
@yihuiliao yihuiliao mentioned this pull request Oct 1, 2025
5 tasks
export {Heading} from './Heading';
export {Input, InputContext} from './Input';
export {Section, CollectionRendererContext, DefaultCollectionRenderer} from './Collection';
export {Collection, createLeafComponent as UNSTABLE_createLeafComponent, createBranchComponent as UNSTABLE_createBranchComponent, CollectionBuilder as UNSTABLE_CollectionBuilder} from '@react-aria/collections';
Copy link
Member

Choose a reason for hiding this comment

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

glad we don't use the UNSTABLE anywhere in our own code, but this could be problematic for libraries that are built on top of us
in order for this to stay non-breaking for semver, we need to keep the UNSTABLE as well :(

Copy link
Member

Choose a reason for hiding this comment

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

if you put the unstables on a separate line, maybe we can put a @deprecated comment before just them

Copy link
Member Author

Choose a reason for hiding this comment

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

urgh, that sucks. Doesn't look like the deprecated comment actually comes through when importing so I've omitted it for now

Copy link
Member

Choose a reason for hiding this comment

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

this one is not a cross-package dependency so it's ok to remove. If other teams are using it then they should be pinning their version of RAC. The original problem was that we were not pinning between our own dependencies.

@rspbot
Copy link

rspbot commented Oct 1, 2025

@rspbot
Copy link

rspbot commented Oct 1, 2025

## API Changes

react-aria-components

/react-aria-components:UNSTABLE_createLeafComponent

-UNSTABLE_createLeafComponent <E extends Element, P extends {}> {
-  CollectionNodeClass: {}<any> | string
-  render: (P, ForwardedRef<E>, any) => ReactElement | null
-  returnVal: undefined
-}

/react-aria-components:UNSTABLE_createBranchComponent

-UNSTABLE_createBranchComponent <E extends Element, P extends {
-    children?: any
-}, T extends {}> {
-  CollectionNodeClass: {}<any> | string
-  render: (P, ForwardedRef<E>, Node<T>) => ReactElement | null
-  useChildren: (P) => ReactNode
-  returnVal: undefined
-}

/react-aria-components:UNSTABLE_CollectionBuilder

-UNSTABLE_CollectionBuilder <C extends BaseCollection<{}>> {
-  children: (BaseCollection<{}>) => ReactNode
-  content: ReactNode
-  createCollection?: () => BaseCollection<{}>
-}

/react-aria-components:createLeafComponent

+createLeafComponent <E extends Element, P extends {}> {
+  CollectionNodeClass: {}<any> | string
+  render: (P, ForwardedRef<E>, any) => ReactElement | null
+  returnVal: undefined
+}

/react-aria-components:createBranchComponent

+createBranchComponent <E extends Element, P extends {
+    children?: any
+}, T extends {}> {
+  CollectionNodeClass: {}<any> | string
+  render: (P, ForwardedRef<E>, Node<T>) => ReactElement | null
+  useChildren: (P) => ReactNode
+  returnVal: undefined
+}

/react-aria-components:CollectionBuilder

+CollectionBuilder <C extends BaseCollection<{}>> {
+  children: (BaseCollection<{}>) => ReactNode
+  content: ReactNode
+  createCollection?: () => BaseCollection<{}>
+}

@LFDanLu LFDanLu added this pull request to the merge queue Oct 1, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 1, 2025
@LFDanLu LFDanLu added this pull request to the merge queue Oct 1, 2025
@LFDanLu
Copy link
Member Author

LFDanLu commented Oct 1, 2025

ugh react 17 test randomly failed

Merged via the queue into main with commit 2ba829b Oct 1, 2025
32 checks passed
@LFDanLu LFDanLu deleted the audit_fixes branch October 1, 2025 23:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants