diff --git a/.changeset/poor-swans-hope.md b/.changeset/poor-swans-hope.md new file mode 100644 index 000000000..804216473 --- /dev/null +++ b/.changeset/poor-swans-hope.md @@ -0,0 +1,5 @@ +--- +'@lion/listbox': patch +--- + +Always scrollIntoView and let the scrollIntoView method handle redundancy instead of implementing a "naive" isInView method. diff --git a/packages/listbox/src/ListboxMixin.js b/packages/listbox/src/ListboxMixin.js index f3f9c96c9..1e12d051d 100644 --- a/packages/listbox/src/ListboxMixin.js +++ b/packages/listbox/src/ListboxMixin.js @@ -57,31 +57,6 @@ function uuid() { return Math.random().toString(36).substr(2, 10); } -/** - * @param {HTMLElement} container - * @param {HTMLElement} element - * @param {Boolean} [partial] - */ -function isInView(container, element, partial = false) { - const cTop = container.scrollTop; - const cBottom = cTop + container.clientHeight; - const eTop = element.offsetTop; - const eBottom = eTop + element.clientHeight; - const isTotal = eTop >= cTop && eBottom <= cBottom; - let isPartial; - - if (partial === true) { - isPartial = (eTop < cTop && eBottom > cTop) || (eBottom > cBottom && eTop < cBottom); - } else if (typeof partial === 'number') { - if (eTop < cTop && eBottom > cTop) { - isPartial = ((eBottom - cTop) * 100) / element.clientHeight > partial; - } else if (eBottom > cBottom && eTop < cBottom) { - isPartial = ((cBottom - eTop) * 100) / element.clientHeight > partial; - } - } - return isTotal || isPartial; -} - /** * @type {ListboxMixin} * @param {import('@open-wc/dedupe-mixin').Constructor} superclass @@ -714,6 +689,17 @@ const ListboxMixinImplementation = superclass => this._listboxNode.focus(); } + /** + * @param {HTMLElement} el element + * @param {HTMLElement} [scrollTargetEl] container + * @protected + * @description allow Subclassers to adjust the animation: like non smooth behavior, different timing etc. + */ + // eslint-disable-next-line class-methods-use-this, no-unused-vars + _scrollIntoView(el, scrollTargetEl) { + el.scrollIntoView({ behavior: 'smooth', block: 'nearest' }); + } + /** @private */ __setupEventListeners() { this._listboxNode.addEventListener( @@ -752,9 +738,8 @@ const ListboxMixinImplementation = superclass => return; } this._activeDescendantOwnerNode.setAttribute('aria-activedescendant', el.id); - if (!isInView(this._scrollTargetNode, el)) { - el.scrollIntoView({ behavior: 'smooth', block: 'nearest' }); - } + + this._scrollIntoView(el, this._scrollTargetNode); } /** diff --git a/packages/listbox/test-suites/ListboxMixin.suite.js b/packages/listbox/test-suites/ListboxMixin.suite.js index 47ea375b1..653c6b99e 100644 --- a/packages/listbox/test-suites/ListboxMixin.suite.js +++ b/packages/listbox/test-suites/ListboxMixin.suite.js @@ -3,7 +3,7 @@ import { repeat, LitElement } from '@lion/core'; import { Required } from '@lion/form-core'; import { LionOptions } from '@lion/listbox'; import '@lion/listbox/define'; -import { expect, fixture as _fixture, defineCE } from '@open-wc/testing'; +import { expect, aTimeout, nextFrame, fixture as _fixture, defineCE } from '@open-wc/testing'; import { html, unsafeStatic } from 'lit/static-html.js'; import sinon from 'sinon'; @@ -319,6 +319,60 @@ export function runListboxMixinSuite(customConfig = {}) { expect(el.checkedIndex).to.equal(1); expect(el.serializedValue).to.equal('hotpink'); }); + + it('scrolls active element into view when necessary, takes into account sticky/fixed elements', async () => { + const el = await fixture(html` +
+
Header 1
+
+
Header 2
+ <${tag} id="color" name="color" has-no-default-selected> + <${optionTag} style="height: 50px; scroll-margin-top: 150px;" .choiceValue=${'red'}>Red + <${optionTag} style="height: 50px; scroll-margin-top: 150px;" .choiceValue=${'hotpink'}>Hotpink + <${optionTag} style="height: 50px; scroll-margin-top: 150px;" .choiceValue=${'teal'}>Teal + <${optionTag} style="height: 50px; scroll-margin-top: 150px;" .choiceValue=${'1'}>1 + <${optionTag} style="height: 50px; scroll-margin-top: 150px;" .choiceValue=${'2'}>2 + <${optionTag} style="height: 50px; scroll-margin-top: 150px;" .choiceValue=${'3'}>3 + <${optionTag} style="height: 50px; scroll-margin-top: 150px;" .choiceValue=${'4'}>4 + <${optionTag} style="height: 50px; scroll-margin-top: 150px;" .choiceValue=${'5'}>5 + <${optionTag} style="height: 50px; scroll-margin-top: 150px;" .choiceValue=${'5'}>6 + <${optionTag} style="height: 50px; scroll-margin-top: 150px;" .choiceValue=${'6'}>7 + +
+
+ `); + const listboxEl = /** @type {LionListbox} */ (el.querySelector('#color')); + + // Skip test if listbox cannot receive focus, e.g. in combobox + // Skip test if the listbox is controlled by overlay system, + // since these overlays "overlay" any fixed/sticky items, meaning + // the active item will not be hidden by them. + // @ts-ignore allow protected members in tests + if (listboxEl._listboxReceivesNoFocus || listboxEl._overlayCtrl) { + return; + } + + Object.defineProperty(listboxEl, '_scrollTargetNode', { + get: () => el, + }); + + const thirdOption = /** @type {LionOption} */ (listboxEl.formElements[2]); + const lastOption = /** @type {LionOption} */ ( + listboxEl.formElements[listboxEl.formElements.length - 1] + ); + await listboxEl.updateComplete; + await nextFrame(); + + // Scroll to last option and wait for browser scroll animation (works) + lastOption.active = true; + await aTimeout(1000); + + thirdOption.active = true; + await aTimeout(1000); + + // top should be offset 2x40px (sticky header elems) instead of 0px + expect(el.scrollTop).to.equal(116); + }); }); describe('Accessibility', () => { diff --git a/packages/listbox/types/ListboxMixinTypes.d.ts b/packages/listbox/types/ListboxMixinTypes.d.ts index 568135daa..700929be4 100644 --- a/packages/listbox/types/ListboxMixinTypes.d.ts +++ b/packages/listbox/types/ListboxMixinTypes.d.ts @@ -63,6 +63,8 @@ export declare class ListboxHost { protected _listboxOnKeyUp(ev: KeyboardEvent): void; + protected _scrollIntoView(el: HTMLElement, scrollTargetEl: HTMLElement | undefined): void; + protected _setupListboxNode(): void; protected _teardownListboxNode(): void; diff --git a/yarn.lock b/yarn.lock index 8ad1757c9..38d7fab80 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1701,7 +1701,7 @@ "@open-wc/dedupe-mixin" "^1.3.0" lit-html "^1.0.0" -"@open-wc/scoped-elements@^2.0.0", "@open-wc/scoped-elements@^2.0.0-next.0": +"@open-wc/scoped-elements@^2.0.0-next.0": version "2.0.0-next.3" resolved "https://registry.yarnpkg.com/@open-wc/scoped-elements/-/scoped-elements-2.0.0-next.3.tgz#adbd9d6fddc67158fd11ffe78c5e11aefdaaf8af" integrity sha512-9dT+0ea/RKO3s2m5H+U8gwG7m1jE89JhgWKI6FnkG4pE9xMx8KACoLZZcUfogVjb6/vKaIeoCj6Mqm+2HiqCeQ== @@ -1710,6 +1710,15 @@ "@open-wc/dedupe-mixin" "^1.3.0" "@webcomponents/scoped-custom-element-registry" "0.0.1" +"@open-wc/scoped-elements@^2.0.0-next.3": + version "2.0.0-next.4" + resolved "https://registry.yarnpkg.com/@open-wc/scoped-elements/-/scoped-elements-2.0.0-next.4.tgz#d8294358e3e8ad2ba44200ab805549fde49245f6" + integrity sha512-BMd5n5BHLi3FBhwhPbBuN7pZdi8I1CIQn10aKLZtg9aplVhN2BG1rwr0ANebXJ6fdq8m1PE1wQAaCXYCcEBTEQ== + dependencies: + "@lit/reactive-element" "^1.0.0-rc.1" + "@open-wc/dedupe-mixin" "^1.3.0" + "@webcomponents/scoped-custom-element-registry" "0.0.2" + "@open-wc/semantic-dom-diff@^0.13.16": version "0.13.21" resolved "https://registry.yarnpkg.com/@open-wc/semantic-dom-diff/-/semantic-dom-diff-0.13.21.tgz#718b9ec5f9a98935fc775e577ad094ae8d8b7dea" @@ -2790,6 +2799,11 @@ resolved "https://registry.yarnpkg.com/@webcomponents/scoped-custom-element-registry/-/scoped-custom-element-registry-0.0.1.tgz#196365260a019f87bddbded154ab09faf0e666fc" integrity sha512-ef5/v4U2vCxrnSMpo41LSWTjBOXCQ4JOt4+Y6PaSd8ympYioPhOP6E1tKmIk2ppwLSjCKbTyYf7ocHvwDat7bA== +"@webcomponents/scoped-custom-element-registry@0.0.2": + version "0.0.2" + resolved "https://registry.yarnpkg.com/@webcomponents/scoped-custom-element-registry/-/scoped-custom-element-registry-0.0.2.tgz#c863d163cb39c60063808e5ae23e06a1766fbe5f" + integrity sha512-lKCoZfKoE3FHvmmj2ytaLBB8Grxp4HaxfSzaGlIZN6xXnOILfpCO0PFJkAxanefLGJWMho4kRY5PhgxWFhmSOw== + "@webcomponents/shadycss@^1.10.2": version "1.10.2" resolved "https://registry.yarnpkg.com/@webcomponents/shadycss/-/shadycss-1.10.2.tgz#40e03cab6dc5e12f199949ba2b79e02f183d1e7b"