carbon-components-svelte icon indicating copy to clipboard operation
carbon-components-svelte copied to clipboard

Published 20 hours ago • verified publisher icon carbon-design-system

fix: remove pointer-events style from multiselect dropdown

Open jorgebv opened this issue 2 years ago • 4 comments

Resolves #1647 This makes the component more consistent with other components that have the dropdown, and also more consistent with the flagship react library.

jorgebv avatar Feb 08 '23 14:02 jorgebv

Adding some context here – looking at the Git blame, this hotfix was added in https://github.com/carbon-design-system/carbon-components-svelte/pull/954 to fix an odd behavior where clicking the icon for a filterable multiselect would double trigger the open behavior.

Agreed that it should be more consistent – is it possible to resolve the previous bug without the pointer-events workaround?

metonym avatar Feb 08 '23 16:02 metonym

Thanks for the context; I should have thought to look at the blame myself. I will look at the issue again with this context in mind as you suggest.

edit: after some more testing I see that my original tests were not performed correctly; this PR does re-introduce the mentioned bug. will mark the PR as draft while I look into it more.

jorgebv avatar Feb 08 '23 16:02 jorgebv

@metonym I found that the primary cause of #940 was the focus handler of the ListBoxField, which would open the MultiSelect. The pointer events of the ListBoxMenuIcon would then immediately close it.

The flagship react MultiSelect doesn't seem to open on focus, so I removed this behavior from the svelte. Maybe this is a breaking change.

Removing the focus on open caused typing to get lost in the MultiSelect, so I added on:input to the input handler, similar to ComboBox and other components. The on:input handler I have implemented for this MultiSelect is a little basic compared to the ComboBox because I didn't quite see what the purpose of everything the ComboBox is doing on:input. Maybe the on:input handler in MultiSelect is insufficient and needs to be more similar to ComboBox. Feedback welcome.

I also added a little bit of styling for when the input is focused, to get the highlighting of the filterable MultiSelect which was missing before.

This became a little bigger than I wanted it to and a little outside my comfort zone. Maybe some of my approach is undesirable. Please let me know if you see any problems with the overall approach.

jorgebv avatar Feb 08 '23 19:02 jorgebv

@metonym / @theetrain Sorry to nag on a PR, but do you think this PR has a future? If there's any part of it we want to move forward I'm happy to continue working on it. But if it doesn't seem like the direction of the PR is going to work out, I can close it.

Thanks for volunteering your time together with the community.

jorgebv avatar Mar 27 '23 13:03 jorgebv