Different behavior (selecting the checkbox VS clicking the label) with multiple instances on the same page

[stoberov]

Describe the bug

I'm currently using version 1.16.0 and have a page with 2 DropdownTreeSelect instances on it.
Dropdown 1 works as expected. Dropdown 2, however, works as expected only when selecting the checkboxes - if you clicked any of the Dropdown 2 labels, the change affects items in Dropdown 1 instead.

I've noticed that the behavior is not present with older version of the module - 1.10.0 and below. So I'm guessing it is introduced recently, or there is some extra prop that needs to be passed these days.

To Reproduce

Sample: https://codesandbox.io/embed/qlzpr7lzmj?fontsize=14

If you select A, B or C (either checkbox or label) - all good.
If you select D, E or F by checkbox - all good.
If you select D, E or F by clicking on the label (label = the actual letter visible on the screen, to the right of the checkbox), this will select A, B or C instead.

Expected behavior
Clicking on D / E / F labels should select D / E / F. It should not select A / B / C.

[mrchief]

I think this id may not be unique across instances.

[mrchief]

I think we can simply get rid of the htmlFor attribute as the input is nested. It may throw some accessibility warning but it's not needed. That change was introduced later and that'd explain why older versions work properly.

[mrchief]

See https://github.com/evcohen/eslint-plugin-jsx-a11y/blob/master/docs/rules/label-has-for.md

To fully cover 100% of assistive devices, you're encouraged to validate for both nesting and id.

But like I said, we can either remove it or generate it using some combination that is both unique and deterministic. node.id may be a good candidate and if not, I think removing it for now will also work.

[stoberov]

I think we can simply get rid of the htmlFor attribute as the input is nested. It may throw some accessibility warning but it's not needed. That change was introduced later and that'd explain why older versions work properly.

@mrchief, thanks for the speedy reply!
Indeed, I can confirm that removing htmlFor (did it manually in my node_modules folder) seem to fix the issue.
It would be wonderful if either this or that other idea for node.id flows into a new release.

[mrchief]

It would be wonderful if either this or that other idea for node.id flows into a new release.

Wanna send me a PR?

[stoberov]

Alright, it's been a while since my last PR, hope that went satisfactory.
Note, there is this CSS file import that I did not see in the repository - hence, I had to comment it out before I was even able to build the project.
Also, opted for the simpler approach of removing htmlFor, rather than an opinionated approach for generating the ID.

[mrchief]

@stoberov Thanks for the quick PR.

...rather than an opinionated approach for generating the ID.

It's not a question of whether we're being opinionated or not. This id is for browsers' and assitive devices' internal use only.

My question is whether node.id guarantees uniqueness or not. If the data doesn't have id in the nodes, then the component generates one for them.

As I was writing this, I realized that the default id is not unique across instances. So let's go with removing the htmlFor attribute.

[mrchief]

@allcontributors[bot] please add @stoberov for code, bug

[allcontributors]

@mrchief

I've put up a pull request to add @stoberov! 🎉