Skip to content

fix(naked_ui): scroll keyboard focus targets into view in overlays, tabs, toggle groups, and menubars - #344

Open
leoafarias wants to merge 4 commits into
mainfrom
fix/naked-ui-overlay-boundary-scroll
Open

leoafarias wants to merge 4 commits into
mainfrom
fix/naked-ui-overlay-boundary-scroll

Conversation

@leoafarias

@leoafarias leoafarias commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator

Description

Several keyboard jumps in naked_ui requested focus directly on the target item:

  • Home and End in anchored overlays, tab bars, and menubars;
  • the arrow keys, Home, and End in toggle groups;
  • the arrow keys from an open menubar menu (MenubarTriggers.move).

That skips the focus-traversal code that calls Scrollable.ensureVisible. When the content scrolled, the target was focused but left off screen. In a NakedSelect, End then Enter selected an item the user could not see. MenubarTriggers.move could open the next menu under a clipped, invisible trigger.

All of these now call one internal helper, requestFocusInView. It focuses the node, then reveals it with revealInView:

  • It walks the scrollables that contain the node, nearest first, like Scrollable.ensureVisible.
  • For each, it runs ScrollPosition.ensureVisible with keepVisibleAtEnd and then keepVisibleAtStart. Each policy scrolls in only one direction, so the pair reveals the target from either side, in either reading direction, and across loop wraps.
  • A target that is already visible does not move, and containing scrollables, including the page, move only as far as needed. Flutter's default callback (explicit, alignment 1) is not used, because it aligns the target to the trailing edge of every ancestor scrollable, even when the target is already visible.
  • The walk stops at the first scrollable that does not render the target. An overlay is built under its anchor's scrollables but rendered in the Overlay. When that Overlay sits inside another scroll view (a nested Navigator in a PageView, or an app-wide scroll view with a list in the page), following the widget tree scrolled the page to an unrelated offset.

Validation

  • New tests use scrolling lists of 20 to 30 items in a small viewport, plus scrolled pages:
    • End reveals the last item and Home reveals the first, in overlays, tabs, toggle groups, and menubars.
    • Toggle arrow keys reveal the next option, and ArrowLeft onto a fully visible option leaves the strip where it is.
    • Wrapping from the last option to the first, and End in right-to-left, reveal the target.
    • Arrow keys and End in a toggle group, tab bar, and menubar keep a scrolled page where it is. A strip below the fold scrolls both itself and the page.
    • ArrowRight from an open menubar menu reveals the next trigger, and its menu stays open under it.
    • Home and End in a NakedSelect and a NakedMenu whose Overlay is inside a PageView of nested Navigators, or inside an app-wide scroll view with a list in the page, reveal the item in the panel and leave every outer offset unchanged.
  • Without the helper, the page-offset, visible-option, menubar move, and nested-scroll tests fail. The loop-wrap, right-to-left, and plain app-wide scroll view tests pass either way and guard the two-pass design.
  • flutter test in packages/naked_ui: 982 passed. dart analyze: no new issues. Root dart format: clean. melos run ci:checks: passed.

Related Issues

Found while reviewing #341, which makes a long RemixSelect list scroll. Belongs in the naked_ui 1.3.0 release with #317, #330, #335, #338, and #347. The combobox highlight reveal and the select focus-on-open fix (#349) build on this helper.


Checklist

  • My PR includes unit or integration tests for all changed/updated/fixed behaviors.
  • I have updated or added relevant documentation (doc comments with ///). No API or documented behavior changed, so only the CHANGELOG Unreleased entry was added.
  • I am prepared to follow up on review comments in a timely manner.

Breaking Change

  • Yes, this is a breaking change.
  • No, this is not a breaking change.

…lays

Home and End in an anchored overlay (NakedSelect, NakedMenu) requested focus
directly on the first or last item, which skipped the traversal policy's
requestFocusCallback. When the overlay content scrolled, End focused the last
item without revealing it, so End then Enter selected an item the user could
not see. Request focus through the policy, as the arrow keys already do.
…e groups, and menubars

NakedTabBar Home and End, NakedToggleGroup arrow keys, Home, and End, and
NakedMenubar Home and End requested focus directly on the target, which skipped
the traversal policy's requestFocusCallback and left a target inside a
scrollable off screen. Request focus through the policy's callback, as the
arrow keys in tabs and the overlay fix already do.
Focusing through the traversal policy's default callback aligns the target to
the trailing edge of every ancestor scrollable, even when it is already
visible, which scrolled the page and shifted fully visible strip items.
requestFocusInView runs the policy callback with keepVisibleAtEnd and then
keepVisibleAtStart so a target is revealed from either side and left alone when
visible. The anchored overlay, tabs, toggle group, and menubar use it, and
MenubarTriggers.move now reveals the next trigger as well.
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-27T22:06:51.096193Z b0e7e74 New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

An overlay is built under its anchor's scrollables but rendered in the Overlay.
Scrollable.ensureVisible found those scrollables through the widget tree while
each position picked its viewport through the render tree, so with an Overlay
inside another scroll view (a nested Navigator in a PageView, or an app wrapped
in a scroll view) the page was scrolled to an unrelated offset on Home and End.

revealInView walks the containing scrollables like Scrollable.ensureVisible and
stops at the first one that does not render the target. requestFocusInView now
focuses the node and calls it.
leoafarias added a commit that referenced this pull request Oct 27, 2026
leoafarias added a commit that referenced this pull request Oct 27, 2026

This branch has not been deployed

No deployments
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