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

support listbox with multiple selection #171

Open
wants to merge 4 commits into
base: master
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
5 changes: 3 additions & 2 deletions ember-headlessui/addon/components/combobox/-option.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,15 @@
<Tag
role='option'
id={{this.guid}}
tabindex='-1'
tabindex={{unless @disabled '-1'}}
{{this.registerOption}}
{{on 'focus' (fn @setActiveOption this)}}
{{on 'mousedown' this.handleMouseDown}}
{{on 'mouseover' (fn @setActiveOption this)}}
{{on 'mouseout' this.handleMouseOut}}
{{on 'click' this.handleOptionClick}}
aria-selected={{if this.isSelectedOption 'true'}}
aria-selected={{if this.isSelectedOption 'true' 'false'}}
aria-disabled={{if @disabled 'true' 'false'}}
data-is-active={{if this.isActiveOption 'true'}}
disabled={{if @disabled true false}}
...attributes
Expand Down
4 changes: 3 additions & 1 deletion ember-headlessui/addon/components/listbox.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
unregisterOptionsElement=this.unregisterOptionsElement
hasLabelElement=this.labelElement
activeOptionGuid=this.activeOptionGuid
selectedOptionGuid=this.selectedOptionGuid
selectedOptionGuids=this.selectedOptionGuids
setActiveOption=this.setActiveOption
unsetActiveOption=this.unsetActiveOption
setSelectedOption=this.setSelectedOption
Expand All @@ -23,6 +23,8 @@
openListbox=this.openListbox
closeListbox=this.closeListbox
handleClickOutside=this.handleClickOutside
multiple=@multiple
selected=@value
)
Button=(component
'listbox/-button'
Expand Down
72 changes: 46 additions & 26 deletions ember-headlessui/addon/components/listbox.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ import Component from '@glimmer/component';
import { tracked } from '@glimmer/tracking';
import { action } from '@ember/object';
import { guidFor } from '@ember/object/internals';
import { debounce } from '@ember/runloop';
import { debounce, scheduleOnce } from '@ember/runloop';

import { TrackedSet } from 'tracked-maps-and-sets';

const ACTIVATE_NONE = 0;
const ACTIVATE_FIRST = 1;
Expand All @@ -28,9 +30,9 @@ export default class ListboxComponent extends Component {
labelElement;
optionsElement;
optionElements = [];
optionValues = {};
optionComponents = [];
search = '';
@tracked selectedOptionIndex;
@tracked selectedOptionIndexes = new TrackedSet();

get activeOptionGuid() {
return this.optionElements[this.activeOptionIndex]?.id;
Expand All @@ -40,8 +42,10 @@ export default class ListboxComponent extends Component {
return !!this.args.disabled;
}

get selectedOptionGuid() {
return this.optionElements[this.selectedOptionIndex]?.id;
get selectedOptionGuids() {
return Array.from(this.selectedOptionIndexes).map(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

way make an array that ends up being a separate array each access?
does this need to be a getter?
can we access the optionElements directly?

(i) => this.optionElements[i]?.id
);
}

get isOpen() {
Expand All @@ -51,9 +55,8 @@ export default class ListboxComponent extends Component {
set isOpen(isOpen) {
if (isOpen) {
this.activeOptionIndex = undefined;
this.selectedOptionIndex = undefined;
this.selectedOptionIndexes.clear();
this.optionElements = [];
this.optionValues = {};
this._isOpen = true;
} else {
this._isOpen = false;
Expand Down Expand Up @@ -132,7 +135,9 @@ export default class ListboxComponent extends Component {
this.activateBehaviour = ACTIVATE_FIRST;
if (this.isOpen) {
this.setSelectedOption(event.target, event);
this.isOpen = false;
if (!this.args.multiple) {
this.isOpen = false;
}
} else {
this.isOpen = true;
}
Expand Down Expand Up @@ -173,23 +178,26 @@ export default class ListboxComponent extends Component {
@action
registerOptionElement(optionComponent, optionElement) {
this.optionElements.push(optionElement);
this.optionValues[optionComponent.guid] = optionComponent.args.value;

// store the index at which the option appears in the list
// so we can avoid a O(n) find operation later
optionComponent.index = this.optionElements.length - 1;
optionElement.setAttribute('data-index', this.optionElements.length - 1);
let index = this.optionElements.length - 1;
optionComponent.index = index;
optionElement.setAttribute('data-index', index);

if (this.args.value) {
if (this.args.value === optionComponent.args.value) {
this.selectedOptionIndex = this.activeOptionIndex =
this.optionElements.length - 1;
this.optionComponents[index] = optionComponent;

this.scrollIntoView(optionElement);
}
}
scheduleOnce('afterRender', this, this.setDefaultActiveOption);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we avoid hacking runloop usage?

runloop is kinda anti-patterny and causes performance problems

}

if (!this.selectedOptionIndex) {
setDefaultActiveOption() {
let selectedIndexes = this.optionComponents
.filter((o) => o.isSelected)
.map((o) => o.index);

this.selectedOptionIndexes = new TrackedSet(selectedIndexes);

if (this.selectedOptionIndexes.size === 0) {
switch (this.activateBehaviour) {
case ACTIVATE_FIRST:
this.setFirstOptionActive();
Expand All @@ -198,6 +206,9 @@ export default class ListboxComponent extends Component {
this.setLastOptionActive();
break;
}
} else {
this.activeOptionIndex = Math.min(...this.selectedOptionIndexes);
this.scrollIntoView(this.optionElements[this.activeOptionIndex]);
}
}

Expand Down Expand Up @@ -229,8 +240,7 @@ export default class ListboxComponent extends Component {
optionValue = optionComponent.args.value;
optionIndex = optionComponent.index;
} else if (this.activeOptionIndex !== undefined) {
optionValue =
this.optionValues[this.optionElements[this.activeOptionIndex].id];
optionValue = this.optionComponents[this.activeOptionIndex].args.value;
optionIndex = parseInt(
this.optionElements[this.activeOptionIndex].getAttribute('data-index')
);
Expand All @@ -239,13 +249,23 @@ export default class ListboxComponent extends Component {
}

if (!this.optionElements[optionIndex].hasAttribute('disabled')) {
this.selectedOptionIndex = optionIndex;

if (this.args.onChange) {
this.args.onChange(optionValue);
if (this.args.multiple) {
let value = this.args.value ?? [];

if (this.selectedOptionIndexes.has(optionIndex)) {
optionValue = value.filter((i) => i !== optionValue);
this.selectedOptionIndexes.delete(optionIndex);
} else {
optionValue = [...value, optionValue];
this.selectedOptionIndexes.add(optionIndex);
}
} else {
this.selectedOptionIndexes.add(optionIndex);
}

if (e.type === 'click') {
this.args.onChange?.(optionValue);

if (e.type === 'click' && !this.args.multiple) {
this.isOpen = false;
}
} else {
Expand Down
9 changes: 5 additions & 4 deletions ember-headlessui/addon/components/listbox/-option.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,21 @@
<Tag
role='option'
id={{this.guid}}
tabindex='-1'
tabindex={{unless @disabled '-1'}}
{{this.registerOption}}
{{on 'focus' (fn @setActiveOption this)}}
{{on 'mouseover' (fn @setActiveOption this)}}
{{on 'mouseout' @unsetActiveOption}}
{{on 'click' this.handleClick}}
aria-selected={{if this.isSelectedOption 'true'}}
aria-selected={{if this.isSelected 'true' 'false'}}
aria-disabled={{if @disabled 'true' 'false'}}
disabled={{if @disabled true false}}
...attributes
>
{{yield
(hash
active=this.isActiveOption
selected=this.isSelectedOption
active=this.isActive
selected=this.isSelected
disabled=(if @disabled true false)
)
}}
Expand Down
15 changes: 12 additions & 3 deletions ember-headlessui/addon/components/listbox/-option.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,20 @@ export default class ListboxOptionComponent extends Component {
this.args.setSelectedOption(this, e);
}

get isActiveOption() {
get isActive() {
return this.args.activeOptionGuid == this.guid;
}

get isSelectedOption() {
return this.args.selectedOptionGuid == this.guid;
get isSelected() {
if (this.args.multiple) {
let selected = this.args.selected ?? [];
return selected.includes(this.args.value);
} else {
return (
// allow 0 and null to as possible values
this.args.selected !== undefined &&
this.args.selected === this.args.value
);
}
}
}
5 changes: 4 additions & 1 deletion ember-headlessui/addon/components/listbox/-options.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
}}
aria-activedescendant={{@activeOptionGuid}}
aria-orientation='vertical'
aria-multiselectable={{if @multiple 'true'}}
{{this.registerOptions}}
{{on 'keypress' @handleKeyPress}}
{{on 'keydown' @handleKeyDown}}
Expand All @@ -28,10 +29,12 @@
'listbox/-option'
registerOptionElement=@registerOptionElement
activeOptionGuid=@activeOptionGuid
selectedOptionGuid=@selectedOptionGuid
selectedOptionGuids=@selectedOptionGuids
setActiveOption=@setActiveOption
unsetActiveOption=@unsetActiveOption
setSelectedOption=@setSelectedOption
multiple=@multiple
selected=@selected
)
)
}}
Expand Down
24 changes: 24 additions & 0 deletions test-app/app/controllers/listbox/listbox-multiple.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { tracked } from '@glimmer/tracking';
import Controller from '@ember/controller';
import { action } from '@ember/object';

const PEOPLE = [
{ id: 1, name: 'Durward Reynolds', unavailable: false },
{ id: 2, name: 'Kenton Towne', unavailable: false },
{ id: 3, name: 'Therese Wunsch', unavailable: false },
{ id: 4, name: 'Benedict Kessler', unavailable: true },
{ id: 5, name: 'Katelyn Rohan', unavailable: false },
];

export default class ListboxWithTransitionController extends Controller {
@tracked selectedPeople = [PEOPLE[1]];

get people() {
return PEOPLE;
}

@action
setSelectedPeople(people) {
this.selectedPeople = people;
}
}
1 change: 1 addition & 0 deletions test-app/app/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ Router.map(function () {
this.route('listbox', function () {
this.route('listbox-basic');
this.route('listbox-with-transition');
this.route('listbox-multiple');
});

this.route('combobox', function () {
Expand Down
7 changes: 6 additions & 1 deletion test-app/app/templates/index.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@
Listbox with Transition
</LinkTo>
</li>
<li>
<LinkTo @route='listbox.listbox-multiple'>
Listbox Multiple
</LinkTo>
</li>
</ul>
</li>
<li>
Expand Down Expand Up @@ -124,4 +129,4 @@
</li>
</ul>
</div>
</div>
</div>
Loading