Skip to content

Commit

Permalink
fix(listbox): teleported options compatible with map/repeat
Browse files Browse the repository at this point in the history
  • Loading branch information
tlouisse committed Apr 16, 2021
1 parent 64475a5 commit d94d6bd
Show file tree
Hide file tree
Showing 3 changed files with 134 additions and 23 deletions.
5 changes: 5 additions & 0 deletions .changeset/calm-pots-tell.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@lion/listbox': patch
---

teleported options compatible with map/repeat for listbox/combobox/select-rich
43 changes: 37 additions & 6 deletions packages/listbox/src/ListboxMixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,41 @@ import { LionOptions } from './LionOptions.js';
* @typedef {import('@lion/form-core/types/FormControlMixinTypes.js').ModelValueEventDetails} ModelValueEventDetails
*/

// TODO: consider adding methods below to @lion/helpers

/**
* Sometimes, we want to provide best DX (direct slottables) and be accessible
* at the same time.
* In the first example below, we need to wrap our options in light dom in an element with
* [role=listbox]. We could achieve this via the second example, but it would affect our
* public api negatively. not allowing us to be forward compatible with the AOM spec:
* https://wicg.github.io/aom/explainer.html
* With this method, it's possible to watch elements in the default slot and move them
* to the desired target (the element with [role=listbox]) in light dom.
*
* @example
* # desired api
* <sel-ect>
* <opt-ion></opt-ion>
* </sel-ect>
* # desired end state
* <sel-ect>
* <div role="listbox" slot="lisbox">
* <opt-ion></opt-ion>
* </div>
* </sel-ect>
* @param {HTMLElement} source host of ShadowRoot with default <slot>
* @param {HTMLElement} target the desired target in light dom
*/
function moveDefaultSlottablesToTarget(source, target) {
Array.from(source.childNodes).forEach((/** @type {* & Element} */ c) => {
const isNamedSlottable = c.hasAttribute && c.hasAttribute('slot');
if (!isNamedSlottable) {
target.appendChild(c);
}
});
}

function uuid() {
return Math.random().toString(36).substr(2, 10);
}
Expand Down Expand Up @@ -821,13 +856,9 @@ const ListboxMixinImplementation = superclass =>
);

if (slot) {
slot.assignedNodes().forEach(node => {
this._listboxNode.appendChild(node);
});
moveDefaultSlottablesToTarget(this, this._listboxNode);
slot.addEventListener('slotchange', () => {
slot.assignedNodes().forEach(node => {
this._listboxNode.appendChild(node);
});
moveDefaultSlottablesToTarget(this, this._listboxNode);
});
}
}
Expand Down
109 changes: 92 additions & 17 deletions packages/listbox/test-suites/ListboxMixin.suite.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import '@lion/core/differentKeyEventNamesShimIE';
import { repeat, LitElement } from '@lion/core';
import { Required } from '@lion/form-core';
import { LionOptions } from '@lion/listbox';
import '@lion/listbox/define';
Expand All @@ -24,23 +25,6 @@ function mimicKeyPress(el, key) {
el.dispatchEvent(new KeyboardEvent('keyup', { key }));
}

// /**
// * @param {LionListbox} lionListboxEl
// */
// function getProtectedMembers(lionListboxEl) {
// // @ts-ignore protected members allowed in test
// const {
// _inputNode: input,
// _activeDescendantOwnerNode: activeDescendantOwner,
// _listboxNode: listbox,
// } = lionListboxEl;
// return {
// input,
// activeDescendantOwner,
// listbox,
// };
// }

/**
* @param { {tagString?:string, optionTagString?:string} } [customConfig]
*/
Expand Down Expand Up @@ -1384,5 +1368,96 @@ export function runListboxMixinSuite(customConfig = {}) {
expect(clickSpy).to.not.have.been.called;
});
});

describe('Dynamically adding options', () => {
class MyEl extends LitElement {
constructor() {
super();
/** @type {string[]} */
this.options = ['option 1', 'option 2'];
}

clearOptions() {
/** @type {string[]} */
this.options = [];
this.requestUpdate();
}

addOption() {
this.options.push(`option ${this.options.length + 1}`);
this.requestUpdate();
}

get withMap() {
return /** @type {LionListbox} */ (this.shadowRoot?.querySelector('#withMap'));
}

get withRepeat() {
return /** @type {LionListbox} */ (this.shadowRoot?.querySelector('#withRepeat'));
}

get registrationComplete() {
return Promise.all([
this.withMap.registrationComplete,
this.withRepeat.registrationComplete,
]);
}

render() {
return html`
<${tag} id="withMap">
${this.options.map(
option => html` <lion-option .choiceValue="${option}">${option}</lion-option> `,
)}
</${tag}>
<${tag} id="withRepeat">
${repeat(
this.options,
option => option,
option => html` <lion-option .choiceValue="${option}">${option}</lion-option> `,
)}
</${tag}>
`;
}
}

customElements.define('my-el', MyEl);

it('works with array map and repeat directive', async () => {
const choiceVals = (/** @type {LionListbox} */ elm) =>
elm.formElements.map(fel => fel.choiceValue);
const insideListboxNode = (/** @type {LionListbox} */ elm) =>
// @ts-ignore [allow-protected] in test
elm.formElements.filter(fel => elm._listboxNode.contains(fel)).length ===
elm.formElements.length;

const el = /** @type {MyEl} */ (await _fixture(html`<my-el></my-el>`));

expect(choiceVals(el.withMap)).to.eql(el.options);
expect(el.withMap.formElements.length).to.equal(2);
expect(insideListboxNode(el.withMap)).to.be.true;
expect(choiceVals(el.withRepeat)).to.eql(el.options);
expect(el.withRepeat.formElements.length).to.equal(2);
expect(insideListboxNode(el.withRepeat)).to.be.true;

el.addOption();
await el.updateComplete;
expect(choiceVals(el.withMap)).to.eql(el.options);
expect(el.withMap.formElements.length).to.equal(3);
expect(insideListboxNode(el.withMap)).to.be.true;
expect(choiceVals(el.withRepeat)).to.eql(el.options);
expect(el.withRepeat.formElements.length).to.equal(3);
expect(insideListboxNode(el.withRepeat)).to.be.true;

el.clearOptions();
await el.updateComplete;
expect(choiceVals(el.withMap)).to.eql(el.options);
expect(el.withMap.formElements.length).to.equal(0);
expect(insideListboxNode(el.withMap)).to.be.true;
expect(choiceVals(el.withRepeat)).to.eql(el.options);
expect(el.withRepeat.formElements.length).to.equal(0);
expect(insideListboxNode(el.withRepeat)).to.be.true;
});
});
});
}

0 comments on commit d94d6bd

Please sign in to comment.