Skip to content

Fix Fingerprint provider menu clipping - #862

Closed
KaranocaVe wants to merge 1 commit into
magpie-community:mainfrom
KaranocaVe:fix/fingerprint-provider-menu
Closed

KaranocaVe wants to merge 1 commit into
magpie-community:mainfrom
KaranocaVe:fix/fingerprint-provider-menu

Conversation

@KaranocaVe

Copy link
Copy Markdown
Contributor

The provider menu was clipped by the models panel, making some providers inaccessible. Render it outside the panel, position it within the viewport, and keep mouse and keyboard navigation usable for long lists. Verified with 35 providers in a 420 x 450 viewport, the Fingerprint checks, and the community plugin checks.

@yetone yetone left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at 0945af5. The fix works. Once it has a version bump, it can merge.

What I ran (macOS test box, Bun 1.4.2, Node 22, Playwright 1.62.0, temp HOME):

  • bun install --frozen-lockfile, bun run typecheck (clean), bun test (37 pass, 0 fail), bun run build, then diff -r of the rebuilt dist/ against the committed one: identical (index-BeAnCYkZ.js, index-BNQ_IIC1.css, index.html).

  • bun scripts/check.mjs fingerprint: ✓ registry.json, ✓ fingerprint.

  • I loaded the built page in Playwright, Chromium and WebKit, with a mocked api/bootstrap that has 35 providers (some with long names). I ran it at 420x450, at 1280x800, and at 420x450 with dir=rtl. For each case I checked:

    • the menu is inside the viewport and nothing covers it (elementFromPoint at its corners and centre);
    • the list scrolls;
    • the menu sits 4px from the trigger, and flips upward at 420x450;
    • scrolling down shows the last option, and clicking it selects it;
    • a click on option 20 selects it;
    • ArrowDown opens the menu with focus in the list, then Home, 5×ArrowDown and Enter select the right option and return focus to the trigger;
    • End scrolls the active option into view;
    • Escape closes the menu and returns focus;
    • an outside click and Tab close it;
    • after a page scroll and a viewport resize, the menu is still attached and inside the viewport.

    All 96 checks pass in both engines.

  • I also broke it by hand: the same script against a build of main's AppMenu.tsx/style.css. The last option is hidden in all 6 runs, Chromium at 420x450 clips the menu itself, and End doesn't bring the active option into view. That is the bug, and this PR fixes it.

From reading the code, the portal is removed on unmount, both listeners (resize, and scroll in capture mode) are removed in the layout effect's cleanup, the outside-pointerdown and blur checks include the portaled list, and the theme variables are on :root so they still reach body.

Nits, not blocking:

  • aria-controls={listId} points at an element that only exists while the menu is open. Set it only when open.
  • If the trigger scrolls out of view while the menu is open, the menu follows it off-screen. Closing on that, or clamping, would be tidier.

Needed before merge: a version bump to 0.2.4 in package.json. #861 has no bump either, so one bump covering both is fine. Please also bump VERSION in src/service-identity.ts, which still says 0.2.2 (#860).

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.

2 participants