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

Multiselect on categories #21

Open
nikodraca opened this issue Jan 28, 2022 · 16 comments
Open

Multiselect on categories #21

nikodraca opened this issue Jan 28, 2022 · 16 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@nikodraca
Copy link

nikodraca commented Jan 28, 2022

It would be great if you could multiselect values on category fields. Currently you can only select one category to filter on.

Any interest in this? I don't mind taking a crack at it.

@Wattenberger
Copy link
Contributor

we would definitely be open to a PR on this, thanks!

@Wattenberger Wattenberger added enhancement New feature or request help wanted Extra attention is needed labels Jan 28, 2022
@nikodraca
Copy link
Author

@Wattenberger would you be able to give me permissions to make PRs 🙏?

@Wattenberger
Copy link
Contributor

@nikodraca do you mind forking the repo and creating a PR from there? Similar to this PR:
#10

@nikodraca nikodraca changed the title Filter on tags? Multiselect on categories Feb 15, 2022
@nikodraca
Copy link
Author

@Wattenberger revisiting this now that I have a better understanding of the codebase - I think the first item of work is multiselect on category fields. I believe this will be doable with Downshift (though I don't have any experience with it).

The second item of work, which I think requires a larger discussion, is about filtering on array fields:

  • is filtering by array fields a feature you/others would want? Personally I have a use case where I'd like to be able to filter on multiple items of an array (ie. give me all rows where the array includes X or Y)
  • is there a reason to display X item(s) in the cell vs something like a stringified array for array fields?
  • I can't quite figure out the difference between a short-array and array, could you explain this to me?

Excited to contribute 😄

@Wattenberger
Copy link
Contributor

is filtering by array fields a feature you/others would want? Personally I have a use case where I'd like to be able to filter on multiple items of an array (ie. give me all rows where the array includes X or Y)

I bet we'll be able to add multiselect in a way that doesn't detract from the single-select use case. If so, I bet a lot of people would find it useful!

is there a reason to display X item(s) in the cell vs something like a stringified array for array fields?
I can't quite figure out the difference between a short-array and array, could you explain this to me?

I think these questions are related. We're differentiating between columns where the contents are usually one item in an array (short-array) and other arrays (array). This is so we can display shorter array values in a more readable fashion, whereas longer arrays are potentially easier to "read" & compare if we just display the number of items in each array.
https://github.com/githubocto/flat-ui/blob/main/src/store.ts#L813-L831

Let me know if any of that didn't make sense!

@nikodraca
Copy link
Author

So if I understand correctly the idea was that arrays really meant to be filtered by length and less by the contents?

Follow up q: couldn't we consolidate categories and short-arrays?

@Wattenberger
Copy link
Contributor

hmm, potentially. They wouldn't automatically be categories, but could be passed further down and treated the same as columns of strings: https://github.com/githubocto/flat-ui/blob/main/src/store.ts#L688

I'm a bit hesitant to do more "magic" handling of data, though! The farther we get from the raw data, the less reliable the table view gets. There is the possibility to treat items within array values are rendered as pills, similar to category values, which is perhaps what you're getting at. In that case, I think the path forward would be to have a multiselect filter, and to change the render component for arrays.

@nikodraca
Copy link
Author

Ok cool, thanks for answering all my questions 😄

@nikodraca
Copy link
Author

nikodraca commented Feb 19, 2022

Hey @Wattenberger 👋 wanted to share some progress and get some feedback:
https://user-images.githubusercontent.com/3537270/154796985-ab232baa-1a24-4cef-991b-7c06208c6794.mov

What's New:

  • Categories filters are now arrays
  • Input is now multiselect (using Downshift's useMultipleSelection
  • Column header shows current selection with option to remove

Questions:

  • Wasn't sure what the UX for displaying/removing current selection should be. I opted for displaying them in a row and making that scrollable. Any thoughts on this?
  • You might notice in the screencast that when filtering on the second category it shows the count as 0 (cool (0)). This is because a row can only have 1 category, but category filters apply to the entire dataset, not just the filtered subset. I'm thinking of instead counting how many rows have that category from originalData instead of filteredData. Does that make sense?

@nikodraca
Copy link
Author

Hey @Wattenberger 😄 just wanted to check in on this, really eager to get the ball rolling!

@nikodraca
Copy link
Author

Hey @Wattenberger sorry to bother again, I'd love to get some eyes on this! Is there someone on the team you could put me in touch with?

@Wattenberger
Copy link
Contributor

hey Niko! sorry about the delay! I was moving cross-country and now the team is heads-down on a launch. I'll try to get to this within the next week!

@nikodraca
Copy link
Author

No worries at all! Hope the move went well 😄

@Wattenberger
Copy link
Contributor

okay! this is looks great, thanks for the progress! A few tiny suggestions:

  • it might be nice to have the input/prompt centered vertically when there are no active filters, to match the other headers
  • I think the horizontal list is a good route, since we don't want to change the height of the header cell. Might be worth bumping the font-size down for the pills, decreasing the horizontal spacing, and adding some vertical padding around the list. You could also expand the list on hover, similar to what we do for cells with long text content.
  • probably also worth removing the circle around the ❌ icons, to decrease visual noise
  • you could also make the "dropdown" overlay taller, skinnier, and add some vertical padding

I'm thinking of instead counting how many rows have that category from originalData instead of filteredData. Does that make sense?
Makes total sense!

great progress so far!

@nikodraca
Copy link
Author

great progress so far!

I got a new MacBook this week and didn't push changes before I wiped it 🙃 looks like I'll be starting from scratch, but thank you for the comments! Talk soon

@Wattenberger
Copy link
Contributor

Ah super frustrating! Good luck re-building those changes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants