Clarity: clr-datagrid should not call clr-dgRefresh when it gets Destroyed/Initialized.

[Harsh072]

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

[x ] bug
[ ] feature request
[ ] enhancement

Expected behavior

Hi, I am facing problem with clr-datagrid. It calls clrDgRefresh multiple times whenever gets destroyed. Is it expected behaviour ?

When i did some research, found that it calls clrDgRefresh for all Active Filters whenever gets destroyed.

Small Analysis: clr-datagrid calls unsubscribe when gets destroyed which in-turn calls unregisterFilters which finally calls clrDgRefresh for all active filters.

I believe clrDgRefresh should even not get called when initialize clr-datagrid. This is not directly related to this issue. But please consider this scenario as well.

Actual behavior

clrDgRefresh gets called for all Active filters when clr-datagrid destroys.

Reproduction of behavior

Steps to Reproduce:
1.) Create clr-datagrid(would be good if with custom filter).
2.) In UI, filter on multiple column(you just need to make multiple filters isActive() to return true)
3.) Do something that destroys clr-datagrid(may be with a button)
4.) clrDgRefresh gets called for every filter which has isActive() true.

Templates:

Environment details

• Angular version: 2.0.X

• Clarity version:

• OS and version:

• Browser: [all | Chrome XX | Firefox XX | IE XX | Safari XX | Mobile Chrome XX | Android X.X Web Browser | iOS XX Safari | iOS XX UIWebView | iOS XX WKWebView ]

[youdz]

After investigation, it appears this is blocked by https://github.com/angular/angular/issues/14252.

Because OnDestroy happens _after_ the component has been destroyed and not just before, it means the ngOnDestroy() calls happen bottom-up instead of top-down. Because of that, we have no way to distinguish between a filter destroyed just by itself or the entire datagrid being destroyed. When the linked Angular issue is fixed, I believe this issue will also be fixed automatically without us changing any code.

[gssjr]

@youdz Does this explain why my filters get cleared when the component gets destroyed? This may conflict (or be resolved) with work on #2094 since we want to keep track of state when navigating away from the grid component.

[youdz]

Sadly, this will not be fixed with #2094. What #2094 will do is just offer a single input for the datagrid state, which will either solve or be a good compromise for many requests. The fact that on destroy, the datagrid emits a state without filters/pagination/sorting is something that is truly blocked until Angular fixes the linked issue. We have absolutely no way to handle it on our side. 😢

Your best bet is to have some flag on the component that uses a datagrid, set that flag to true when the event that destroys the datagrid happens (change of route, tab, ...) and ignore any events the datagrid emits when that flag is true.

[youdz]

I took the time to write a quick StackBlitz showing what I mean: https://stackblitz.com/edit/datagrid-track-state?file=app%2Fapp.component.ts

On the application side, your flag is always updated before the "wrong" events fire. So you can just ignore them. The problem is that on our side, we have no way to distinguish the filter itself being destroyed from the whole datagrid being destroyed. So the event will go through every time.

[speti43]

Hi Guys,

When will this be fixed?
I cannot save the filter state for a user, because clrdgrefresh is called on ngOnDestroy, and in that call the clarity calls it without the filter object (sorting paging data is there). There are two solutions for me, call it correctly (with correct ClrDatagridStateInterface object) in case of ondestroy, or don't call it at all.

[youdz]

As mentioned in my previous comment, we cannot fix this until Angular fixes https://github.com/angular/angular/issues/14252.
That issue is more than 18 months old and marked as severity3: broken but the Angular team seems to show no interest in fixing it.

This is out of our hands at the moment, we cannot give you an ETA on a fix for Clarity.

[speti43]

@youdz Understood, sorry I missed the info. It's not a problem for me if it's called on destroy, however the object is not correct in that call. Filter property of ClrDatagridStateInterface is undefined in that call (should not be because I set a one or more filter values) , sort and paging is there. Other clrdgRefresh calls are fine, but somehow in ondestroy filter values have been lost somewhere. If I create a separated defect and a stackblitz, is there any chance that it will be fixed, or you won't have time for this in this century? :)

[youdz]

Your problem is exactly the one posted here, sadly. (clrDgRefresh) is called on destroy because it will always believe the filter has been removed and the datagrid is still in use, so it will always call it with the wrong state. If we knew the correct state (in your case, that the filters do have a value), we wouldn't emit in the first place. 🙂

To give you a better idea, the order of events is:

1. The datagrid is marked for destruction.
2. The filters ngOnDestroy() are called first. They tell the datagrid, which removes them from the state.
3. The state has changed (no more filters), we emit the new state.
4. The datagrid's ngOnDestroy() is finally called.

If the Angular issue I linked was fixed, step 4 would happen before step 2, and we could ignore state changes because we know the entire datagrid is up for destruction.

The workaround until then would be for you to ignore events after scheduling the Datagrid's destruction. If you're willing to create a Stackblitz like you proposed, I'd be happy to update it to show you what I mean with this workaround. 👍

EDIT: Actually, I already posted a StackBlitz showing this in the thread: https://stackblitz.com/edit/datagrid-track-state?file=app%2Fapp.component.ts

[speti43]

Thanks for the clarification and help @youdz
Here is my stackblitz https://stackblitz.com/edit/clr-filter-settings-save
I displayed the filter state, set some filtering and sorting and if you navigate to home by header and back to grid, then you'll see the issue as described previously.
Please update the code with the workaround, to prevent the last call when I change the route, so I can keep the filterstate as well.

[youdz]

I swear I'm going crazy.
Filters on even columns are preserved fine in your StackBlitz, but filters on odd columns are cleared on destroy. Is it just for me?

I'll update with the workaround as soon as I have some more time today.

EDIT: on top of this the destruction in your case happens through routing, so you're even more dependent on the Angular issue I posted above and the workaround becomes dirtier, having to listen to the router to check if that's the source of the destruction. 😞

[speti43]

You observation is perfect, it does the same to me. XD
I have a different idea, I use refresh only because i can get the grid state from there.
If there could be a different source of the grid state that would be wonderful (maybe the best would be a model binding).
What's you opinion @youdz ?
Is there any chance to get the state another way, which is not spoiled by this angular issue?

[youdz]

The two-way binding would work the same way, it's just an input + an output, and we already have the output. We have an open issue (#2094) to add the input, just haven't gotten around to it yet. So that wouldn't solve your problem at all.

So here is your StackBlitz with the workaround, correctly saving the state even with a route change: https://stackblitz.com/edit/clr-filter-settings-save-with-workaround?file=src/app/grid-component/grid-component.component.ts

I subscribe to route changes (I used ResolveEnd but I'm not sure it's the most appropriate one for this, you might want to play around and maybe use a different one) and set a flag to ignore new states when we're destroying because we're moving to a new route. It's just a few lines of code, I made sure to use takeUntil to avoid memory leaks in case you are planning to copy-paste this directly.

[speti43]

Rock solid, works like a charm.
Thank you, tomorrow it will be in production.

[speti43]

Another issue came up with this, I have to preset the filtervalues when we are navigating back to the grid. I can see that we can preset the string filters like this: String filtering
But in this case I have to refactor 100+ fields, which have filter through binding properties:
[clrDgField]="'my.deep.property'"
Is there a possibility to preset those filters which are populated through binding properties?

[youdz]

Thats exactly #2094, which we don't have yet. An external contributor apparently tried to work on it and hit several issues he couldn't solve, so it doesn't look like it's going in in the short term, sorry.

[speti43]

Understood, I just wanted to be sure, this is the only way. Thanks!

[creadicted]

Would it be possible to destory the datagrid manually before making an action and catch that error? I just stumbled over that behaviour.

[whizkidwwe1217]

@youdz Is there any workaround for datagrids in tabs? It seems like clr-dgRefresh is being called first before the two-way bound variable of a tab content is set. I am using the variable that has a binding to clrIfActive directive of a tab content and use that as a flag to disable auto refresh of the datagrid. Unfortunately, clr-dgRefresh is always called first. I have two tab panels with each panel contains a data grid. Switching from one tab to another refreshes both grids and sends two http requests.

[gnomeontherun]

@whizkidwwe1217 Best to open a stack overflow question with a demo of what you're doing rather than get the ticket off topic.

[idris-rampurawala]

Hi Fellas, I was working with Datagrid and found this issue :(
I managed to find a workaround for this being demosntrated in this stackblitz

The idea is to add a debounce in between clrDgRefresh calls and have a flag being unset on component destroy (ngDestroy). This way the extra clrDgRefresh calls are avoided on route change events. Check out the demo!

[mathisscott]

@hippee-lee : Thoughts?

[gnomeontherun]

There is a workaround available for this issue, and we recommend using it for Clarity Angular. As we build Clarity Core implementations, we expect that this issue won’t be an issue. To help us clean up our backlog, we are going to close this with a functional workaround available and suggest you follow updates for Clarity Core for enhancements that can support your use case with Clarity Core components.

[Jinnie]

Logged the workaround/pattern in our support db.