feat(datagrid): add built-in date filter

[derkoe]

PR Checklist

Please check if your PR fulfills the following requirements:

• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)
• If applicable, have a visual design approval

PR Type

What kind of change does this PR introduce?

• Bugfix
• 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?

Issue Number: #227

What is the new behavior?

This adds a new built-in Datagrid filter for date fields.

Does this PR introduce a breaking change?

• Yes
• No

This adds a new built-in filter for dates. The Clarity API changes with that - I am not sure if this is breaking change.

Other information

Here is a screenshot of how this looks like:

[github-actions]

👋 @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

[colinreedmiller]

The PR is not the right time for design to first see this. This may be delayed until we have the bandwidth to assess - there are some changes to the date picker component itself currently in flight.

[Partick]

Hi, this is Patrick : designer on Clarity. I'd like to see functioning demo of the functionality and be able to test with voiceover for accessibility. Until I can see it working I can't approve the design.

[kevinbuhmann]

This should be added to demo (or storybook, but we don't have datagrid filters in the storybook yet) so that @Partick can see a working demo via the preview link.

[derkoe]

@kevinbuhmann it is added to the demo (https://github.com/vmware-clarity/ng-clarity/pull/250/files#diff-842c25e99bf62305df017f60c80148ef1ba342beb51368f9a67a6f2d12b82c38).

[kevinbuhmann]

@derkoe: Sorry about that. I should have checked. I must have forgotten since the last time I looked at this.

@Partick: You can see this filter at https://250--storybook-clarity-design.netlify.app/demo/datagrid/string-filtering.

[derkoe]

@Partick any updates on this?

[kevinbuhmann]

The tests are not imported and executed in the datagrid all.spec.ts. This means the tests are not actually executed. Also, changes need to be made after #279. (I can do these changes, but I want to make sure we don't forget.)

[kevinbuhmann]

I went ahead and 1) rebased onto main and 2) added the tests to all.spec.ts and applied changes from #279 to prevent issues with Renderer2.

The tests do not pass. I will leave that to @derkoe to fix since this is his code.

[kevinbuhmann]

Test are also needed for datagrid-property-date-filter.ts.

[derkoe]

@kevinbuhmann I've fixed the tests

I also realized that the the spec for DatagridPropertyNumericFilter's test is not in all.spec.ts and the test is not working. I have fixed this here: main...derkoe:chore/fix-datagrid-tests. Should I create a PR?

[kevinbuhmann]

Thanks!

And yeah, a PR for the other tests would be greatly appreciated!

[derkoe]

Fix for the DatagridPropertyNumericFilter's tests: #283

[kevinbuhmann]

Please rebase instead of merging. If you don't know how, I can do it for you.

[derkoe]

Please rebase instead of merging. If you don't know how, I can do it for you.

Done

[ashleyryan]

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

[Partick]

So there are several concerns with this pull request from a UX perspective.

• Selecting a date closes both popups
• Hitting escape closes both popups
• Does not read as a date range
  • i.e. “from” [start date] “to” [end date]
• Lacks a descriptive header

Here's a concept that I created that could work.

[kevinbuhmann]

Embedding the date picker into the filter popup is likely to be a difficult undertaking. The future of datagrid filters is in question due to various outstanding accessibility concerns, so I don't think we should implement a date filter in Clarity until the existing datagrid filter issues are resolved.

[derkoe]

Embedding the date picker into the filter popup is likely to be a difficult undertaking. The future of datagrid filters is in question due to various outstanding accessibility concerns, so I don't think we should implement a date filter in Clarity until the existing datagrid filter issues are resolved.

Yes, there are different issues but I think they could be fixed.

Current issues are:

• Cannot clear filters easily - see feat(datagrid): add possibility to clear filters #246
• The built-in number filter has not very good UX
• Not providing common filters (like the here proposed date filter) will lead to inconsistencies in the different implementations by different team. That means you'll see different look and behaviour in different parts of apps using Clarity

We implemented the filter now in clarity-addons: porscheinformatik/clarity-addons#1760. But this is not as nice as having a built-in filter. Built-in filters cannot be added in a third-party library because the API for that is private. Maybe we can change that?

[kevinbuhmann]

Our accessibility experts consistently tell us that filters should not be in the column headers, so I think there is a real possibility of the built-in datagrid filters being completely changed or even removed.

So I don't think we'll consider opening up the built-in filter API at this time. Sorry!

[d-m-s]

Hi guys,

this issue should come to a decision!
At the moment Clarity's datagrid supports and has documented the column filters, but they are really not working well with datepickers and additional popovers in the filter.
Neither in regular use, nor in terms of accessibility.

So this is a hop or drop decision I guess

• A.) go with what is already there and improve it:
  I really like the suggested improvements from @Partick

Here's a concept that I created that could work.

• B.) drop the support completely, and
  adapt the documentation quickly, and come up with an alternative concept.

The current state leaves everybody unclear whether to use the filter or not, plus what to use instead.
So this will result in lesser consistency and worse ux for sure.

@pdaigle, @kevinbuhmann, @Partick can u make or drive that decision so we at least avoid this state of confusion?

[lerch015]

First off, I agree generally with the concerns concerns raised by @d-m-s and @derkoe , though I think it would be helpful to maybe qualify two of the points here, just so we have a better handle on what exactly you're trying to solve for:

1. @derkoe : The built-in number filter has not very good UX Why?
2. @d-m-s : 'but they are really not working well with datepickers and additional popovers in the filter' I think I understand the popovers bit, but what else is causing a problem?

I think getting more detail here will help us better understand what you're trying to solve, which should help get us closer to a solution that can be applied consistently, and effectively for all of our users.

--

As @colinreedmiller mentioned earlier, when a PR impacting design comes in without the design team's prior knowledge, it does require us to take a couple of steps back to ensure that what's being requested is inline with the overall design system rational, and some of our planning around future work. This isn't foot planting as much as its normal due diligence that would have ideally happened a bit earlier. We'd like to make this work for you, but we'll need you to pardon our mess a bit as we work things through.

Just for context, an exacerbating factor right now is the number of things we're currently looking into around this particular topic that would need to adjust, and be adjusted by what's already submitted.

You're probably not aware, but we've got a bunch of needs here we're currently looking into:

1. Time picker support
2. Date, month, and year picker support, all requiring ranges and the ability to select with time.
3. Longer term updates to filtering in general, including column filtering, but also more advanced global filtering options
4. A request for a custom dropdown component that will likely be a dependency eventually Create Custom Dropdown Component #448
5. General updates to table architecture to provide more requested functionality consistently
6. General accessibility support and enhancement

So I think the trick here is to work through a solution to the needs you're trying to resolve, while also lining that solution up against some of the other needs in 1-6 so we don't run into change management issues as we enhance further. Right now 2 & 4 seem to me like the areas most impacted by this request and vice versa.

As for next steps, we can look into what would be needed to reconcile other component needs with what @Partick already shared. In the meantime, any specifics you can provide @d-m-s and @derkoe would be very helpful.

[derkoe]

@lerch015

The built-in number filter has not very good UX Why?

There are many issues with this component:

1. Clearing the number filter is too complicated (open, go to field one, clear it, go to field two, clear it).
   See also feat(datagrid): add possibility to clear filters #246 (comment)
2. The fields have no labels, the dialog has no labels (I'm not sure if this is also an accessibility issue):
   
3. The fields are way too big - they have space for up to a quadrillion

[lerch015]

@derkoe Thanks for getting back.

1. Got it, but this makes me wonder if this is an issue with filters, or counters (in that we'd need one), or form fields in general. It's inefficient for sure, but that would probably be the case in other contexts as well.
2. Makes sense. Again, this takes me back to forms as well, but partially our execution on them in this particular context.
3. Not sure what the impact is here, and I'm assuming things are general largely because there's no way to understand what's being counted. I can definitely see how some level of control over the limits, and their error handling methods, would be useful here.

Appreciate the feedback. Are any of these three creating specific issues with your users?

[derkoe]

@lerch015
Yes, clearing the filter is a major issue - that's why I have create the PR #246. Clearing a filter is quite a common task and users get frustrated when they have to perform at least 4 actions (clicks, keystrokes) to clear a number filter.

[lerch015]

'users get frustrated when they have to perform at least 4 actions (clicks, keystrokes) to clear a number filter'

@derkoe Is this your own observation, or something from prior research?

Thanks for the feedback.

[d-m-s]

Hi guys,

'users get frustrated when they have to perform at least 4 actions (clicks, keystrokes) to clear a number filter'
@derkoe Is this your own observation, or something from prior research?

@lerch015 I guess it's pretty obvious - but yes we do have multiple user observations and user feedback that the current behaviour is cumbersome.

...when a PR impacting design comes in without the design team's prior knowledge ...

The issue was raised in the Discussions > Ideas more than 6 months ago. (see #227)
The idea of providing a date-range picker dates several years back vmware-archive/clarity#474

How to go on?

For my part, I think three major parts should be solved ...

1.) Avoiding filter-popups to close itself, when the user is acting inside the filter-popup
Eg. filling two or more datepicker fields, currently the filter-popup closes itself when selecting the first value.

2.) Provide an easy way to clear the whole column-filter.
Inside the filter-popup there are likely more than one input field. Clearing each one separately is cumbersome. #246 (comment)

3.) Built-in Filters for default-types (like Date, DateRange, ... )
For datagrids which have date-columns, an appropriate/probably built-in filter would be great.

Some usecases to be considered:

• Date: select just single day.
• DateRange: select start and end date. How this can look like, has been describe by your team in #474
• DateTime: select day and time of day.
• DateTimeRange: select start and end date as well as time.

For the beginning I'm sure Date would be sufficient, just to get some consistency throughout large applications - with custom filters, theses "standard-usecases" are implemented always differently and hence harm the usability as well as the efficiency of Clarity for users as well as for developers.