goetzrobin / spartan

Cutting-edge tools powering Angular full-stack development.
https://spartan.ng
MIT License
1.1k stars 121 forks source link

The select control become dirty on multiple mode with initlal value or patch value #224

Closed wizardnet972 closed 1 month ago

wizardnet972 commented 3 months ago

If the select control is in multiple mode and I have an initial value or patch value (without interacting with the control), it incorrectly gets marked as dirty (true). This is incorrect because only when I interact with it should it become dirty, similar to input and other controls.

Which area/package is the issue in?

select

I would be willing to submit a PR to fix this issue

thatsamsonkid commented 3 months ago

@wizardnet972 - just wanted to confirm, will you be opening a PR for this issue? Otherwise I can take a look, let me know.

alexciesielski commented 3 months ago

So I took the liberty to write some unit tests for the brn-select component, before taking a look at the list of issues again, and I found the same bug when writing a test case for initializing it with a default value.

I don't have time to find the underlying issue, but I can at least share the code for the unit tests that I wrote so at least that isn't time wasted. https://pastecode.io/s/xs76uc19

alexciesielski commented 3 months ago

@wizardnet972 any plans on tackling this soon? this is the last bug that prevents me from starting to use the select component instead of the native one

thatsamsonkid commented 3 months ago

I think I'll have some time tonight to look into this. Assigning this issue to myself

thatsamsonkid commented 2 months ago

I apologize I have been ill for the past week or so and then had to catch up on alot of work so I didnt get a chance to open this till now. I added quite a few more tests for select to try and find the issue but I don't think I'm able to replicate the issue in the tests. From what I can tell the form statuses seem to be correct unless I'm missing something. Maybe someone can double check me #278

goetzrobin commented 2 months ago

So I took the liberty to write some unit tests for the brn-select component, before taking a look at the list of issues again, and I found the same bug when writing a test case for initializing it with a default value.

I don't have time to find the underlying issue, but I can at least share the code for the unit tests that I wrote so at least that isn't time wasted. https://pastecode.io/s/xs76uc19

@thatsamsonkid were you able to incorporate these tests also?

thatsamsonkid commented 2 months ago

I did not take directly but wanted to keep all the tests using the testing-library, but I think I captured same scenarios and plus some. I will say looking at the tests alex put together again maybe oddly the changes are not are getting picked up at each step. I didn't force the detect changes like he did, but let me try that and see if I get a different result.

goetzrobin commented 2 months ago

Sounds good. And I do prefer testing library also. I am wondering if there's some CD issues

thatsamsonkid commented 1 month ago

Wanted to just give a small update. I did resolve the issue with the tests not showing this and also now finally found what was causing the issue for this finally. I've just been short on time but hoping will be pushing a PR fix for this in next few days. Apologies it took this long and hopefully we should have a good number of tests to cover these things now