Clarity: Datagrid: Undefined [clrDgSortBy] makes column unsortable

[jc-white]

Select one ... (check one with "x")

[x] bug
[ ] feature request
[ ] enhancement

Expected behavior

If a value of undefined is passed to [clrDgSortBy], the column becomes unsortable. This might be related to my previous issue #781 and the corresponding fix #809.

To give some background on my use case, I am populating [clrDgSortBy] and [clrDgSortOrder] from parameters passed via the URL. What column the user has sorted by and sort order is unknown, so all columns have to have these properties bound, even if they are undefined. Thus, even if [clrDgSortBy] is undefined but supplied, the column needs to be (re)sortable. I can possibly provide more detailed code as an example if necessary, but the below plunk is the gist of it.

Actual behavior

Column is not sortable if [clrDgSortBy] is supplied but undefined.

Reproduction of behavior

https://plnkr.co/edit/yJEe1xTqCatZ2v4TfrSB?p=preview (Name and Age are not sortable)

Environment details

• Angular version: 4.1.0

• Clarity version: 0.9.3

• OS and version: Windows 10

• Browser: all

[Jinnie]

Hi @jarinwhite,

That's a valid case, IMO, although the concept of dynamic columns is not of my personal taste ☣️
I can give you an idea how to hack it, though, while waiting for a fix from the Clarity Team:
Separate columns iteration from the actual clr-dg-column rendering. Then conditionally create clr-dg-columns with the desired attributes. Clarity expects clr-dg-column to be direct child of the clr-datagrid, though, so you can't use ng-container or any other insignificant element. But you can nest two clr-dg-columns. This leads to stacked paddings, so you need to manually reset the css for one of them.
Here's the demo:
https://plnkr.co/edit/OyCbpa2JLPbMP53VAN8k?p=preview

It's an ugly hack, I admit 😞, but it works.

[Shijir]

Hi @jarinwhite, this is definitely a valid case as @Jinnie suggested. In fact, it was working before the introduction of #809 even though it fixes another issue. This bug has emerged to due to the fact that I've moved @Input("clrDgField") before both @Input("clrDgSortBy") and @Input("clrDgSortOrder"). It would have been adequate and proper if I moved @Input("clrDgField") only before @Input("clrDgSortOrder"). I will work on it and fix the error as soon as possible. I apologize for this one 😞 🙌

[jc-white]

Thank you @Jinnie and @Shijir. This product is a ways off from release, so I can definitely wait until you've got a proper fix in place. Thanks for your hard work.

[youdz]

@Shijir: Moving @Input("clrDgField") to after @Input("clrDgSortBy") is not a correct fix, it will just create the opposite problem where you have no way of overriding a field column's sort with a custom one. It would actually be a worse bug. 😅

What you need to do is slightly more involved:

• In field, you make sure you never overwrite an existing custom comparator from sortBy. That's the behavior we want, sortBy has a higher priority than Field.
• In sortBy, if you are given undefined and there is a field property, then you ignore the input (which means you fallback on the default field comparator). If there is no field property, then you do set the sortBy to undefined, to allow consumers to dynamically change a column from sortable to unsortable.

[jc-white]

Hi folks, don't mean to rush you on this, just wondering if I could get a status update on this bug. Thanks again for looking into it.

[adityarb88]

Hey @jarinwhite. This ticket hasn't been prioritized yet. The teams priorities for the next few weeks is pretty much decided. But we will try to look into this as soon as possible. Thanks for checking in.

[Shijir]

closed by https://github.com/vmware/clarity/pull/1051

[github-actions[bot]]

Hi there 👋, this is an automated message. To help Clarity keep track of discussions, we automatically lock closed issues after 14 days. Please look for another open issue or open a new issue with updated details and reference this one as necessary.