Skip to content
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

Support onAction and isDisabled on item elements #5874

Merged
merged 7 commits into from
Apr 1, 2024

Conversation

devongovett
Copy link
Member

Relates to #5058 #5459

This adds support for the onAction and isDisabled props directly on collection items in addition to the existing collection-level onAction and disabledKeys. In some cases it can be easier to set these props at an item level, for example with static collections (e.g. common in menus). If both are provided, an item is disabled if either the isDisabled prop is set or its key is in disabledKeys, and both onAction handlers are called.

Note that while this is implemented in the hooks, these props are only in the TS definitions for RAC and not RSP (or the older stately collections). Since RSP uses a single <Item> component across all components, we can't easily add props to some components but not others. In RAC we separated these out into separate components so this is possible.

@rspbot
Copy link

rspbot commented Feb 14, 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.

move from state.disabledKeys to state.selectionManager is a bit unfortunate if anyone has written their own work on top of collections, but probably not very common and we can mention it in release notes

I forget, have we also discussed isSelected already as well?

@devongovett
Copy link
Member Author

yeah it shouldn't break because disabledKeys is still there, but just won't support the new feature until migrating.

what about isSelected?

@snowystinger
Copy link
Member

snowystinger commented Feb 14, 2024

what about isSelected?

did we want to support the same kind of thing? is that possible? I have a vague feeling it wasn't possible, but can't recall why

@devongovett
Copy link
Member Author

you mean isSelected on individual items? I think that's not possible because multiple items can be selected at once, so onSelectionChange really needs to be on the root level. Makes sense to keep selectedKeys at the same level.

@rspbot
Copy link

rspbot commented Mar 28, 2024

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.

LGTM, I'll add a followup to add isDisabled to TreeItem as well

@rspbot
Copy link

rspbot commented Apr 1, 2024

@rspbot
Copy link

rspbot commented Apr 1, 2024

## API Changes

unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any', access: 'private' }
unknown top level export { type: 'any', access: 'private' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'identifier', name: 'Column' }
unknown top level export { type: 'identifier', name: 'Column' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }

@react-aria/grid

GridKeyboardDelegateOptions

 GridKeyboardDelegateOptions<C, T> {
   collator?: Intl.Collator
   collection: C
   direction: Direction
+  disabledBehavior?: DisabledBehavior
   disabledKeys: Set<Key>
   focusMode?: 'row' | 'cell'
   layout?: Layout<Node<T>>
   ref?: RefObject<HTMLElement>
 

it changed:

  • GridKeyboardDelegate

@LFDanLu LFDanLu merged commit 5d86732 into main Apr 1, 2024
27 checks passed
@LFDanLu LFDanLu deleted the disabled-onaction-items branch April 1, 2024 22:24
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.

None yet

4 participants