diff --git a/packages/components/autocomplete/autocomplete-trigger.directive.ts b/packages/components/autocomplete/autocomplete-trigger.directive.ts index 25d912ae38..dd7bd3d179 100644 --- a/packages/components/autocomplete/autocomplete-trigger.directive.ts +++ b/packages/components/autocomplete/autocomplete-trigger.directive.ts @@ -59,12 +59,12 @@ import { KbqAutocompleteOrigin } from './autocomplete-origin.directive'; import { KbqAutocomplete } from './autocomplete.component'; /** - * The following style constants are necessary to save here in order - * to properly calculate the scrollTop of the panel. Because we are not - * actually focusing the active item, scroll must be handled manually. + * The total height of the autocomplete panel. + * + * @deprecated Unused — the panel is capped by `--kbq-autocomplete-size-panel-max-height` and reveals its + * active option through `KbqOption.focus`, so nothing computes a scroll offset from this. Will be removed + * in the next major release. */ - -/** The total height of the autocomplete panel. */ export const AUTOCOMPLETE_PANEL_HEIGHT = 256; /** @@ -289,7 +289,8 @@ export class KbqAutocompleteTrigger if (this.panelOpen) { this.scrollActiveOptionIntoView(); - this.elementRef.nativeElement.focus(); + // Focus returns to the input on every arrow key; it must not drag the scroll with it. + this.elementRef.nativeElement.focus({ preventScroll: true }); } else if (!this.panelOpen && autocompleteValue.keyManager.activeItem) { autocompleteValue.keyManager.activeItem?.selectViaInteraction(); } diff --git a/packages/components/autocomplete/e2e.webkit.playwright-spec.ts b/packages/components/autocomplete/e2e.webkit.playwright-spec.ts new file mode 100644 index 0000000000..330b7815f2 --- /dev/null +++ b/packages/components/autocomplete/e2e.webkit.playwright-spec.ts @@ -0,0 +1,117 @@ +import { expect, Page, test } from '@playwright/test'; +import { + e2eIsFullyInView, + e2eReadOptionFocusOptions, + e2eRecordOptionFocusOptions, + e2eScrollTopOf, + e2eSettleFrames, + e2eWaitForScrollEnd +} from '../../e2e/utils'; + +/* + * WebKit-only guard for panel scrolling (DS-3299). + * + * Focus scrolls its target into view implicitly. Blink does that synchronously; WebKit defers it to a + * later rendering update, where it lands after — and undoes — whatever the reader scrolled in between. + * Components therefore focus with `preventScroll` and reveal the option themselves. + * + * These assert on scroll offsets rather than screenshots, so they need no baselines and no Docker. + */ + +test.use({ browserName: 'webkit' }); + +test.describe('KbqAutocomplete panel scrolling', () => { + const getPort = (page: Page) => page.locator('.kbq-autocomplete-panel__content'); + + test.beforeEach(async ({ page }) => { + // Installed before navigation so the focus calls the panel makes while opening are recorded too. + await e2eRecordOptionFocusOptions(page); + await page.goto('/E2eAutocompleteScrollbar'); + await page.getByTestId('e2eAutocompleteInput').focus(); + // The routes fit the viewport, so without this the page-scroll assertion below could never fail. + await page.addStyleTag({ content: 'body { min-height: 3000px; }' }); + await expect(getPort(page)).toBeVisible(); + await expect(getPort(page).locator('.kbq-option').first()).toBeVisible(); + }); + + test('has a panel that actually overflows', async ({ page }) => { + // Guards the premise of the other tests: without overflow they would all pass vacuously. + await expect.poll(() => getPort(page).evaluate((el) => el.scrollHeight > el.clientHeight)).toBe(true); + }); + + test('never asks the browser to scroll on focus, including while opening', async ({ page }) => { + for (let i = 0; i < 5; i++) { + await page.keyboard.press('ArrowDown'); + } + + const preventScrollFlags = await e2eReadOptionFocusOptions(page); + + // A non-empty recording proves focus was exercised, so the assertion cannot pass vacuously. + expect(preventScrollFlags.length).toBeGreaterThan(0); + expect(preventScrollFlags).not.toContain(false); + }); + + test('scrolls with the wheel and stays where the reader left it', async ({ page }) => { + const port = getPort(page); + + const start = await e2eScrollTopOf(port); + + await port.hover(); + await page.mouse.wheel(0, 200); + + // The gesture is applied over several frames; wait for the offset to move and then settle. + const scrolled = await e2eWaitForScrollEnd(port, start); + + expect(scrolled).toBeGreaterThan(0); + + // Nothing may move the panel afterwards — a deferred focus scroll used to land right here. + await e2eSettleFrames(page); + + // A deferred focus scroll moves the panel by at least a row; a pixel of settling is not movement. + expect(Math.abs((await e2eScrollTopOf(port)) - scrolled)).toBeLessThanOrEqual(1); + }); + + test('does not move the list when the pointer lands on a partially clipped option', async ({ page }) => { + const port = getPort(page); + + const offsets = await port.evaluate(async (element, sel) => { + // Land on a fractional offset so a row straddles the top edge of the scrollport. + element.scrollTop = 50; + await new Promise((resolve) => requestAnimationFrame(resolve)); + + const before = element.scrollTop; + const port = element.getBoundingClientRect(); + const clipped = [...element.querySelectorAll(sel)].find((item) => { + const box = item.getBoundingClientRect(); + + return box.top < port.top && box.bottom > port.top; + }); + + if (!clipped) return null; + + clipped.dispatchEvent(new MouseEvent('mouseenter')); + await new Promise((resolve) => requestAnimationFrame(() => requestAnimationFrame(resolve))); + + return { before, after: element.scrollTop }; + }, '.kbq-option'); + + expect(offsets).not.toBeNull(); + expect(offsets!.after).toBe(offsets!.before); + }); + + test('brings the active option into view on keyboard navigation without scrolling the page', async ({ page }) => { + const port = getPort(page); + const itemCount = await port.locator('.kbq-option').count(); + + for (let i = 0; i < itemCount; i++) { + await page.keyboard.press('ArrowDown'); + } + + await e2eSettleFrames(page); + + expect(await e2eScrollTopOf(port)).toBeGreaterThan(0); + expect(await e2eIsFullyInView(port, '.kbq-option.kbq-active')).toBe(true); + // The page is deliberately made scrollable in beforeEach, so this can actually fail. + expect(await page.evaluate(() => window.scrollY)).toBe(0); + }); +}); diff --git a/packages/components/core/option/option.spec.ts b/packages/components/core/option/option.spec.ts index ba07b1c5d7..43ed64daf3 100644 --- a/packages/components/core/option/option.spec.ts +++ b/packages/components/core/option/option.spec.ts @@ -34,6 +34,59 @@ describe('KbqOption component', () => { subscription.unsubscribe(); }); + it('should reveal the option on focus without letting the browser scroll from focus itself', () => { + const fixture = TestBed.createComponent(OptionWithDisable); + + fixture.detectChanges(); + + const option: KbqOption = fixture.debugElement.query(By.directive(KbqOption)).componentInstance; + const host = option.getHostElement(); + const focusSpy = jest.spyOn(host, 'focus'); + const scrollSpy = jest.spyOn(host, 'scrollIntoView'); + + option.focus(); + + expect(focusSpy).toHaveBeenCalledWith({ preventScroll: true }); + expect(scrollSpy).toHaveBeenCalledWith({ block: 'nearest', inline: 'nearest' }); + }); + + it('should focus but not reveal while the pointer is over the option', () => { + const fixture = TestBed.createComponent(OptionWithDisable); + + fixture.detectChanges(); + + const option: KbqOption = fixture.debugElement.query(By.directive(KbqOption)).componentInstance; + const host = option.getHostElement(); + const focusSpy = jest.spyOn(host, 'focus'); + const scrollSpy = jest.spyOn(host, 'scrollIntoView'); + + host.dispatchEvent(new MouseEvent('mouseenter')); + option.focus(); + + // Focus must still move, otherwise type-ahead and aria-activedescendant break. + expect(focusSpy).toHaveBeenCalledWith({ preventScroll: true }); + // The option is already under the cursor; revealing it would shift the list out from under it. + expect(scrollSpy).not.toHaveBeenCalled(); + }); + + it('should reveal again once the pointer has left, even if no focus happened in between', () => { + const fixture = TestBed.createComponent(OptionWithDisable); + + fixture.detectChanges(); + + const option: KbqOption = fixture.debugElement.query(By.directive(KbqOption)).componentInstance; + const host = option.getHostElement(); + const scrollSpy = jest.spyOn(host, 'scrollIntoView'); + + // Hovering the option that is already active never reaches focus(), so a flag cleared only + // there would stay armed and silently swallow every later reveal. + host.dispatchEvent(new MouseEvent('mouseenter')); + host.dispatchEvent(new MouseEvent('mouseleave')); + option.focus(); + + expect(scrollSpy).toHaveBeenCalledWith({ block: 'nearest', inline: 'nearest' }); + }); + it('should not emit to `onSelectionChange` if selecting an already-selected option', () => { const fixture = TestBed.createComponent(OptionWithDisable); diff --git a/packages/components/core/option/option.ts b/packages/components/core/option/option.ts index 274c00cd2e..6636357068 100644 --- a/packages/components/core/option/option.ts +++ b/packages/components/core/option/option.ts @@ -22,6 +22,7 @@ import { ActiveDescendantKeyManager } from '../a11y'; import { ENTER, hasModifierKey, SPACE } from '../keycodes'; import { KbqPseudoCheckboxModule } from '../selection'; import { KBQ_TITLE_TEXT_REF, KbqTitleTextRef } from '../title'; +import { kbqFocusAndReveal } from '../utils'; import { KbqOptgroup } from './optgroup'; /** @@ -157,6 +158,7 @@ export class KbqVirtualOption extends KbqOptionBase { '(click)': 'handleClick($event)', '(mouseenter)': 'onMouseenter()', + '(mouseleave)': 'onMouseleave()', '(keydown)': 'handleKeydown($event)' }, exportAs: 'kbqOption' @@ -259,14 +261,11 @@ export class KbqOption extends KbqOptionBase implements AfterViewChecked, OnDest private mostRecentViewValue = ''; /** - * Flag that indicates whether the component is currently focused by a mouse interaction. - * - * When set to `true`, the component has focus resulting from a mouse click or - * other pointer event. It is automatically cleared when the component loses - * focus or if focus is obtained through keyboard navigation or programmatic means. + * Whether the pointer is currently over this option. Cleared on `mouseleave` rather than when the + * option is focused: hovering the option that is already active does not move the key manager's + * index, so no focus follows and a flag cleared only there would stay armed indefinitely. */ - private isFocusedByMouse: boolean = false; - + private hoveredByPointer = false; ngAfterViewChecked() { // Since parent components could be using the option's label to display the selected values // (e.g. `kbq-select`) and they don't have a way of knowing if the option's label has changed @@ -310,14 +309,9 @@ export class KbqOption extends KbqOptionBase implements AfterViewChecked, OnDest } } + /** Moves keyboard focus to this option and reveals it — see {@link kbqFocusAndReveal}. */ focus(): void { - const element = this.getHostElement(); - - if (typeof element.focus === 'function') { - element.focus({ preventScroll: this.isFocusedByMouse }); - - this.isFocusedByMouse = false; - } + kbqFocusAndReveal(this.getHostElement(), this.hoveredByPointer); } /** @@ -400,10 +394,15 @@ export class KbqOption extends KbqOptionBase implements AfterViewChecked, OnDest protected onMouseenter() { if (this.disabled) return; - this.isFocusedByMouse = true; + this.hoveredByPointer = true; this.parent?.keyManager?.setActiveItem(this); } + + /** @docs-private */ + protected onMouseleave() { + this.hoveredByPointer = false; + } } /** @@ -443,6 +442,9 @@ export function countGroupLabelsBeforeOption( * @param currentScrollPosition Current scroll position of the panel. * @param panelHeight Height of the panel. * @docs-private + * @deprecated Unused — an option reveals itself on focus through `KbqOption.focus`, which lets the + * browser resolve the scroll container instead of computing an offset from a uniform row height. Will be + * removed in the next major release. */ export function getOptionScrollPosition( optionIndex: number, diff --git a/packages/components/core/utils/dom.ts b/packages/components/core/utils/dom.ts index 65bc4e9dea..82708ebad1 100644 --- a/packages/components/core/utils/dom.ts +++ b/packages/components/core/utils/dom.ts @@ -1,3 +1,4 @@ +import { _getFocusedElementPierceShadowDom } from '@angular/cdk/platform'; import { ElementRef, inject } from '@angular/core'; /** @@ -6,3 +7,28 @@ import { ElementRef, inject } from '@angular/core'; export const kbqInjectNativeElement = (): T => { return inject>(ElementRef).nativeElement; }; + +/** + * Focuses `element` and scrolls it into view by the shortest distance, leaving a visible one alone. + * + * The reveal is explicit because the one `focus()` performs implicitly is not portable: WebKit defers it + * to a later rendering update, where it lands after — and undoes — any scrolling the reader did in the + * meantime. Letting the element reveal itself keeps every consumer working, whatever it uses as a scroll + * container, and the browser resolves that container rather than the caller guessing at it. + * + * Pass `skipReveal` when the pointer caused the focus: the element is already under the cursor, so + * revealing it would shift the list out from under it. + */ +export const kbqFocusAndReveal = (element: HTMLElement, skipReveal = false): void => { + if (typeof element.focus !== 'function') return; + + // A focus listener calling back into `focus` must not scroll a second time. Read through shadow + // roots, where `document.activeElement` reports the host instead of the element that holds focus. + const wasFocused = _getFocusedElementPierceShadowDom() === element; + + element.focus({ preventScroll: true }); + + if (skipReveal || wasFocused) return; + + element.scrollIntoView?.({ block: 'nearest', inline: 'nearest' }); +}; diff --git a/packages/components/dropdown/dropdown-item.component.ts b/packages/components/dropdown/dropdown-item.component.ts index 751f19e3b3..5bb618a657 100644 --- a/packages/components/dropdown/dropdown-item.component.ts +++ b/packages/components/dropdown/dropdown-item.component.ts @@ -131,14 +131,28 @@ export class KbqDropdownItem implements KbqTitleTextRef, KbqDropdownItemActionHo this.getHostElement().classList.remove('cdk-keyboard-focused'); } - /** Focuses the dropdown item. */ + /** + * Focuses the dropdown item and reveals it. + * + * The reveal is explicit because the one `focus()` performs implicitly is not portable: WebKit defers + * it to a later rendering update, where it lands after — and undoes — any scrolling the reader did in + * the meantime. `preventScroll` is therefore always forced on, overriding `options`. + */ focus(origin?: FocusOrigin, options?: FocusOptions): void { if (this.disabled) return; + const element = this.getHostElement(); + const focusOptions: FocusOptions = { ...options, preventScroll: true }; + if (this.focusMonitor && origin) { - this.focusMonitor.focusVia(this.getHostElement(), origin, options); + this.focusMonitor.focusVia(element, origin, focusOptions); } else { - this.getHostElement().focus(options); + element.focus(focusOptions); + } + + // With the pointer already on this item, revealing it would shift the list out from under it. + if (origin !== 'mouse') { + element.scrollIntoView?.({ block: 'nearest', inline: 'nearest' }); } this.focused.next(this); diff --git a/packages/components/dropdown/dropdown.component.ts b/packages/components/dropdown/dropdown.component.ts index 4af9d733d1..c68f9ef6c1 100644 --- a/packages/components/dropdown/dropdown.component.ts +++ b/packages/components/dropdown/dropdown.component.ts @@ -592,7 +592,7 @@ export class KbqDropdown implements AfterContentInit, KbqDropdownPanel, OnInit, // same as `setFirstItemActive` no-ops in that case. const panel = this.directDescendantItems.first?.getHostElement().closest('.kbq-dropdown__panel'); - panel?.focus(); + panel?.focus({ preventScroll: true }); } /** diff --git a/packages/components/dropdown/e2e.webkit.playwright-spec.ts b/packages/components/dropdown/e2e.webkit.playwright-spec.ts new file mode 100644 index 0000000000..4d9abdf43f --- /dev/null +++ b/packages/components/dropdown/e2e.webkit.playwright-spec.ts @@ -0,0 +1,117 @@ +import { expect, Page, test } from '@playwright/test'; +import { + e2eIsFullyInView, + e2eReadOptionFocusOptions, + e2eRecordOptionFocusOptions, + e2eScrollTopOf, + e2eSettleFrames, + e2eWaitForScrollEnd +} from '../../e2e/utils'; + +/* + * WebKit-only guard for panel scrolling (DS-3299). + * + * Focus scrolls its target into view implicitly. Blink does that synchronously; WebKit defers it to a + * later rendering update, where it lands after — and undoes — whatever the reader scrolled in between. + * Components therefore focus with `preventScroll` and reveal the item themselves. + * + * These assert on scroll offsets rather than screenshots, so they need no baselines and no Docker. + */ + +test.use({ browserName: 'webkit' }); + +test.describe('KbqDropdown panel scrolling', () => { + const getPort = (page: Page) => page.locator('.kbq-dropdown__panel'); + + test.beforeEach(async ({ page }) => { + // Installed before navigation so the focus calls the panel makes while opening are recorded too. + await e2eRecordOptionFocusOptions(page); + await page.goto('/E2eDropdownScrollbar'); + await page.getByTestId('e2eDropdownScrollbarTrigger').click(); + // The routes fit the viewport, so without this the page-scroll assertion below could never fail. + await page.addStyleTag({ content: 'body { min-height: 3000px; }' }); + await expect(getPort(page)).toBeVisible(); + await expect(getPort(page).locator('.kbq-dropdown-item').first()).toBeVisible(); + }); + + test('has a panel that actually overflows', async ({ page }) => { + // Guards the premise of the other tests: without overflow they would all pass vacuously. + await expect.poll(() => getPort(page).evaluate((el) => el.scrollHeight > el.clientHeight)).toBe(true); + }); + + test('never asks the browser to scroll on focus, including while opening', async ({ page }) => { + for (let i = 0; i < 5; i++) { + await page.keyboard.press('ArrowDown'); + } + + const preventScrollFlags = await e2eReadOptionFocusOptions(page); + + // A non-empty recording proves focus was exercised, so the assertion cannot pass vacuously. + expect(preventScrollFlags.length).toBeGreaterThan(0); + expect(preventScrollFlags).not.toContain(false); + }); + + test('scrolls with the wheel and stays where the reader left it', async ({ page }) => { + const port = getPort(page); + + const start = await e2eScrollTopOf(port); + + await port.hover(); + await page.mouse.wheel(0, 200); + + // The gesture is applied over several frames; wait for the offset to move and then settle. + const scrolled = await e2eWaitForScrollEnd(port, start); + + expect(scrolled).toBeGreaterThan(0); + + // Nothing may move the panel afterwards — a deferred focus scroll used to land right here. + await e2eSettleFrames(page); + + // A deferred focus scroll moves the panel by at least a row; a pixel of settling is not movement. + expect(Math.abs((await e2eScrollTopOf(port)) - scrolled)).toBeLessThanOrEqual(1); + }); + + test('does not move the list when the pointer lands on a partially clipped item', async ({ page }) => { + const port = getPort(page); + + const offsets = await port.evaluate(async (element, sel) => { + // Land on a fractional offset so a row straddles the top edge of the scrollport. + element.scrollTop = 50; + await new Promise((resolve) => requestAnimationFrame(resolve)); + + const before = element.scrollTop; + const port = element.getBoundingClientRect(); + const clipped = [...element.querySelectorAll(sel)].find((item) => { + const box = item.getBoundingClientRect(); + + return box.top < port.top && box.bottom > port.top; + }); + + if (!clipped) return null; + + clipped.dispatchEvent(new MouseEvent('mouseenter')); + await new Promise((resolve) => requestAnimationFrame(() => requestAnimationFrame(resolve))); + + return { before, after: element.scrollTop }; + }, '.kbq-dropdown-item'); + + expect(offsets).not.toBeNull(); + expect(offsets!.after).toBe(offsets!.before); + }); + + test('brings the active item into view on keyboard navigation without scrolling the page', async ({ page }) => { + const port = getPort(page); + const itemCount = await port.locator('.kbq-dropdown-item').count(); + + for (let i = 0; i < itemCount; i++) { + await page.keyboard.press('ArrowDown'); + } + + await e2eSettleFrames(page); + + expect(await e2eScrollTopOf(port)).toBeGreaterThan(0); + expect(await e2eIsFullyInView(port, '.kbq-dropdown-item:focus')).toBe(true); + // The page is deliberately made scrollable in beforeEach, so this can actually fail. + expect(await page.evaluate(() => window.scrollY)).toBe(0); + }); +}); diff --git a/packages/components/select/e2e.webkit.playwright-spec.ts b/packages/components/select/e2e.webkit.playwright-spec.ts new file mode 100644 index 0000000000..19eed325af --- /dev/null +++ b/packages/components/select/e2e.webkit.playwright-spec.ts @@ -0,0 +1,117 @@ +import { expect, Page, test } from '@playwright/test'; +import { + e2eIsFullyInView, + e2eReadOptionFocusOptions, + e2eRecordOptionFocusOptions, + e2eScrollTopOf, + e2eSettleFrames, + e2eWaitForScrollEnd +} from '../../e2e/utils'; + +/* + * WebKit-only guard for panel scrolling (DS-3299). + * + * Focus scrolls its target into view implicitly. Blink does that synchronously; WebKit defers it to a + * later rendering update, where it lands after — and undoes — whatever the reader scrolled in between. + * Components therefore focus with `preventScroll` and reveal the option themselves. + * + * These assert on scroll offsets rather than screenshots, so they need no baselines and no Docker. + */ + +test.use({ browserName: 'webkit' }); + +test.describe('KbqSelect panel scrolling', () => { + const getPort = (page: Page) => page.locator('.kbq-select__content'); + + test.beforeEach(async ({ page }) => { + // Installed before navigation so the focus calls the panel makes while opening are recorded too. + await e2eRecordOptionFocusOptions(page); + await page.goto('/E2eSelectScrollbar'); + await page.getByTestId('e2eSelect').click(); + // The routes fit the viewport, so without this the page-scroll assertion below could never fail. + await page.addStyleTag({ content: 'body { min-height: 3000px; }' }); + await expect(getPort(page)).toBeVisible(); + await expect(getPort(page).locator('.kbq-option').first()).toBeVisible(); + }); + + test('has a panel that actually overflows', async ({ page }) => { + // Guards the premise of the other tests: without overflow they would all pass vacuously. + await expect.poll(() => getPort(page).evaluate((el) => el.scrollHeight > el.clientHeight)).toBe(true); + }); + + test('never asks the browser to scroll on focus, including while opening', async ({ page }) => { + for (let i = 0; i < 5; i++) { + await page.keyboard.press('ArrowDown'); + } + + const preventScrollFlags = await e2eReadOptionFocusOptions(page); + + // A non-empty recording proves focus was exercised, so the assertion cannot pass vacuously. + expect(preventScrollFlags.length).toBeGreaterThan(0); + expect(preventScrollFlags).not.toContain(false); + }); + + test('scrolls with the wheel and stays where the reader left it', async ({ page }) => { + const port = getPort(page); + + const start = await e2eScrollTopOf(port); + + await port.hover(); + await page.mouse.wheel(0, 200); + + // The gesture is applied over several frames; wait for the offset to move and then settle. + const scrolled = await e2eWaitForScrollEnd(port, start); + + expect(scrolled).toBeGreaterThan(0); + + // Nothing may move the panel afterwards — a deferred focus scroll used to land right here. + await e2eSettleFrames(page); + + // A deferred focus scroll moves the panel by at least a row; a pixel of settling is not movement. + expect(Math.abs((await e2eScrollTopOf(port)) - scrolled)).toBeLessThanOrEqual(1); + }); + + test('does not move the list when the pointer lands on a partially clipped option', async ({ page }) => { + const port = getPort(page); + + const offsets = await port.evaluate(async (element, sel) => { + // Land on a fractional offset so a row straddles the top edge of the scrollport. + element.scrollTop = 50; + await new Promise((resolve) => requestAnimationFrame(resolve)); + + const before = element.scrollTop; + const port = element.getBoundingClientRect(); + const clipped = [...element.querySelectorAll(sel)].find((item) => { + const box = item.getBoundingClientRect(); + + return box.top < port.top && box.bottom > port.top; + }); + + if (!clipped) return null; + + clipped.dispatchEvent(new MouseEvent('mouseenter')); + await new Promise((resolve) => requestAnimationFrame(() => requestAnimationFrame(resolve))); + + return { before, after: element.scrollTop }; + }, '.kbq-option'); + + expect(offsets).not.toBeNull(); + expect(offsets!.after).toBe(offsets!.before); + }); + + test('brings the active option into view on keyboard navigation without scrolling the page', async ({ page }) => { + const port = getPort(page); + const itemCount = await port.locator('.kbq-option').count(); + + for (let i = 0; i < itemCount; i++) { + await page.keyboard.press('ArrowDown'); + } + + await e2eSettleFrames(page); + + expect(await e2eScrollTopOf(port)).toBeGreaterThan(0); + expect(await e2eIsFullyInView(port, '.kbq-option.kbq-active')).toBe(true); + // The page is deliberately made scrollable in beforeEach, so this can actually fail. + expect(await page.evaluate(() => window.scrollY)).toBe(0); + }); +}); diff --git a/packages/components/select/select.component.ts b/packages/components/select/select.component.ts index 4d3af3eb7f..46c6cfebdf 100644 --- a/packages/components/select/select.component.ts +++ b/packages/components/select/select.component.ts @@ -1075,7 +1075,7 @@ export class KbqSelect /** Subscription to the options panel scroll event used by `scrolledToBottom`. */ private scrollSubscription = Subscription.EMPTY; - /** The scroll position of the overlay panel, calculated to center the selected option. */ + /** The scroll offset the panel is restored to when it attaches — the list always opens at the top. */ private scrollTop = 0; /** Unique id for this input. Auto-incremented for each instance. */ @@ -2251,7 +2251,6 @@ export class KbqSelect } } - /** Scrolls the active option into view. */ private scrollActiveOptionIntoView(): void { this.keyManager.activeItem?.focus(); } diff --git a/packages/components/tree-select/e2e.webkit.playwright-spec.ts b/packages/components/tree-select/e2e.webkit.playwright-spec.ts new file mode 100644 index 0000000000..7617e689fa --- /dev/null +++ b/packages/components/tree-select/e2e.webkit.playwright-spec.ts @@ -0,0 +1,117 @@ +import { expect, Page, test } from '@playwright/test'; +import { + e2eIsFullyInView, + e2eReadOptionFocusOptions, + e2eRecordOptionFocusOptions, + e2eScrollTopOf, + e2eSettleFrames, + e2eWaitForScrollEnd +} from '../../e2e/utils'; + +/* + * WebKit-only guard for panel scrolling (DS-3299). + * + * Focus scrolls its target into view implicitly. Blink does that synchronously; WebKit defers it to a + * later rendering update, where it lands after — and undoes — whatever the reader scrolled in between. + * Components therefore focus with `preventScroll` and reveal the node themselves. + * + * These assert on scroll offsets rather than screenshots, so they need no baselines and no Docker. + */ + +test.use({ browserName: 'webkit' }); + +test.describe('KbqTreeSelect panel scrolling', () => { + const getPort = (page: Page) => page.locator('.kbq-tree-select__content'); + + test.beforeEach(async ({ page }) => { + // Installed before navigation so the focus calls the panel makes while opening are recorded too. + await e2eRecordOptionFocusOptions(page); + await page.goto('/E2eTreeSelectScrollbar'); + await page.getByTestId('e2eTreeSelect').click(); + // The routes fit the viewport, so without this the page-scroll assertion below could never fail. + await page.addStyleTag({ content: 'body { min-height: 3000px; }' }); + await expect(getPort(page)).toBeVisible(); + await expect(getPort(page).locator('.kbq-tree-option').first()).toBeVisible(); + }); + + test('has a panel that actually overflows', async ({ page }) => { + // Guards the premise of the other tests: without overflow they would all pass vacuously. + await expect.poll(() => getPort(page).evaluate((el) => el.scrollHeight > el.clientHeight)).toBe(true); + }); + + test('never asks the browser to scroll on focus, including while opening', async ({ page }) => { + for (let i = 0; i < 5; i++) { + await page.keyboard.press('ArrowDown'); + } + + const preventScrollFlags = await e2eReadOptionFocusOptions(page); + + // A non-empty recording proves focus was exercised, so the assertion cannot pass vacuously. + expect(preventScrollFlags.length).toBeGreaterThan(0); + expect(preventScrollFlags).not.toContain(false); + }); + + test('scrolls with the wheel and stays where the reader left it', async ({ page }) => { + const port = getPort(page); + + const start = await e2eScrollTopOf(port); + + await port.hover(); + await page.mouse.wheel(0, 200); + + // The gesture is applied over several frames; wait for the offset to move and then settle. + const scrolled = await e2eWaitForScrollEnd(port, start); + + expect(scrolled).toBeGreaterThan(0); + + // Nothing may move the panel afterwards — a deferred focus scroll used to land right here. + await e2eSettleFrames(page); + + // A deferred focus scroll moves the panel by at least a row; a pixel of settling is not movement. + expect(Math.abs((await e2eScrollTopOf(port)) - scrolled)).toBeLessThanOrEqual(1); + }); + + test('does not move the list when the pointer lands on a partially clipped node', async ({ page }) => { + const port = getPort(page); + + const offsets = await port.evaluate(async (element, sel) => { + // Land on a fractional offset so a row straddles the top edge of the scrollport. + element.scrollTop = 50; + await new Promise((resolve) => requestAnimationFrame(resolve)); + + const before = element.scrollTop; + const port = element.getBoundingClientRect(); + const clipped = [...element.querySelectorAll(sel)].find((item) => { + const box = item.getBoundingClientRect(); + + return box.top < port.top && box.bottom > port.top; + }); + + if (!clipped) return null; + + clipped.dispatchEvent(new MouseEvent('mouseenter')); + await new Promise((resolve) => requestAnimationFrame(() => requestAnimationFrame(resolve))); + + return { before, after: element.scrollTop }; + }, '.kbq-tree-option'); + + expect(offsets).not.toBeNull(); + expect(offsets!.after).toBe(offsets!.before); + }); + + test('brings the active node into view on keyboard navigation without scrolling the page', async ({ page }) => { + const port = getPort(page); + const itemCount = await port.locator('.kbq-tree-option').count(); + + for (let i = 0; i < itemCount; i++) { + await page.keyboard.press('ArrowDown'); + } + + await e2eSettleFrames(page); + + expect(await e2eScrollTopOf(port)).toBeGreaterThan(0); + expect(await e2eIsFullyInView(port, '.kbq-tree-option:focus')).toBe(true); + // The page is deliberately made scrollable in beforeEach, so this can actually fail. + expect(await page.evaluate(() => window.scrollY)).toBe(0); + }); +}); diff --git a/packages/components/tree-select/tree-select.component.ts b/packages/components/tree-select/tree-select.component.ts index fd9dfc6892..b6d9310f82 100644 --- a/packages/components/tree-select/tree-select.component.ts +++ b/packages/components/tree-select/tree-select.component.ts @@ -783,7 +783,7 @@ export class KbqTreeSelect private originalOnKeyDown: (event: KeyboardEvent) => void; - /** The scroll position of the overlay panel, calculated to center the selected option. */ + /** The scroll offset the panel is restored to when it attaches — the list always opens at the top. */ private scrollTop = 0; /** Unique id for this input. */ @@ -1233,8 +1233,8 @@ export class KbqTreeSelect this.overlayDir.positionChange.pipe(take(1)).subscribe(() => { this.changeDetectorRef.detectChanges(); this.setOverlayPosition(); - // `panel` is guaranteed to exist here: this callback only fires once the overlay has attached. - this.panel()!.nativeElement.scrollTop = this.scrollTop; + // The panel itself is an `overflow: hidden` box; the option list is what scrolls. + this.optionsContainer()!.nativeElement.scrollTop = this.scrollTop; this.tree()!.updateScrollSize(); // Deliberately out of this frame — see `reanchorPanel`. A microtask still lands before paint. @@ -1654,7 +1654,6 @@ export class KbqTreeSelect } } - /** Scrolls the active option into view. */ private scrollActiveOptionIntoView() { this.tree()!.keyManager.activeItem?.focus(); } diff --git a/packages/components/tree/tree-option.component.ts b/packages/components/tree/tree-option.component.ts index 5970ea0310..2d4c5f5cad 100644 --- a/packages/components/tree/tree-option.component.ts +++ b/packages/components/tree/tree-option.component.ts @@ -24,6 +24,7 @@ import { KBQ_OPTION_ACTION_PARENT, KBQ_TITLE_TEXT_REF, KbqActionContainer, + kbqFocusAndReveal, kbqFocusOptionActionOnTab, KbqOptionActionComponent, KbqPseudoCheckbox, @@ -323,7 +324,8 @@ export class KbqTreeOption extends KbqTreeNode implements AfterCo focus(focusOrigin?: FocusOrigin) { if (focusOrigin === 'program' || this.disabled || this.actionButton()?.hasFocus) return; - this.elementRef.nativeElement.focus({ preventScroll: focusOrigin === 'mouse' }); + // With the pointer already on this node, revealing it would shift the tree out from under it. + kbqFocusAndReveal(this.elementRef.nativeElement, focusOrigin === 'mouse'); if (!this.hasFocus) { this.onFocus.next({ option: this }); diff --git a/packages/e2e/utils/focus-scroll.ts b/packages/e2e/utils/focus-scroll.ts new file mode 100644 index 0000000000..bbda4fcbb5 --- /dev/null +++ b/packages/e2e/utils/focus-scroll.ts @@ -0,0 +1,111 @@ +import { Locator, Page } from '@playwright/test'; + +type FocusScrollWindow = Window & { + __kbqOptionFocusOptions?: boolean[]; + __kbqOptionFocusPatched?: boolean; +}; + +/** + * Records the `FocusOptions` every option-like element is focused with, so a spec can assert that a panel + * never leans on the scroll `HTMLElement.focus()` performs implicitly. + * + * That implicit scroll is not portable: Blink runs it synchronously, while WebKit defers it to a later + * rendering update, where it lands after — and undoes — whatever the reader scrolled in the meantime. + * Components are expected to focus with `preventScroll: true` and reveal the element explicitly. + * + * Installed with `addInitScript` so the recording covers the focus calls a panel makes as it opens, not + * just the ones a spec triggers afterwards. Call before `page.goto`, then read the flags back with + * {@link e2eReadOptionFocusOptions}. + */ +export const e2eRecordOptionFocusOptions = async (page: Page): Promise => { + // Awaited rather than returned: `addInitScript` resolves to a Disposable — see `e2eDisableResizeObserver`. + await page.addInitScript(() => { + const target = window as FocusScrollWindow; + + target.__kbqOptionFocusOptions = []; + + // A page may be navigated more than once per spec; patch the prototype only once. + if (target.__kbqOptionFocusPatched) return; + + target.__kbqOptionFocusPatched = true; + + const originalFocus = HTMLElement.prototype.focus; + + HTMLElement.prototype.focus = function (this: HTMLElement, options?: FocusOptions) { + if (this.matches('.kbq-option, .kbq-tree-option, .kbq-dropdown-item')) { + (window as FocusScrollWindow).__kbqOptionFocusOptions?.push(options?.preventScroll === true); + } + + return originalFocus.call(this, options); + }; + }); +}; + +/** Reads back what {@link e2eRecordOptionFocusOptions} captured: one `preventScroll` flag per focus call. */ +export const e2eReadOptionFocusOptions = (page: Page): Promise => + page.evaluate(() => (window as FocusScrollWindow).__kbqOptionFocusOptions ?? []); + +/** Waits two animation frames — long enough for a scroll a browser deferred past the current task to land. */ +export const e2eSettleFrames = (page: Page): Promise => + page.evaluate( + () => new Promise((resolve) => requestAnimationFrame(() => requestAnimationFrame(() => resolve()))) + ); + +/** Reads a scrollport's current vertical offset. */ +export const e2eScrollTopOf = (scrollport: Locator): Promise => + scrollport.evaluate((element) => element.scrollTop); + +/** + * Waits until `scrollport`'s offset stops changing and returns it. + * + * A wheel gesture is applied over several frames, so reading the offset a fixed number of frames later + * catches an intermediate value — the reason to wait for the value itself rather than for a frame count. + */ +export const e2eWaitForScrollEnd = (scrollport: Locator, from: number): Promise => + scrollport.evaluate( + (element, start) => + new Promise((resolve) => { + const stableFramesNeeded = 3; + + let previous = element.scrollTop; + let stableFrames = 0; + + const check = () => { + // Waiting only for stability would resolve `start` unchanged when the gesture has not + // been applied yet — `mouse.wheel` does not wait for that — and report "never scrolled". + if (element.scrollTop !== start && element.scrollTop === previous) { + stableFrames++; + } else { + stableFrames = 0; + previous = element.scrollTop; + } + + if (stableFrames >= stableFramesNeeded) { + resolve(previous); + + return; + } + + // No frame cap: Playwright's own test timeout reports a stuck scrollport with a far + // clearer message than a mid-flight offset that silently satisfies the assertion. + requestAnimationFrame(check); + }; + + requestAnimationFrame(check); + }), + from + ); + +/** Whether `selector`'s match inside `scrollport` is fully within it. Null when nothing matches. */ +export const e2eIsFullyInView = (scrollport: Locator, selector: string): Promise => + scrollport.evaluate((element, itemSelector) => { + const item = element.querySelector(itemSelector); + + if (!item) return null; + + const port = element.getBoundingClientRect(); + const box = item.getBoundingClientRect(); + + // A one-pixel tolerance absorbs sub-pixel rounding at fractional zoom levels. + return box.top >= port.top - 1 && box.bottom <= port.bottom + 1; + }, selector); diff --git a/packages/e2e/utils/index.ts b/packages/e2e/utils/index.ts index a794db1c18..9b7ab721e0 100644 --- a/packages/e2e/utils/index.ts +++ b/packages/e2e/utils/index.ts @@ -1,4 +1,5 @@ export * from './autofill'; +export * from './focus-scroll'; export * from './overflow-shadow'; export * from './scrollbar'; export * from './theme'; diff --git a/tools/public_api_guard/components/autocomplete.api.md b/tools/public_api_guard/components/autocomplete.api.md index 78d2eee0c1..2ee5b4947a 100644 --- a/tools/public_api_guard/components/autocomplete.api.md +++ b/tools/public_api_guard/components/autocomplete.api.md @@ -28,7 +28,7 @@ import { ScrollDispatcher } from '@angular/cdk/overlay'; import { ScrollStrategy } from '@angular/cdk/overlay'; import { TemplateRef } from '@angular/core'; -// @public +// @public @deprecated export const AUTOCOMPLETE_PANEL_HEIGHT = 256; // @public diff --git a/tools/public_api_guard/components/core.api.md b/tools/public_api_guard/components/core.api.md index 7f587f1a9f..26b1471f16 100644 --- a/tools/public_api_guard/components/core.api.md +++ b/tools/public_api_guard/components/core.api.md @@ -1030,7 +1030,7 @@ export function getKbqSelectNonFunctionValueError(): Error; // @public (undocumented) export const getNodesWithoutComments: (nodes: NodeList) => Node[]; -// @public +// @public @deprecated export function getOptionScrollPosition(optionIndex: number, optionHeight: number, currentScrollPosition: number, panelHeight: number): number; // @public @@ -3020,6 +3020,9 @@ export type KbqFlexDirection = 'row' | 'column'; // @public export type KbqFlexWrap = 'nowrap' | 'wrap'; +// @public +export const kbqFocusAndReveal: (element: HTMLElement, skipReveal?: boolean) => void; + // @public export function kbqFocusOptionActionOnTab($event: KeyboardEvent, actionButton: KbqOptionActionComponent | undefined): void; @@ -3464,7 +3467,6 @@ export class KbqOption extends KbqOptionBase implements AfterViewChecked, OnDest // (undocumented) get disabled(): any; set disabled(value: any); - // (undocumented) focus(): void; getHeight(): number; // (undocumented) @@ -3484,6 +3486,7 @@ export class KbqOption extends KbqOptionBase implements AfterViewChecked, OnDest // (undocumented) ngOnDestroy(): void; protected onMouseenter(): void; + protected onMouseleave(): void; readonly onSelectionChange: EventEmitter>; // (undocumented) protected readonly parent: KbqOptionParentComponent;