From d94d6bd84a0cabb056d4f769e3f8b4215cc8275e Mon Sep 17 00:00:00 2001 From: Thijs Louisse Date: Fri, 16 Apr 2021 16:00:19 +0200 Subject: [PATCH] fix(listbox): teleported options compatible with map/repeat --- .changeset/calm-pots-tell.md | 5 + packages/listbox/src/ListboxMixin.js | 43 ++++++- .../listbox/test-suites/ListboxMixin.suite.js | 109 +++++++++++++++--- 3 files changed, 134 insertions(+), 23 deletions(-) create mode 100644 .changeset/calm-pots-tell.md diff --git a/.changeset/calm-pots-tell.md b/.changeset/calm-pots-tell.md new file mode 100644 index 000000000..c58f09052 --- /dev/null +++ b/.changeset/calm-pots-tell.md @@ -0,0 +1,5 @@ +--- +'@lion/listbox': patch +--- + +teleported options compatible with map/repeat for listbox/combobox/select-rich diff --git a/packages/listbox/src/ListboxMixin.js b/packages/listbox/src/ListboxMixin.js index aa2296a15..38a9974ca 100644 --- a/packages/listbox/src/ListboxMixin.js +++ b/packages/listbox/src/ListboxMixin.js @@ -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 + * + * + * + * # desired end state + * + *
+ * + *
+ *
+ * @param {HTMLElement} source host of ShadowRoot with default + * @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); } @@ -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); }); } } diff --git a/packages/listbox/test-suites/ListboxMixin.suite.js b/packages/listbox/test-suites/ListboxMixin.suite.js index 36e908d86..72ec2de52 100644 --- a/packages/listbox/test-suites/ListboxMixin.suite.js +++ b/packages/listbox/test-suites/ListboxMixin.suite.js @@ -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'; @@ -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] */ @@ -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` ${option} `, + )} + + <${tag} id="withRepeat"> + ${repeat( + this.options, + option => option, + option => html` ${option} `, + )} + + `; + } + } + + 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``)); + + 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; + }); + }); }); }