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

Possibly rename useSelectionState to something more generic like useToggleList or useArraySubset #66

Open
mturley opened this issue Jun 24, 2021 · 0 comments
Labels
kind/feature Categorizes issue or PR as related to a new feature. low-priority

Comments

@mturley
Copy link
Collaborator

mturley commented Jun 24, 2021

In forklift-ui, we use useSelectionState for both checkbox selections and row expansions in tables because the state requirements are exactly the same. However, the functions returned from this hook are named very specifically for selection, so it might be nice to make them more generic (toggleItem, isItemToggled, toggleAll etc) and avoid confusing renames in destructuring like this one:

  const {
    toggleItemSelected: toggleVMExpanded,
    isItemSelected: isVMExpanded,
    selectAll: expandAll,
  } = useSelectionState<SourceVM>({
    items: sortedItems,
    isEqual: (a, b) => a.selfLink === b.selfLink,
  });

We could then create thin wrappers with specific naming if we want, like useSelectionState, useExpansionState, etc. Or we could have these functions returned in an array so they can be named on the fly, but there are so many...

@mturley mturley added kind/feature Categorizes issue or PR as related to a new feature. low-priority labels Jun 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. low-priority
Projects
None yet
Development

No branches or pull requests

1 participant