fix: fix isPressed state to be separate from overlay state#9563
fix: fix isPressed state to be separate from overlay state#9563
Conversation
…he overlay is opened on mousedown
|
Build successful! 🎉 |
|
Build successful! 🎉 |
snowystinger
left a comment
snowystinger
left a comment
There was a problem hiding this comment.
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.
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
snowystinger
left a comment
There was a problem hiding this comment.
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.
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.
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.
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.
ah yep, accidentally conflated the press scaling with the background color styles
|
changes needed to tailwind starter as well |
|
Build successful! 🎉 |
LFDanLu
changed the title
LFDanLu
changed the title
|
Build successful! 🎉 |
## API Changes
react-aria-components/react-aria-components:ButtonRenderProps ButtonRenderProps {
isDisabled: boolean
+ isExpanded?: boolean
isFocusVisible: boolean
isFocused: boolean
isHovered: boolean
isPending: boolean
}/react-aria-components:HeaderProps-HeaderProps {
- render?: DOMRenderFunction<keyof React.JSX.IntrinsicElements, undefined>
-}/react-aria-components:NumberFieldState NumberFieldState {
canDecrement: boolean
canIncrement: boolean
- commit: (string) => void
+ commit: () => void
commitValidation: () => void
decrement: () => void
decrementToMin: () => void
defaultNumberValue: number
increment: () => void
incrementToMax: () => void
inputValue: string
maxValue?: number
minValue?: number
numberValue: number
realtimeValidation: ValidationResult
resetValidation: () => void
setInputValue: (string) => void
setNumberValue: (number) => void
updateValidation: (ValidationResult) => void
validate: (string) => boolean
}@react-stately/color/@react-stately/color:ColorChannelFieldState ColorChannelFieldState {
canDecrement: boolean
canIncrement: boolean
colorValue: Color
- commit: (string) => void
+ commit: () => void
commitValidation: () => void
decrement: () => void
decrementToMin: () => void
defaultColorValue: Color | null
displayValidation: ValidationResult
increment: () => void
incrementToMax: () => void
inputValue: string
maxValue?: number
minValue?: number
numberValue: number
realtimeValidation: ValidationResult
resetValidation: () => void
setColorValue: (Color | null) => void
setInputValue: (string) => void
setNumberValue: (number) => void
updateValidation: (ValidationResult) => void
validate: (string) => boolean
}@react-stately/numberfield/@react-stately/numberfield:NumberFieldState NumberFieldState {
canDecrement: boolean
canIncrement: boolean
- commit: (string) => void
+ commit: () => void
commitValidation: () => void
decrement: () => void
decrementToMin: () => void
defaultNumberValue: number
increment: () => void
incrementToMax: () => void
inputValue: string
maxValue?: number
minValue?: number
numberValue: number
realtimeValidation: ValidationResult
resetValidation: () => void
setInputValue: (string) => void
setNumberValue: (number) => void
updateValidation: (ValidationResult) => void
validate: (string) => boolean
} |
starters/docs/src/Form.css
Outdated
|
|
||
| &[data-pressed] { | ||
| &[data-pressed], | ||
| &[data-expanded] { |
There was a problem hiding this comment.
I think this one isn't quite right. In DatePicker, the button remains scaled down when the popover is open. It should only change the background.
There was a problem hiding this comment.
yep, I had added it for visual parity with what was on prod for combobox/datepicker/other field button styling but forgot that was actually what we didn't want to have
| style: {'--trigger-width': buttonWidth} as React.CSSProperties | ||
| }] | ||
| }], | ||
| [ButtonContext, {isExpanded: state.isOpen}] |
There was a problem hiding this comment.
Should we only do this for popovers and not modals somehow? I think this will still cause #8339
5 participants
Fixes #8339
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.The original behavior of tying the isPressed state to the expanded state was a mistake due to assumptions in behavior made for S2 which should not of propagated to React Aria Components.
✅ 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