Skip to content

Conversation

@ktabors
Copy link
Member

@ktabors ktabors commented Aug 6, 2024

Closes issue
Closes #5503

  • Do we need an expandable rows with disabledKeys story?
  • Do we need a story with disabledKeys and content like buttons in the cells?
  • Documentation wasn't mentioned in the scoping, but the docs would be updated to match ListView, correct?
  • Add or update a chromatic story?

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

Added control "disabledBehavior" to TableView stories.
Easiest to test is you set selectionMode to "multiple" because the checkboxes will be disabled in both modes.

Test with "dynamic with disabled keys" story.

Test DnD with "Drag between tables" story.

Didn't see a story to test expandable row with.

  • Confirm "selection" is the default for TableView. (Should be opposite ListView with is "all".)
  • Confirm "all" shows the cell content text as disabled.
  • Confirm "all" prevents DnD, focus, hover, actions, etc.

🧢 Your Project:

RSP

@rspbot
Copy link

rspbot commented Aug 6, 2024

Copy link
Member

@snowystinger snowystinger left a comment

Choose a reason for hiding this comment

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

Noticed an interesting side effect of this change.
https://reactspectrum.blob.core.windows.net/reactspectrum/51dfb35aff8dea1e82bd72c76a19c5e5415f0d7e/storybook/index.html?path=/story/tableview--dynamic-with-disabled-keys&args=disabledBehavior:selection;selectionMode:multiple&providerSwitcher-express=false

If I enter selection mode by selecting a row. Then clicking another enabled row will select it and will NOT call the action for that row.
However, while in selection mode with a row selected, if I click a disabled row, then the action IS called.

I think while a row is selected actions should not be called regardless of the disabledBehavior.

@ktabors
Copy link
Member Author

ktabors commented Aug 6, 2024

However, while in selection mode with a row selected, if I click a disabled row, then the action IS called.

I checked ListView and it's the same. You can select an enabled item (folder) and clicking on a disabled item will trigger the action.

Copy link
Member

@snowystinger snowystinger left a comment

Choose a reason for hiding this comment

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

pre-existing then is what you're saying? can we add an item to the backlog?

@ktabors
Copy link
Member Author

ktabors commented Aug 6, 2024

pre-existing then is what you're saying? can we add an item to the backlog?

Added RSP Component Milestones (view)

@rspbot
Copy link

rspbot commented Aug 7, 2024

@rspbot
Copy link

rspbot commented Aug 7, 2024

## API Changes

@react-spectrum/table

TableView

 TableView <T extends {}> {
   UNSAFE_className?: string
   UNSAFE_style?: CSSProperties
   alignSelf?: Responsive<'auto' | 'normal' | 'start' | 'end' | 'center' | 'flex-start' | 'flex-end' | 'self-start' | 'self-end' | 'stretch'>
   aria-describedby?: string
   aria-details?: string
   aria-label?: string
   aria-labelledby?: string
   bottom?: Responsive<DimensionValue>
   children: [ReactElement<TableHeaderProps<{}>>, ReactElement<TableBodyProps<{}>>]
   defaultSelectedKeys?: 'all' | Iterable<Key>
   density?: 'compact' | 'regular' | 'spacious' = 'regular'
+  disabledBehavior?: DisabledBehavior = "selection"
   disabledKeys?: Iterable<Key>
   disallowEmptySelection?: boolean
   dragAndDropHooks?: DragAndDropHooks['dragAndDropHooks']
   end?: Responsive<DimensionValue>
   flexBasis?: Responsive<number | string>
   flexGrow?: Responsive<number>
   flexShrink?: Responsive<number>
   gridArea?: Responsive<string>
   gridColumn?: Responsive<string>
   gridColumnEnd?: Responsive<string>
   gridColumnStart?: Responsive<string>
   gridRow?: Responsive<string>
   gridRowEnd?: Responsive<string>
   gridRowStart?: Responsive<string>
   height?: Responsive<DimensionValue>
   id?: string
   isHidden?: Responsive<boolean>
   isQuiet?: boolean
   justifySelf?: Responsive<'auto' | 'normal' | 'start' | 'end' | 'flex-start' | 'flex-end' | 'self-start' | 'self-end' | 'center' | 'left' | 'right' | 'stretch'>
   left?: Responsive<DimensionValue>
   margin?: Responsive<DimensionValue>
   marginBottom?: Responsive<DimensionValue>
   marginEnd?: Responsive<DimensionValue>
   marginStart?: Responsive<DimensionValue>
   marginTop?: Responsive<DimensionValue>
   marginX?: Responsive<DimensionValue>
   marginY?: Responsive<DimensionValue>
   maxHeight?: Responsive<DimensionValue>
   maxWidth?: Responsive<DimensionValue>
   minHeight?: Responsive<DimensionValue>
   minWidth?: Responsive<DimensionValue>
   onAction?: (Key) => void
   onResize?: (Map<Key, ColumnSize>) => void
   onResizeEnd?: (Map<Key, ColumnSize>) => void
   onResizeStart?: (Map<Key, ColumnSize>) => void
   onSelectionChange?: (Selection) => void
   onSortChange?: (SortDescriptor) => any
   order?: Responsive<number>
   overflowMode?: 'wrap' | 'truncate' = 'truncate'
   position?: Responsive<'static' | 'relative' | 'absolute' | 'fixed' | 'sticky'>
   renderEmptyState?: () => JSX.Element
   right?: Responsive<DimensionValue>
   selectedKeys?: 'all' | Iterable<Key>
   selectionMode?: SelectionMode
   selectionStyle?: 'checkbox' | 'highlight'
   sortDescriptor?: SortDescriptor
   start?: Responsive<DimensionValue>
   top?: Responsive<DimensionValue>
   width?: Responsive<DimensionValue>
   zIndex?: Responsive<number>
 }

SpectrumTableProps

 SpectrumTableProps <T> {
   UNSAFE_className?: string
   UNSAFE_style?: CSSProperties
   alignSelf?: Responsive<'auto' | 'normal' | 'start' | 'end' | 'center' | 'flex-start' | 'flex-end' | 'self-start' | 'self-end' | 'stretch'>
   aria-describedby?: string
   aria-details?: string
   aria-label?: string
   aria-labelledby?: string
   bottom?: Responsive<DimensionValue>
   children: [ReactElement<TableHeaderProps<T>>, ReactElement<TableBodyProps<T>>]
   defaultSelectedKeys?: 'all' | Iterable<Key>
   density?: 'compact' | 'regular' | 'spacious' = 'regular'
+  disabledBehavior?: DisabledBehavior = "selection"
   disabledKeys?: Iterable<Key>
   disallowEmptySelection?: boolean
   dragAndDropHooks?: DragAndDropHooks['dragAndDropHooks']
   end?: Responsive<DimensionValue>
   flexBasis?: Responsive<number | string>
   flexGrow?: Responsive<number>
   flexShrink?: Responsive<number>
   gridArea?: Responsive<string>
   gridColumn?: Responsive<string>
   gridColumnEnd?: Responsive<string>
   gridColumnStart?: Responsive<string>
   gridRow?: Responsive<string>
   gridRowEnd?: Responsive<string>
   gridRowStart?: Responsive<string>
   height?: Responsive<DimensionValue>
   id?: string
   isHidden?: Responsive<boolean>
   isQuiet?: boolean
   justifySelf?: Responsive<'auto' | 'normal' | 'start' | 'end' | 'flex-start' | 'flex-end' | 'self-start' | 'self-end' | 'center' | 'left' | 'right' | 'stretch'>
   left?: Responsive<DimensionValue>
   margin?: Responsive<DimensionValue>
   marginBottom?: Responsive<DimensionValue>
   marginEnd?: Responsive<DimensionValue>
   marginStart?: Responsive<DimensionValue>
   marginTop?: Responsive<DimensionValue>
   marginX?: Responsive<DimensionValue>
   marginY?: Responsive<DimensionValue>
   maxHeight?: Responsive<DimensionValue>
   maxWidth?: Responsive<DimensionValue>
   minHeight?: Responsive<DimensionValue>
   minWidth?: Responsive<DimensionValue>
   onAction?: (Key) => void
   onResize?: (Map<Key, ColumnSize>) => void
   onResizeEnd?: (Map<Key, ColumnSize>) => void
   onResizeStart?: (Map<Key, ColumnSize>) => void
   onSelectionChange?: (Selection) => void
   onSortChange?: (SortDescriptor) => any
   order?: Responsive<number>
   overflowMode?: 'wrap' | 'truncate' = 'truncate'
   position?: Responsive<'static' | 'relative' | 'absolute' | 'fixed' | 'sticky'>
   renderEmptyState?: () => JSX.Element
   right?: Responsive<DimensionValue>
   selectedKeys?: 'all' | Iterable<Key>
   selectionMode?: SelectionMode
   selectionStyle?: 'checkbox' | 'highlight'
   sortDescriptor?: SortDescriptor
   start?: Responsive<DimensionValue>
   top?: Responsive<DimensionValue>
   width?: Responsive<DimensionValue>
   zIndex?: Responsive<number>
 }

@ktabors ktabors merged commit 4d8b655 into main Aug 7, 2024
@ktabors ktabors deleted the tableView_disabledBehavior branch August 7, 2024 18:37
reidbarber added a commit that referenced this pull request Aug 21, 2024
* Update Picker placeholder to be shorter (#6796)

* feat: Support fragments in collections (#6430)

Co-authored-by: Reid Barber <[email protected]>
Co-authored-by: Robert Snow <[email protected]>

* Exposing prop disabledBehavior to TableView (#6832)

* fix/bug useTablist #5996 (#6023)

* fix/bug useTabist #5996 and added tests

* Extract `ToggleStateProps` type to use only what is needed in `useToggleState` (#3836)

* Extract `ToggleStateProps` type to use only what is needed in `useToggleState`

---------

Co-authored-by: Robert Snow <[email protected]>
Co-authored-by: solimant <[email protected]>
Co-authored-by: Kyle Taborski <[email protected]>
Co-authored-by: Medhashis Maiti <[email protected]>
Co-authored-by: Mateusz Burzyński <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

disabledBehavior support in TableView

5 participants