Angular grids: interval columns are not filterable

[Jane Sandberg]

The new Angular grid filters are cool, but they don't yet know how to filter columns of the "interval" type. If you try adding [filterable]="true" to grids with interval columns, the filter area will just say "I don't know how to filter [column name]"

[Terran McCanna (tmccanna)]

Agreed, this is super confusing. Changing it from wish list to bug because of the confusion.

Maybe allowing it to filter intervals is a wish list, but I consider the "I don't know how" message a bug because it creates confusion and looks like a comment that a developer left there. Short term workaround would be to just hide that message.

[Mike Risher (mrisher)]

I went ahead and implemented the suggested short term fix of hiding the "I don't know how" message.

Branch here:
https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/mrisher/lp1848579-interval-columns-filter-message

[Chris Sharp (chrissharp123)]

I think a better approach here for grids including non-implemented filters is to break out the columns into eg-grid-column elements with [filterable]="false". It has the same end result as Mike's patch while availing ourselves of a built-in feature of the eg-grid infrastructure.

[Mike Risher (mrisher)]

Do we want to hide the "I don't know how to filter this" error message, as suggested? Or do we want to take the approach Chris suggested of using eg-grid-column?

[Mike Risher (mrisher)]

(FWIW my suggestion is to remove the lengthy error message.)

[Mike Risher (mrisher)]

A bit more detail on why I'm in favor of suppressing the error message. TLDR - it's quicker and more intuitive.

Let's say we have a grid with 8 columns, one of which is intervals. If we suppress the error message the developer needs to do nothing special when they call the eg-grid directive. Straightforward. If we don't suppress the error message the workaround involves adding eg-grid-column and setting filterable to false. However, a side effect of eg-grid-column is that the column in question now shows up first. We probably don't want the interval shown first. Usually we want name or org first. So now we need to go back to eg-grid, and specify the order in which it shows all the fields. The workaround involves several extra lines of code and might not be obvious to all developers. Suppressing the error message is a quicker more intuitive solution and the end result is the same. Additionally, are there any situations when we'd want the grid to show the error message about not being able to filter intervals?

[Michele Morgan (mmorgan)]

First, I would consider the inability to filter on interval columns to be a bug rather than a wishlist item, and also a regression since the pre-angular interface was able to filter interval columns.

That said, I'm in favor of suppressing the "I don't know how to..." error message, and Mike's approach makes sense to me. With the error message suppressed, the user just sees that the column has no filter widget, which conveys the same thing as the error message, but more clearly by its absence.

Unless there's concensus on a different approach, I'm willing to add my signoff to Mike's patch.

Also, if this patch is accepted, I would open a new bug for the inability to filter columns containing intervals as this bug went in the direction of improving usability for staff users (Yay!) rather than restoring the lost functionality.

Also, since this bug went in the direction of improving usability for staff users (Yay!) rather than restoring the lost functionality, if this patch is accepted I would open a new bug to restore the lost functionality.

[Terran McCanna (tmccanna)]

I agree with Michele that this improves usability for the time-being. My sign off is at:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/mccanna/lp1848579-interval-columns-filter-message-signoff

I've added a new bug for the inability to filter interval columns here:
https://bugs.launchpad.net/evergreen/+bug/1915550

[Galen Charlton (gmc)]

Pushed to master and rel_3_6. Thanks, Mike and Terran!