Skip to content

fix(frontend): keep scroll position when a menu closes (#672) - #679

Merged
Starosdev merged 3 commits into
developfrom
fix/672-menu-scroll-jump
Jul 28, 2026
Merged

fix(frontend): keep scroll position when a menu closes (#672)#679
Starosdev merged 3 commits into
developfrom
fix/672-menu-scroll-jump

Conversation

@Starosdev

Copy link
Copy Markdown
Owner

Fixes #672.

Problem

Opening a three-dot menu, scrolling the page, then clicking anywhere made the page jump back to the scroll position it had when the menu was opened.

MatMenuTrigger._destroyMenu() restores focus to the trigger on close by calling focus() with no FocusOptions, which routes to nativeElement.focus(undefined). Without { preventScroll: true } the browser scrolls the trigger back into view.

Menus use the default reposition scroll strategy, so BlockScrollStrategy was not involved, and MAT_MENU_DEFAULT_OPTIONS has no restoreFocus field, so there is no provider-level lever.

Changes

  1. MenuTriggerRestoreFocusDirective (app/shared/) shadows Material's [matMenuTriggerFor] selector, takes focus restoration over from the trigger and restores focus with { preventScroll: true }. Keyboard users still get focus back on the trigger, so this is not the [matMenuTriggerRestoreFocus]="false" trade that drops focus return (WCAG 2.4.3). Submenu triggers are skipped: they live inside the fixed overlay and cannot scroll the page. Added to the six components that render root-level triggers.

    Deliberately excluded: @treo/components/navigation/horizontal/components/branch - the live layout uses treo-vertical-navigation, so that component is dead code, and wiring it would create a @treo -> app/shared dependency.

  2. CI guard in the Test Frontend job. The directive only applies to components that import it, and a component that forgets it still compiles, so the job fails when a component template renders a menu trigger and its sibling component file does not reference the directive.

  3. Separate commit: cdkScrollable on the mobile layout's scroll container. That div is the scroller on narrow viewports but was never registered with the CDK ScrollDispatcher, so overlays never saw its scroll events - an open menu stayed frozen in place while content slid underneath it, and tooltips did not hide on scroll.

Tests

New spec menu-trigger-restore-focus.directive.spec.ts (6 specs) runs the trigger inside its own overflow container so the assertions do not depend on the karma window size. It covers focus restoration with preventScroll, the preserved scroll position, focus actually returning to the trigger, the restoreFocus handover, the explicit opt-out, submenu triggers being left alone, and a detached trigger.

It also includes a control spec on a host without the directive, which asserts the scroll jump still happens. That proves the other specs are not tautological, and it will fail if Angular Material ever passes preventScroll itself, at which point the directive can be deleted.

Verification

  • npm run lint: clean
  • npx ng test --watch=false --browsers=ChromeHeadless: 160 passing
  • npm run build: clean

Not yet validated against a live instance; the Zeus test host was unreachable from the build session.

MatMenuTrigger restores focus to the trigger when a menu closes, calling
focus() without any FocusOptions. The browser then scrolls the trigger back
into view, so the page jumped back to the scroll position it had when the
menu was opened.

Add a directive that shadows Material's [matMenuTriggerFor] selector, takes
focus restoration over from the trigger and restores focus with
{ preventScroll: true }. Keyboard users still get focus back on the trigger,
but the page no longer moves. Submenu triggers are left alone: they live
inside the fixed overlay and cannot scroll the page.
MenuTriggerRestoreFocusDirective only applies to components that list it in
their imports, and a component that forgets it still compiles. Fail the
frontend job when a component template renders a mat-menu trigger and its
sibling component file does not reference the directive.
The mobile layout scrolls an inner div, but it was never registered with the
CDK ScrollDispatcher, so overlays using the default reposition scroll strategy
never saw its scroll events: an open menu stayed frozen in place while the
content slid underneath it, and tooltips did not hide on scroll.
@Starosdev
Starosdev merged commit 5d196cb into develop Jul 28, 2026
15 checks passed
@Starosdev

Copy link
Copy Markdown
Owner Author

Merged (2026-07-28)

Changes Made

  1. Menu close no longer scrolls the page

    • MenuTriggerRestoreFocusDirective shadows Material's [matMenuTriggerFor] selector, takes focus restoration over from MatMenuTrigger and restores focus with { preventScroll: true }
    • MatMenuTrigger._destroyMenu() restores focus with a bare focus(), so the browser scrolled the trigger back into view and the page jumped to where it was when the menu opened
    • Keyboard focus return to the trigger is preserved, unlike [matMenuTriggerRestoreFocus]="false"
    • Submenu triggers are skipped: they live inside the fixed overlay and cannot scroll the page
  2. CI guard for future menus

    • The directive only applies to components that import it, and a component that forgets it still compiles
    • The frontend job now fails when a component template renders a menu trigger and its sibling component file does not reference the directive
  3. Mobile layout registered as a CDK scroll container

    • cdkScrollable on the mobile content div, which was the scroller on narrow viewports but was never registered with the ScrollDispatcher
    • Open menus stayed frozen in place while content slid underneath them, and tooltips did not hide on scroll
PR #679
Issue #672
Merged into develop
Docker ghcr.io/starosdev/scrutiny:develop-omnibus

Issue #672 stays open until this reaches master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant