-
Notifications
You must be signed in to change notification settings - Fork 10
FilterMenu: support a "defaultExcludedValues" prop #418
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
Conversation
It took a long time to get this to behave intuitively. In short, it took a long to spec it out. How to have it behave when you click one of the values, how to represent the default state, ... The feature as implemented: * FilterMenu has a new prop defaultExcludedValues: string[] * Without defaultExcludedValues set, it operates exactly as it does now * When set, the UI has one new "showDefault" state in addition to existing "showAll" * In "showDefault", Pulls matching one of the excluded values are hidden (these excluded values appear de-selected in the UI) * One click "selects" them and leaves "showDefault" mode * "Show All" overrides the "default exclusions" and shows everything
Provide a way to hide a repo by default from the config by adding an
option to the config. We have to pass the repo specs part of the config
around more, and transform these `{org}/{repo}` names into just the
{name} portion for the filter, but this works for now.
jarstelfox
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CR 🌵
jarstelfox
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CR 🌵
|
Glad to see this happening. I think ifixit-schooner-* and PD-Test should probably be added to the default exclusion list. It used to be that clicking a single repo would show just that repo, which is usually what I want from a menu like that, but now since they all start checked, clicking a single repo hides it. That is reasonably intuitive when they start checked, though there's no way to select none, so if I do just want to see a single repo, it's like 15 clicks. I think if there was a select none option, that would be sufficient. Datadog handles this with a little button on each menu option that says "only", but that might be overkill for this. Maybe also if it could remember my selection in localstorage or something, that would be good. I think that Select All, Default, and None should be buttons (actions) rather that checkboxes. In any case I think this is a great step in the right direction, so thanks for working on it. |
All selections are stored in the url. Bookmarking works well and allows several different configurations.
That's not a bad idea. I'll try that out. |
Instead of "Show All" being a state, it's now just a button. Instead of a click isolating that option when in Show All mode, it will simply uncheck it. This adds an "only" option to every menu item to replace the old functionality.
Done! And I've added an "only" instead of "None" |
|
QA 💎 - We can now exclude repos from pulldasher by default. We can still view the excluded repo with the Repo dropdown by clicking Show All, or by clicking the repo name.
very small deploy_block 🦖 - When you click Show All, the dropdown indicates that only one repo has been selected, instead of all repos. Then if you unselect a repo, and reselect it, the dropdown now indicates that 17 repos have been selected. Maybe when Show All is selected, we can just show the number of repos that are selected (right now it's 17), and when Show Default, we can hide the number like we currently do? |
|
I think we should keep it simple, and do I'm open to |
jarstelfox
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CR 🌵
Previously, it would show `{FilterName} (1)` when "Show All" was chosen
because it was reporting the number of values in the url.
Now, it shows the total count any time the selection deviates from the
default.



It took a long time to get this to behave intuitively. In short, it took
a long to spec it out. How to have it behave when you click one of the
values, how to represent the default state, ...
The feature as implemented:
existing "showAll"
(these excluded values appear de-selected in the UI)
Closes #414