Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(pickers): ColumnPicker/GridMenu with 2 independent grids #500

Merged
merged 1 commit into from
May 19, 2020

Conversation

ghiscoding
Copy link
Collaborator

@ghiscoding ghiscoding commented May 18, 2020

  • ColumnPicker & GridMenu Picker should work independently
  • There was an issue with the independence of each picker because the checkbox labels were sharing the exact same label text (because both grids had exact same column headers) and that caused side effect that clicking on bottom grid checkbox picker would actually change the 1st grid instead of 2nd grid and vice versa... that was caused by checkbox label not being unique
  • add uniqueness by adding the grid UID to each checkbox labels
  • this bug impacted both the ColumnPicker & the GridMenu and was only happening when having tow or more grids in the same page with same column headers

TODO

  • modified Example with 2 Grids to test it all out
  • add Cypress E2E tests to cover all possibilities
  • tested in my libs as well

This bug was hard to find, I thought I had a Singleton issue but it turns out to simply be that each Checkbox Label must be unique in their names. Since both grids had the exact same header titles, the checkbox labels (not the checkbox itself, just its associated label) were being shared in both pickers and was causing the issue which can be seen in the animated gif (you can see the checkbox themselves are ok, but their associated labels were problematic).

This last fix is looking good on my side, I did not find any other issues in my libs.

The BUG (before the fix) can be seen below

3t1SQQgDDm

- ColumnPicker & GridMenu Picker should work independently
- There was an issue with the independence of each picker because the checkbox labels were sharing the exact same label text (because both grids had exact same column headers) and that caused side effect that clicking on bottom grid checkbox picker would actually change the 1st grid instead of 2nd grid and vice versa... that was caused by checkbox label not being unique
- add uniqueness by adding the grid UID to each checkbox labels
@ghiscoding
Copy link
Collaborator Author

@6pac
I tested all of my libs and that is the last bug I found. So after this is merged, I would be ok with a new release. Also note that Ataft also created a PR to add a new Example (which I asked for).

Cheers

@6pac 6pac merged commit a7af294 into master May 19, 2020
@ghiscoding ghiscoding deleted the bugfix/pickers-singleton branch May 19, 2020 02:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants