ember-power-select icon indicating copy to clipboard operation
ember-power-select copied to clipboard

Fix power-select with closeOnSelect=false nested within basic-dropdown

Open jamescdavis opened this issue 3 years ago • 4 comments
trafficstars

Overriding aria-controls on the dropdown trigger causes issues when a power-select is nested within a basic-dropdown and @closeOnSelect={{false}} is set on the power-select. Specifically, the parent dropdown is erroneously closed when an option is selected. This change was introduced in https://github.com/cibernox/ember-power-select/pull/1481/commits/bb1b8e9e10dfa23dd2b362461cb5be4da3a6b7df as part of #1481.

https://github.com/cibernox/ember-basic-dropdown/pull/633 is the ember-basic-dropdown PR that made it not ok to override aria-controls since it uses this to build a relationship between the trigger and content.

Repro: https://github.com/jamescdavis/power-select-nested-dropdown-repro/blob/main/app/templates/application.hbs (this repro uses PowerSelectMultiple but the behavior is the same for PowerSelect)

This PR adds a failing test for the issue along with a couple of smoke tests for nested power-select. It also includes a commit that simply removes the aria-controls override, which makes the failing tests pass and resolves the bug, but I'm sure is not the desired resolution (and obviously makes a11y tests fail). I am as of yet uncertain how to fix this properly.

jamescdavis avatar Mar 02 '22 09:03 jamescdavis

@calvin-fb any ideas on a proper fix?

jamescdavis avatar Mar 03 '22 18:03 jamescdavis

@matthew-robertson tagging you as well

jamescdavis avatar Mar 03 '22 19:03 jamescdavis

@jamescdavis Interesting. I just played around with it a bit and I found that if I pass @renderInPlace={{true}} for the power-select, it also fixes the issue. If you are able to do that with your app, that would be a quick workaround until this gets fixed properly. I'm going to do some digging to find out what is actually going on and if there's a cleaner fix.

calvin-fb avatar Mar 03 '22 19:03 calvin-fb

So the issue is with this block in ember-basic-dropdown: https://github.com/cibernox/ember-basic-dropdown/blob/master/addon/components/basic-dropdown-content.ts#L390-L392 It expects aria-controls to be matched up with ember-basic-dropdown-content. With the accessibility fixes that I made, that's no longer the case. aria-controls is now connected to ember-power-select-options instead of ember-basic-dropdown-content. Here's another relevant line: https://github.com/cibernox/ember-basic-dropdown/blob/master/addon/components/basic-dropdown-content.ts#L352

In terms of fixing it... one hacky fix would be adding ember-basic-dropdown-content at the same level as ember-power-select-options. This is pretty gross though because the ember-basic-dropdown-content class would also be on the parent and if there any styling associated with it, it would look pretty gross. The other option would be making a change in the ember-basic-dropdown repo to make it aware of the differences in ember-power-select (not ideal). The closestContent function would have to be changed to look for either ember-basic-dropdown-content or ember-power-select-options.

calvin-fb avatar Mar 03 '22 20:03 calvin-fb