ng-clarity
ng-clarity copied to clipboard
feat(datagrid): add possibility to clear filters
PR Checklist
Please check if your PR fulfills the following requirements:
- [x] Tests for the changes have been added (for bug fixes / features)
- [x] Docs have been added / updated (for bug fixes / features)
Added it to the demo page.
I could not find the sources for https://clarity.design/documentation/datagrid/structure - [ ] If applicable, have a visual design approval
PR Type
What kind of change does this PR introduce?
- [ ] Bugfix
- [x] Feature
- [ ] Code style update (formatting, local variables)
- [ ] Refactoring (no functional changes, no api changes)
- [ ] Build related changes
- [ ] CI related changes
- [ ] Documentation content changes
- [ ] Other... Please describe:
What is the current behavior?
Now, you cannot easily clear a filter. For string filters you have to delete all chars in the field, for number filters you have to click both fields and clear them
Issue Number: #225
What is the new behavior?
There is a new clear button added to the filter in the Datagrid which clears the current filter
Does this PR introduce a breaking change?
- [ ] Yes
- [x] No
Other information
👋 @derkoe,
- 🙏 The Clarity team thanks you for opening a pull request
- 🎉 The build for this PR has succeeded
- 🔍 The PR is now ready for review
- 🍿 In the meantime, checkout out a preview of this PR
- 🖐 You can always follow up here. If you're a VMware employee, you can also reach us on our internal #clarity-support Slack channel
Thank you,
🤖 Clarity Release Bot
The visual regression test is failing because of a bug in the workflow for forks. I am going to look into fixing it.
We need to get @colinreedmiller involved since this includes a visual change (and a slightly breaking one at that)
'Does this PR introduce a breaking change?' checkbox indicates no breaking change - does need to be updated? Please add me as a reviewer.
@colinreedmiller this PR came out of this discussion - I think it's a visual breaking change based on the screenshot?
https://github.com/vmware-clarity/ng-clarity/discussions/225
@colinreedmiller this PR came out of this discussion - I think it's a visual breaking change based on the screenshot?
There is no breaking change it the API - it only adds the "Clear" button to the built-in filters (hence the visual change). Custom filters do not get the "Clear" button since they have to implement the clear(): void
method.
Thanks for the screenshot @ashleyryan - Yes, this is visually breaking and a design change more generally. UX needs to look at this before moving ahead.
Is it possible to share the visual design via a screenshot? I need to see a working demo before I can provide feedback.
I think we have filters in the demo app, but this build is failing. @derkoe, please fix the build errors, so that @Partick can see a live demo via the preview link.
Fixed the build - @Partick you can see the working demo here https://246--storybook-clarity-design.netlify.app/?path=/story/datagrid-column--default
@Partick any updates on this?
it has been a month since the contribution attempt ... can we help speed things up a little?
Is it possible to share the visual design via a screenshot? I need to see a working demo before I can provide feedback.
also discussed upfront: https://github.com/vmware-clarity/ng-clarity/discussions/225
working demo as already posted here https://246--storybook-clarity-design.netlify.app/?path=/story/datagrid-column--default
Heads up on this - there has been a lot of PTO with summer vacations the last month or so. Design is currently looking at this, there's some concerns about the popup in a popup behavior
Design is currently looking at this, there's some concerns about the popup in a popup behavior
Which popup? This change only adds the trash can to clear the filter.
Which popup? This change only adds the trash can to clear the filter.
Oops, I commented on the wrong PR. Design is looking at this one too
BTW this the current UX of clearing filters. For numbers you have to first focus the lower bounds, clear the field, then focus the upper bound and clear the field; then close the filter.
Any news on this?
If this will not be merged, can we at least export CustomFilter
from Clarity and make smartToggleService
protected? Then we can create our own clearable datagrid filter component.
@colinreedmiller As you're currently the assigned person I'd like to ask how we can help to speedup the progress a little bit? Thanks :)
@colinreedmiller As you're currently the assigned person I'd like to ask how we can help to speedup the progress a little bit? Thanks :)
Hello @Partick - I just noticed you're in the lead now :D - please tell me what is missing to get this PR merged. Thanks :)
any updates to this issue? @Partick