Skip to content

fix: polish instance picker, fix modal focus rings, and hide remote storage info#985

Open
MaanilVerma wants to merge 4 commits into
Comfy-Org:mainfrom
MaanilVerma:fix/picker-about-focus-polish
Open

fix: polish instance picker, fix modal focus rings, and hide remote storage info#985
MaanilVerma wants to merge 4 commits into
Comfy-Org:mainfrom
MaanilVerma:fix/picker-about-focus-polish

Conversation

@MaanilVerma

Copy link
Copy Markdown
Collaborator

Summary

Polish pass on the launcher's instance picker and per-instance About tab, plus a focus-trap fix shared by every modal/takeover. Opening any dialog no longer flashes a focus ring, the picker list now animates when filtered, and the About tab stops showing local-storage rows for cloud/remote connections.

Changes

What

  • BrandTakeoverLayout / BaseModal / BaseAlert / WhyTryCloudModal / MigrateConfirmTakeover: focus the role="dialog" container (tabindex="-1") on open instead of the first button, so opening via mouse/programmatic focus no longer paints a :focus-visible ring. Focus stays trapped for keyboard/AT users — they Tab from the container to reach controls.
  • TrackModal / LoadSnapshotModal: on open, prefer a real input/select (lets the user type immediately), otherwise fall back to the dialog container — never a button. This fixes LoadSnapshotModal's "Browse" button getting the ring in its empty state.
  • InstancePickerView: wrap the left instance list in a <TransitionGroup name="picker-row"> reusing ChooserView's tile motion (enter rise+fade 200ms, leave fade-out-of-flow 140ms, FLIP move 220ms iOS curve) so search/filter reflows smoothly. Added a lockLeavingRowSize FLIP hook and a prefers-reduced-motion fallback.
  • StatusFactPanel: hide the "Location & storage" group when sourceCategory !== 'local' — cloud/remote connections have no local footprint (also covers the "no disk data" case, since diskUsage is null for them).
  • StatusFactPanel: show the remote-URL edit pencil at rest (opacity: 0.6) instead of hover-only; dropped the redundant row-hover reveal rule.
  • ProgressModal: replace the clipped stack ellipse scrim with a centered blurred circle behind the wordmark/bar/text.

Breaking

  • None. No IPC channels, props, or stores changed signatures; the editable remote-URL row is unaffected (its remote.url = "URL" label lands under Instance details, not Location & storage).

Review Focus

  • The dialog-container focus pattern is the crux — confirm focus is still trapped (ESC closes, Tab cycles within) on each modal, just without the on-open ring.
  • StatusFactPanel gate: verify cloud/remote instances lose only the storage group and keep the editable URL row.
  • Scope is launcher/picker + shared modal focus only. Other surfaces that focus a text input on open (ModalDialog, BaseSelect, BaseMenu, BaseActionSheet) were deliberately left unchanged — inputs don't exhibit the button-ring issue.

Testing

UI/visual changes with no logic-path impact, so verification is manual + the existing typecheck/lint gauntlet; no new unit tests (they'd only re-assert unchanged store/IPC behavior).

  • pnpm typecheck (node/web/e2e/integration) — clean.
  • pnpm lint — clean.
  • Manual: open each modal/takeover → no focus ring on open, ring appears on first Tab. Filter the picker list → rows cross-fade and slide. About tab on a local vs cloud/remote instance → storage group present only for local; URL pencil visible at rest for remote.

- BrandTakeoverLayout/BaseModal/BaseAlert/WhyTryCloudModal/MigrateConfirmTakeover: focus the role="dialog" container (tabindex="-1") instead of the first button, so opening no longer paints a focus-visible ring; keyboard focus stays trapped.
- TrackModal/LoadSnapshotModal: prefer a real input/select, else fall back to the dialog container — never a button.
- Drop now-unused button refs.
- InstancePickerView: wrap the left instance list in a TransitionGroup reusing ChooserView's tile motion (enter/leave fade + FLIP move) so filtering reflows smoothly.
- StatusFactPanel: hide the "Location & storage" group for cloud/remote instances (no local footprint); keep the editable remote URL row, which lives under Instance details.
- StatusFactPanel: show the URL edit pencil at rest (opacity 0.6) instead of hover-only.
- ProgressModal: replace the clipped stack scrim with a centered blurred circle behind the wordmark.
@MaanilVerma MaanilVerma changed the title fix: Polish instance picker, fix modal focus rings, and hide remote storage info fix: polish instance picker, fix modal focus rings, and hide remote storage info Jun 8, 2026
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