Skip to content

Conversation

snowystinger
Copy link
Member

Closes

A few remaining items that I realised I'd forgotten

Adds a background colour to hovered cells.
Adds new docs example
Remove unused onCancel prop

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


### EditableCell

<PropTable component={docs.exports.EditableCell} links={docs.links} showDescription />
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 do i debug the PropTable.tsx component? I can't tell why placement is being added to an Overlay group in the table

Also, noticed that I cannot see the bottom of the TOC unless I scroll to the bottom of the page

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure I'm seeing this placement prop that you are mentioning?

Image

styles: style({
// TODO: really need access to display here instead, but not possible right now
// will be addressable with displayOuter
// Could use `hidden` attribute instead of css, but I don't have access to much of this state at the moment
Copy link
Member Author

Choose a reason for hiding this comment

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

still need to solve this issue

@rspbot
Copy link

rspbot commented Oct 9, 2025

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.

Is this focus-visible state correct?

Shows a table where the editable cell is focused with focus visible, and there is no background color on that cell

LFDanLu
LFDanLu previously approved these changes Oct 20, 2025
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, but some small comments


### EditableCell

<PropTable component={docs.exports.EditableCell} links={docs.links} showDescription />
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure I'm seeing this placement prop that you are mentioning?

Image

Comment on lines +1087 to +1095
':is([role="rowheader"]:hover, [role="gridcell"]:hover)': {
selectionMode: {
none: colorMix('gray-25', 'gray-900', 7),
single: 'gray-25',
multiple: 'gray-25'
}
},
':is([role="row"][data-focus-visible-within] [role="rowheader"]:focus-within, [role="row"][data-focus-visible-within] [role="gridcell"]:focus-within)': 'gray-25'
}
Copy link
Member

Choose a reason for hiding this comment

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

similar question to what @reidbarber mentioned, but more about the hover state: should there really be hover state if selection isn't supported but editing is? IMO it makes it kinda feel like there is an associated row action when there isn't. The designs aren't super clear to me if this is desired or not haha

Copy link
Member Author

Choose a reason for hiding this comment

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

From discussions with design, yes, it should, and we'll want to open that up in RAC in the future

Copy link
Member Author

Choose a reason for hiding this comment

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

From discussions with design, yes, it should, and we'll want to open that up in RAC in the future

@snowystinger
Copy link
Member Author

@LFDanLu

I'm not sure I'm seeing this placement prop that you are mentioning?

I'm not sure about placement, but align is wrong, that's cell alignment, not overlay alignment

@snowystinger
Copy link
Member Author

@reidbarber

Is this focus-visible state correct?

yep, I believe so

@rspbot
Copy link

rspbot commented Oct 20, 2025

@rspbot
Copy link

rspbot commented Oct 20, 2025

## API Changes

react-aria-components

/react-aria-components:TooltipTrigger

 TooltipTrigger {
   children: ReactNode
   closeDelay?: number = 500
-  closeOnPress?: boolean = true
   defaultOpen?: boolean
   delay?: number = 1500
   isDisabled?: boolean
   isOpen?: boolean
   trigger?: 'hover' | 'focus' = 'hover'
 }

/react-aria-components:SearchFieldRenderProps

 SearchFieldRenderProps {
   isDisabled: boolean
   isEmpty: boolean
   isInvalid: boolean
-  isReadOnly: boolean
-  isRequired: boolean
   state: SearchFieldState
 }

/react-aria-components:ColumnRenderProps

 ColumnRenderProps {
   allowsSorting: boolean
   isFocusVisible: boolean
   isFocused: boolean
   isHovered: boolean
-  isPressed: boolean
   isResizing: boolean
   sort: (SortDirection) => void
   sortDirection: SortDirection | undefined
   startResize: () => void

/react-aria-components:TooltipTriggerComponentProps

 TooltipTriggerComponentProps {
   children: ReactNode
   closeDelay?: number = 500
-  closeOnPress?: boolean = true
   defaultOpen?: boolean
   delay?: number = 1500
   isDisabled?: boolean
   isOpen?: boolean
   trigger?: 'hover' | 'focus' = 'hover'
 }

/react-aria-components:RangeCalendarState

-RangeCalendarState <T extends DateValue = DateValue> {
+RangeCalendarState {
   anchorDate: CalendarDate | null
   focusNextDay: () => void
   focusNextPage: () => void
   focusNextSection: (boolean) => void
   focusPreviousDay: () => void
   focusPreviousPage: () => void
   focusPreviousRow: () => void
   focusPreviousSection: (boolean) => void
   focusSectionEnd: () => void
   focusSectionStart: () => void
   focusedDate: CalendarDate
   getDatesInWeek: (number, CalendarDate) => Array<CalendarDate | null>
   highlightDate: (CalendarDate) => void
   highlightedRange: RangeValue<CalendarDate> | null
   isCellDisabled: (CalendarDate) => boolean
   isCellFocused: (CalendarDate) => boolean
   isCellUnavailable: (CalendarDate) => boolean
   isDisabled: boolean
   isDragging: boolean
   isFocused: boolean
   isInvalid: (CalendarDate) => boolean
   isNextVisibleRangeInvalid: () => boolean
   isPreviousVisibleRangeInvalid: () => boolean
   isReadOnly: boolean
   isSelected: (CalendarDate) => boolean
   isValueInvalid: boolean
   maxValue?: DateValue | null
   minValue?: DateValue | null
   selectDate: (CalendarDate) => void
   selectFocusedDate: () => void
   setAnchorDate: (CalendarDate | null) => void
   setDragging: (boolean) => void
   setFocused: (boolean) => void
   setFocusedDate: (CalendarDate) => void
   setValue: (RangeValue<DateValue> | null) => void
   timeZone: string
   value: RangeValue<DateValue> | null
   visibleRange: RangeValue<CalendarDate>
 }

@react-aria/table

/@react-aria/table:TableColumnHeaderAria

 TableColumnHeaderAria {
   columnHeaderProps: DOMAttributes
-  isPressed: boolean
 }

@react-aria/tooltip

/@react-aria/tooltip:TooltipTriggerProps

 TooltipTriggerProps {
   closeDelay?: number = 500
-  closeOnPress?: boolean = true
   defaultOpen?: boolean
   delay?: number = 1500
   isDisabled?: boolean
   isOpen?: boolean
   trigger?: 'hover' | 'focus' = 'hover'
 }

@react-spectrum/s2

/@react-spectrum/s2:EditableCell

 EditableCell {
   align?: 'start' | 'center' | 'end' = 'start'
   children: ReactNode
   className?: ClassNameOrFunction<CellRenderProps> = 'react-aria-Cell'
   colSpan?: number
   id?: Key
   isSaving?: boolean
-  onCancel: () => void
   onSubmit: () => void
   renderEditing: () => ReactNode
   showDivider?: boolean
   style?: StyleOrFunction<CellRenderProps>
 }

/@react-spectrum/s2:TooltipTrigger

 TooltipTrigger {
   children: ReactNode
-  closeOnPress?: boolean = true
   containerPadding?: number = 12
   crossOffset?: number = 0
   defaultOpen?: boolean
   delay?: number = 1500
   isOpen?: boolean
   offset?: number = 0
   onOpenChange?: (boolean) => void
   placement?: 'start' | 'end' | 'right' | 'left' | 'top' | 'bottom' = 'top'
   shouldFlip?: boolean = true
   trigger?: 'hover' | 'focus' = 'hover'
 }

/@react-spectrum/s2:TooltipTriggerProps

 TooltipTriggerProps {
   children: ReactNode
   closeDelay?: number = 500
-  closeOnPress?: boolean = true
   defaultOpen?: boolean
   delay?: number = 1500
   isDisabled?: boolean
   isOpen?: boolean
   trigger?: 'hover' | 'focus' = 'hover'
 }

@react-spectrum/tooltip

/@react-spectrum/tooltip:TooltipTrigger

 TooltipTrigger {
   children: [ReactElement, ReactElement]
-  closeOnPress?: boolean = true
   containerPadding?: number = 12
   crossOffset?: number = 0
   defaultOpen?: boolean
   delay?: number = 1500
   isOpen?: boolean
   offset?: number = 7
   onOpenChange?: (boolean) => void
   placement?: Placement = 'top'
   shouldFlip?: boolean = true
   trigger?: 'hover' | 'focus' = 'hover'
 }

/@react-spectrum/tooltip:SpectrumTooltipTriggerProps

 SpectrumTooltipTriggerProps {
   children: [ReactElement, ReactElement]
-  closeOnPress?: boolean = true
   containerPadding?: number = 12
   crossOffset?: number = 0
   defaultOpen?: boolean
   delay?: number = 1500
   isOpen?: boolean
   offset?: number = 7
   onOpenChange?: (boolean) => void
   placement?: Placement = 'top'
   shouldFlip?: boolean = true
   trigger?: 'hover' | 'focus' = 'hover'
 }

@react-stately/calendar

/@react-stately/calendar:RangeCalendarState

-RangeCalendarState <T extends DateValue = DateValue> {
+RangeCalendarState {
   anchorDate: CalendarDate | null
   focusNextDay: () => void
   focusNextPage: () => void
   focusNextSection: (boolean) => void
   focusPreviousDay: () => void
   focusPreviousPage: () => void
   focusPreviousRow: () => void
   focusPreviousSection: (boolean) => void
   focusSectionEnd: () => void
   focusSectionStart: () => void
   focusedDate: CalendarDate
   getDatesInWeek: (number, CalendarDate) => Array<CalendarDate | null>
   highlightDate: (CalendarDate) => void
   highlightedRange: RangeValue<CalendarDate> | null
   isCellDisabled: (CalendarDate) => boolean
   isCellFocused: (CalendarDate) => boolean
   isCellUnavailable: (CalendarDate) => boolean
   isDisabled: boolean
   isDragging: boolean
   isFocused: boolean
   isInvalid: (CalendarDate) => boolean
   isNextVisibleRangeInvalid: () => boolean
   isPreviousVisibleRangeInvalid: () => boolean
   isReadOnly: boolean
   isSelected: (CalendarDate) => boolean
   isValueInvalid: boolean
   maxValue?: DateValue | null
   minValue?: DateValue | null
   selectDate: (CalendarDate) => void
   selectFocusedDate: () => void
   setAnchorDate: (CalendarDate | null) => void
   setDragging: (boolean) => void
   setFocused: (boolean) => void
   setFocusedDate: (CalendarDate) => void
   setValue: (RangeValue<DateValue> | null) => void
   timeZone: string
   value: RangeValue<DateValue> | null
   visibleRange: RangeValue<CalendarDate>
 }

@react-stately/tooltip

/@react-stately/tooltip:TooltipTriggerProps

 TooltipTriggerProps {
   closeDelay?: number = 500
-  closeOnPress?: boolean = true
   defaultOpen?: boolean
   delay?: number = 1500
   isDisabled?: boolean
   isOpen?: boolean
   trigger?: 'hover' | 'focus' = 'hover'
 }

@snowystinger snowystinger added this pull request to the merge queue Oct 20, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 20, 2025
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.

4 participants