Skip to content

fix(multiselect): scope chevron clicks to the correct container #643

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -692,10 +692,10 @@ export default class ToucanFormMultiselectControlComponent extends Component<Tou
this.args.options && this.args.onFilter
? this.args.onFilter(value)
: this.args.options
? this.args.options.filter((option) => {
return option.toLowerCase().startsWith(value.toLowerCase());
})
: [];
? this.args.options.filter((option) => {
return option.toLowerCase().startsWith(value.toLowerCase());
})
: [];

if (this.filteredOptions?.length > 0) {
this.activeIndex = 0;
Expand All @@ -717,9 +717,10 @@ export default class ToucanFormMultiselectControlComponent extends Component<Tou
* directly on the input tag itself.
*/
@action
handleContainerClick() {
handleContainerClick(event: MouseEvent) {
if (!this.isPopoverOpen) {
const inputElement = document.querySelector(
const container = event.currentTarget as HTMLElement;
const inputElement = container.querySelector(
'[data-toucan-multiselect-input]',
);

Expand Down Expand Up @@ -768,7 +769,7 @@ export default class ToucanFormMultiselectControlComponent extends Component<Tou
@placement="bottom-start"
as |velcro|
>
<div data-multiselect>
<div data-multiselect>
{{! Disabling this rule as the user interacts with the input directly. The click on the div is simply for convenience. }}
{{! template-lint-disable no-invalid-interactive }}
<div
Expand Down Expand Up @@ -902,14 +903,18 @@ export default class ToucanFormMultiselectControlComponent extends Component<Tou
(hash
Option=(component
this.Option
isActive=(this.isEqual generatedIndex this.activeIndex)
isActive=(this.isEqual
generatedIndex this.activeIndex
)
isDisabled=@isDisabled
isSelected=(this.isSelected option)
isReadOnly=@isReadOnly
onClick=this.onChange
onMouseover=(fn this.onOptionMouseover generatedIndex)
popoverId=this.popoverId
index=(if this.isSelectAllEnabled generatedIndex index)
index=(if
this.isSelectAllEnabled generatedIndex index
)
)
option=option
)
Expand Down
89 changes: 86 additions & 3 deletions test-app/tests/integration/components/multiselect-test.gts
Original file line number Diff line number Diff line change
Expand Up @@ -1959,9 +1959,92 @@ module('Integration | Component | Multiselect', function (hooks) {
</:chip>

<:default as |multiselect|>
<multiselect.Option>{{multiselect.option}}</multiselect.Option>
</:default>
</Multiselect>
<multiselect.Option>{{multiselect.option}}</multiselect.Option>
</:default>
</Multiselect>
</template>);
});

test('when multiple multiselect controls are on a page, clicking on a container opens the correct popover', async function (assert) {
await render(
<template>
<div class="first-multiselect">
<Multiselect
@noResultsText="No results"
@options={{testColors}}
data-input="first"
>
<:chip as |chip|>
<chip.Chip>
{{chip.option}}
<chip.Remove @label="Remove" />
</chip.Chip>
</:chip>

<:default as |multiselect|>
<multiselect.Option>{{multiselect.option}}</multiselect.Option>
</:default>
</Multiselect>
</div>

<div class="second-multiselect">
<Multiselect
@noResultsText="No results"
@options={{testColors}}
data-input="second"
>
<:chip as |chip|>
<chip.Chip>
{{chip.option}}
<chip.Remove @label="Remove" />
</chip.Chip>
</:chip>

<:default as |multiselect|>
<multiselect.Option>{{multiselect.option}}</multiselect.Option>
</:default>
</Multiselect>
</div>
</template>,
);

// Create page objects for both multiselects
let firstMultiselect = new MultiselectPageObject('[data-input="first"]');
let secondMultiselect = new MultiselectPageObject('[data-input="second"]');

// Initially, both popovers should be closed
assert.dom(firstMultiselect.list).doesNotExist();
assert.dom(secondMultiselect.list).doesNotExist();

// Click on the container of the second multiselect
await click(secondMultiselect.container as Element);

// The second multiselect's popover should be open
assert.dom(secondMultiselect.list).exists();

// The first multiselect's popover should remain closed
assert.dom(firstMultiselect.list).doesNotExist();

// The second multiselect's input should be focused
assert.dom(secondMultiselect.element).isFocused();

// Close the second multiselect's popover
await triggerKeyEvent(
secondMultiselect.element as Element,
'keydown',
'Escape',
);

// Now click on the container of the first multiselect
await click(firstMultiselect.container as Element);

// The first multiselect's popover should be open
assert.dom(firstMultiselect.list).exists();

// The second multiselect's popover should remain closed
assert.dom(secondMultiselect.list).doesNotExist();

// The first multiselect's input should be focused
assert.dom(firstMultiselect.element).isFocused();
});
});