-
Notifications
You must be signed in to change notification settings - Fork 1.4k
feat: Add isExpanded renderProp to Button #9563
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
base: main
Are you sure you want to change the base?
Conversation
…he overlay is opened on mousedown
|
Build successful! 🎉 |
|
Build successful! 🎉 |
snowystinger
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we run chromatic against this?
|
Build successful! 🎉 |
|
https://www.chromatic.com/build?appId=5f0dd5ad2b5fc10022a2e320&number=1107, changes seem unrelated to my updates |
|
We should change lines like this (e.g. S2 ActionButton/Button) to use the isExpanded state instead: // Retain hover styles when an overlay is open.
isHovered: renderProps.isHovered || overlayTriggerState?.isOpen || false, |
|
Looks like the RAC starters should also be updated to use |
| } = props; | ||
| let stringFormatter = useLocalizedStringFormatter(intlMessages, '@react-spectrum/s2'); | ||
|
|
||
| // For mouse interactions, pickers open on press start. When the popover underlay appears |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this code is in S2 MenuTrigger too, so we should move that into RAC too I guess. The RAC Menu examples don't have any press scaling anymore.
This reverts commit f0d9772.
|
Build successful! 🎉 |
snowystinger
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can a ToggleButton be expanded? it's now in the types
|
@snowystinger oh good catch, I suppose not. I'll remove it for now and we can always readd it if people are using the toggle button for opening overlays |
starters/docs/src/Menu.css
Outdated
|
|
||
| &[data-open], | ||
| &[data-pressed] { | ||
| &[data-expanded] { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is right? This is changing the MenuItem styles, not the button. I think you need to change Button.css to add data-expanded that sets the background to match the pressed state.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh whoops, should've checked my bulk change here, I'll fix
starters/docs/src/Select.css
Outdated
| min-width: 0; | ||
|
|
||
| &[data-pressed] { | ||
| &[data-expanded] { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is right either. On touch the menu doesn't open until press up, so now it has press scaling when it didn't before.
You need to add a new selector for data-expanded that just adds the background color change so that persists while the popover is open.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah yep, accidentally conflated the press scaling with the background color styles
|
changes needed to tailwind starter as well |
|
Build successful! 🎉 |
## API Changes
react-aria-components/react-aria-components:ButtonRenderProps ButtonRenderProps {
isDisabled: boolean
+ isExpanded?: boolean
isFocusVisible: boolean
isFocused: boolean
isHovered: boolean
isPending: boolean
} |
As a alternative for
isTriggerUpWhenOpenfrom #8971, we've decided to addisExpandedalongsideisPressedso that users can decide style their ComboBox/DatePicker/Menu etc trigger button states to be independent of the open state.✅ Pull Request Checklist:
📝 Test Instructions:
Tests should cover this, but you can sanity check ComboBox/DatePicker/etc and make sure the RAC button has the proper data attribute for expanded. Additionally, double check the equivalent S2 components (and Picker/TabsPicker) and make sure the press state doesn't get stuck when opening the dropdowns/menus.
🧢 Your Project:
RSP