search

ghiscoding / slickgrid-universal

Slickgrid-Universal is a monorepo which includes all Editors, Filters, Extensions, Services related to SlickGrid usage and is also Framework Agnostic
https://ghiscoding.github.io/slickgrid-universal/
Other
89 stars 29 forks source link

New DateFilter's clear does not remove the components value or call stateChange #1544

Closed zewa666 closed 5 months ago

zewa666 commented 5 months ago

Describe the bug

I'm updating to Angular v18 and latest Angular-Slickgrid v8. During that I found that it seems there is a bug with clearing the value from the new VanillaJS datepicker.

besides not unsetting the components actual internal value, it also never calls a potential state change event, which I typically use to persist the selection/filters in localStorage

I'll check into that and see what might be the cause

Reproduction

Which Framework are you using?

Angular

Environment Info

| Executable          | Version |
| ------------------- | ------- |
| (framework used)    | Angular (latest) |
| Slickgrid-Universal | latest |
| TypeScript          | VERSION |
| Browser(s)          | VERSION |
| System OS           | VERSION |

Validations

zewa666 commented 5 months ago

so it seems one part that is missing is a call to

this.calendarInstance!.update({
  dates: true,
});

which somewhat sounds like a poormans change detection :)

Now the control is internally reset at least. But that still doesn't trigger the state change event

zewa666 commented 5 months ago

further more adding a this.onTriggerEvent(new Event("keyup")); would at least now enter the state change handler, but it still shows the old value.

ghiscoding commented 5 months ago

I'm not sure that I completely understand the issue, can you maybe add an animated gif of the problem?

and yes the .update() is what they use, it's a little weird that we need this at all but yeah, we need to go with it. For example I use it in the Date Editor to change the picker value but I guess I didn't use it in Date Filter

https://github.com/ghiscoding/slickgrid-universal/blob/21d8d4c4bd5d2c4e1fe03bbab853a8d81637f955/packages/common/src/editors/dateEditor.ts#L204-L221

from what I've tried in Example 4 which has a console log of Grid State changes, I see it working as expected from my point of view

msedge_bXTkU2ap4C

ghiscoding commented 5 months ago

ohhh I just noticed your repro info above, so you're using the Backspace key, that's funny because there wasn't any key events associated, I added that myself in the Date Filter/Editor. It looks like the clear(true) might be missing something, is that where you were editing the code?

https://github.com/ghiscoding/slickgrid-universal/blob/21d8d4c4bd5d2c4e1fe03bbab853a8d81637f955/packages/common/src/filters/dateFilter.ts#L143-L148

EDIT

ok I understand what happens, I have code in place to not send 2 onFilterCleared event, I let the header menu send the clear event and not the clear() function to avoid duplicate events. However when we do it through keyboard, since it's not called by the header menu, then the event is not registered. There's also another condition that when "Clear all Filters" is triggered, I didn't wanted to call multiple clear event for each filter, I just wanted 1 clear all event in Grid State changed.

So I'll have to think of doing this in a better way, that might require changes in a lot of places (but might be necessary), I'll take a look over the weekend. It seems that I do this clear technique that possible doesn't trigger a State change in other filters as well like the Autocomplete

zewa666 commented 5 months ago

well we're actually building a small clear button that gets overlayed in the right side of the input field as this is what users are used to. so with flatpicker I merely called this.clear and it worked.

I can perhaps try to recreate that part if its helpful but the backspace key essentially behaved the same way

ghiscoding commented 5 months ago

I'm a little surprised that it worked with Flatpickr considering the code I've seen today, but anyway I'll rework the code in the weekend. Actually the reason Flatpickr was working is maybe because it sent value changed event and in my case I specifically call clear(true) which is not a value changed event and doesn't trigger the same event in my project (a value change always trigger an event but a clear doesn't always for the reasons I mentioned above). You don't need to dig deeper, I know what is happening and I'll fix it, no worries

Yup taking a look at Flatpickr's code, that's exactly what they've done, a clear was triggering a change event and that's why it worked. Perhaps I could do the same with Backspace

image

ghiscoding commented 5 months ago

to simulate a change event like Flatpickr does, I could do the following changes and it does trigger the State change and also removes the date from picker. BTW, for a real test you should use a different month (even year) to make sure that the picker is reset to current year/month. I might just go with that, that's an easy workaround solution

image

msedge_K5eSgA0fCn