fix(ui): make I hotkey work in Canvas, Gallery, and Viewer

[DustyShoe]

Summary

Follow-up to #9482.

The Canvas eyedropper and Viewer metadata panel both use I. Disabled hotkeys were wrapped in an enabled callback, causing react-hotkeys-hook to keep their listeners registered. A disabled listener could then block the active handler for the same key.

This keeps enabled: false static so disabled hotkeys are not registered, while preserving the Canvas Text session guard and custom predicates. As a result, disabled handlers also no longer suppress native browser behavior when no enabled application hotkey owns the key.

The shared I binding is now routed according to whether Canvas or Viewer is active in the central workspace, even when focus moves to Gallery in the right sidebar:

• When Canvas is active, I selects the eyedropper after interacting with Gallery.
• When Viewer is active, I toggles the metadata panel for the selected Gallery item without requiring another click in Viewer.

The metadata hotkey is registered independently of the conditionally rendered toolbar button, preventing gaps while selection data is resolving. Hotkey options are memoized so unrelated renders do not tear down and re-register the listeners.

Related Issues / Discussions

Re-Closes #9467

QA Instructions

• Switch to Canvas and confirm I selects the eyedropper.
• Select another Canvas tool and confirm I selects the eyedropper again.
• Switch to Viewer and confirm I toggles the metadata panel.
• Select another image in Gallery and confirm that I toggles the info panel without clicking the Viewer.

Checklist

• The PR has a short but descriptive title, suitable for a changelog
• Tests added / updated (if applicable)
• Changes to a redux slice have a corresponding migration
• Documentation added / updated (if applicable)
• Updated What's New copy (if doing a release after this PR)

[lstein]

Adversarial review of 9eb484c. I verified the premise in react-hotkeys-hook@4.5.0 before judging the change, and it holds:

• useHotkeys' layout effect returns early when options.enabled === false, so no listener is attached (dist/react-hotkeys-hook.esm.js:389).
• When enabled is a function, the listener is attached and, on a key match, calls stopPropagation(e) → stopImmediatePropagation() + preventDefault() before consulting the predicate (:424-427). Every registered hotkey binds to document, so a disabled-by-predicate handler suppresses every same-key handler registered after it.

So the generic fix in getRegisteredHotkeyOptions is the right shape. Checks on my side: the 3 new tests pass with no type errors, eslint and prettier are clean on the changed files. The branch is BEHIND main but mergeable.

One blocker below, plus some non-blocking notes.

---

Blocker — I becomes a dead key on Canvas when the gallery is focused with nothing selected

ToolColorPickerButton.tsx now gates the eyedropper on !isGalleryFocused && !isViewerFocused, on the assumption that the metadata handler always covers the complement. It doesn't:

• ToggleMetadataViewerButton is only rendered when galleryItem?.kind === 'image' | 'video' (ImageViewerToolbar.tsx:28) — i.e. only when an item is selected and its DTO has resolved.
• gallery.selection starts as [] and is in persistDenylist (gallerySlice.ts:22, :233), so every page reload starts with no selection. It is also emptied by imageSelected(null) (:52) and showVirtualBoardsChanged (:150).
• The panel container is tabIndex={-1} (AutoLayoutPanelContainer.tsx:45), so clicking gallery whitespace, the scrollbar, or an empty board focuses the gallery region without selecting anything.

Repro: reload → Canvas tab → click empty space in the gallery panel → press I. Nothing happens.

On main the eyedropper hotkey passed no options at all, so it was unconditionally enabled, and with no selection there was no metadata listener to mask it — I selected the eyedropper. So this is a regression against main for the very key the PR is fixing. It also hits transiently while useGalleryItemDTO is still resolving right after a selection.

The registration site is the root of it: toggleMetadata lives inside the viewer toolbar, whose render conditions are strictly narrower than the focus predicate now gating the eyedropper. GlobalImageHotkeys.tsx:39 already establishes isFocusOK = isGalleryFocused || isViewerFocused for exactly this family of hotkeys and registers them unconditionally at app level — that's where this one belongs. Note it can't be moved verbatim: GlobalImageHotkeys filters out videos (:23) and the metadata toggle supports them.

Non-blocking

1. isDisabledOverride leaves a second dead-key hole. With a progress image showing and not temporarily overridden, toggleMetadata is enabled: false → unregistered, and the eyedropper is off because gallery/viewer holds focus → I does nothing. Same outcome on main (for a different reason), so not a regression, but QA step 3 in the description ("Switch to Viewer and confirm I toggles the metadata panel") fails in that state.

2. The generic change also drops preventDefault masking app-wide. maybePreventDefault runs before the enabled check, so statically-disabled hotkeys used to swallow their key's browser default. They no longer do. Concretely: workflows.selectAll is mod+a + preventDefault gated on isWorkflowsFocused (Flow.tsx:581), and gallery.selectAllOnPage is mod+a + preventDefault gated on isGalleryFocused (GallerySelectionCountTag.tsx:27). Focus the left panel on the Workflows tab and Ctrl/Cmd+A now select-alls the page text; previously one of those two disabled listeners ate it. Same class for mod+c/mod+v/mod+z and delete/backspace. This looks like an acceptable trade for the fix, but it is an app-wide behaviour change that deserves a line in the PR description.

3. The helper's stated guarantee doesn't cover predicate gates. getRegisteredHotkeyOptions short-circuits only a literal false. Two call sites pass a function (useNextPrevEntity.ts:76,95), which still register listeners that stopImmediatePropagation when the predicate returns false — the exact bug class the doc comment claims to close. Harmless today (both merely duplicate the text-session guard), but the comment overstates what is enforced.

4. Tests don't cover the behavioural change. The 3 new tests exercise only the pure helper; nothing pins which region owns I, which is the risky half of the diff. The enabled: undefined → true branch is untested. And expect(...).toBe(options) asserts reference identity, which is stronger than the contract — a correct {...options} refactor would fail it.

5. Listener churn on the eyedropper. ToolColorPickerButton now passes an inline options literal, so _options gets a fresh identity every render → a fresh enabled closure → deepEqual compares functions by reference (dist:265) → the document keydown/keyup listeners are torn down and re-added on every render. On main it registered once. This is idiomatic for the codebase (every other call site does the same), but it does keep moving the eyedropper listener to the end of the document listener list.

Attacks that failed

For completeness, things I tried to break and couldn't:

• Double-fire: eyedropper !(g‖v) and metadata (g‖v) && … are mutually exclusive — no state fires both.
• Duplicate registration: only one tab layout mounts at a time (AppContent.tsx:48-49). ImageViewerPanel does render two <ImageViewer/> when !lastSelectedItem (ImageViewerPanel.tsx:23,25), but in that state the metadata button isn't rendered at all, so no double-toggle.
• Hidden viewer panel: dockview's default onlyWhenVisible only detaches the DOM element (dockview.cjs.js:4996); the React portal survives (ReactPart.createPortal, :11290), so the viewer's hotkeys stay live behind the Canvas tab — which is what made #9467 reproducible in the first place.
• Losing the canvas text-session guard: skipping the wrapper for enabled: false can't weaken it — an unregistered hotkey can't fire.
• Newly-unmasked esc/enter collisions: the canvas apply/cancel handlers all gate on isCanvasFocused and clearSelection on isGalleryFocused — disjoint, so unmasking them doesn't create new double-fires.

[DustyShoe]

Thanks for the thorough review.

Blocker - addressed. I is now routed according to the active central panel rather than Gallery focus alone. When Canvas is active, interacting with the adjacent Gallery no longer blocks the eyedropper. When Viewer is active, selecting another Gallery item keeps I assigned to metadata without requiring another click in Viewer. The metadata hotkey is also registered independently of the conditionally rendered toolbar button.

1. Progress image override - expected behavior. After clicking a Gallery thumbnail, the selected image is displayed briefly. During that interval, the existing info button turns blue when activated but does not display metadata. The I hotkey triggers the same UI state and behavior as that button, so this is not a regression introduced by the PR.

2. Browser-default masking - acknowledged. Preserving literal enabled: false means disabled handlers no longer mount solely to suppress another handler or the browser default. That is an intentional tradeoff of the generic fix and should be called out in the PR description.

3. Predicate gates - acknowledged. The helper only avoids mounting a listener when enabled is literally false. Predicate-based gates remain mounted by react-hotkeys-hook; the helper documentation should state that limitation explicitly. The existing predicate call sites are unchanged by this PR.

4. Test coverage - addressed. The follow-up adds logic-level regression coverage for I ownership across active Canvas and Viewer states, Gallery and Viewer focus, and a missing metadata target. It also covers enabled: undefined and replaces the reference-identity assertion with value equality. The rendered interaction remains part of manual QA because the repository has no approved DOM test framework.

5. Listener churn - addressed. The eyedropper and metadata hotkey options are now memoized, so unrelated component renders no longer cause listener teardown and re-registration.

[lstein]

Adversarial re-review of c6b92a5 (+ the main merge b364f79). I tried to break the new routing rather than confirm it; the attack list is at the bottom. No blockers. The round-1 blocker is fixed: toggleMetadata now registers in ImageViewerPanelContent regardless of selection, and the pure getSharedInfoHotkeyTarget makes the two I owners mutually exclusive by construction (same inputs, one function) instead of by two hand-written complements. Tests, eslint, prettier, knip and dpdm are green locally; lint:tsc fails only on the uninstalled html-to-image dep from #9501 in my worktree, and CI is green.

Non-blocking, in priority order:

1. isViewerPanelActive is a render-time read of a plain Map, so the routing is correct by three invariants rather than by subscription. useSharedInfoHotkeyTarget.ts:14 → navigation-api.ts:750 reads _currentActiveDockviewPanel (:88), which nothing subscribes to. It works today because (a) every write to the Map is immediately followed by a focus-region write (:278-280, :282-286) and that write always changes the atom (nanostores set is a no-op on an equal value), (b) both consumers mount strictly after registerContainer has populated the Map (the viewer panel is itself a dockview panel; the eyedropper button sits behind CanvasManagerProviderGate, whose manager is created inside the workspace panel), and (c) the previous tab's AutoLayoutPanelContainer cleanups null the focus atom before the next tab renders, so the restore-time write notifies. I traced every path I could find (tab-header pointerdown vs dockview activation order, drag-enter activation, focusPanel*/toggleViewerPanel, reload/restore, split groups, blurActiveElement) and they all hold. The fragility is that if any invariant breaks (a new dockview panel without a focusRegion param, a Map write outside those two sites, a consumer mounted outside a dockview panel), the two hooks can hold different stale values: ToolColorPickerButton re-renders alone on tool changes, useToggleMetadataHotkey re-renders alone on progress-image changes, and then both listeners are mounted and I double-fires (react-hotkeys-hook does not stop propagation after an enabled callback, esm.js:428-432). The codebase's own precedent reads this Map inside an event handler, not at render (GalleryImageGrid.tsx:129). Cheapest hardening: mirror the active dockview panel into a nanostores atom on NavigationApi and useStore it here. Fine as a follow-up.

2. The wiring is untested; only the pure function is. getSharedInfoHotkeyTarget is fully pinned (dropping any input or flipping the || fails a case), but nothing under *.test.ts* imports useSharedInfoHotkeyTarget, useToggleMetadataHotkey or selectHasMetadataViewerItem. All of these mutations pass every test: VIEWER_PANEL_ID → WORKSPACE_PANEL_ID at useSharedInfoHotkeyTarget.ts:14; swapping 'metadata'/'colorPicker' at the two registration sites; dropping !imageToCompare from gallerySelectors.ts:96; removing the useMemo behind the no-churn claim. I accept the manual-QA position for the DOM interaction, but a hook-level test with the repo's happy-dom docblock pattern would pin the selector and the panel id without a DOM framework. Optional.

3. Progress override leaves I with no listener at all. Viewer active, viewer/gallery focused, item selected, generation running with progress shown: metadata is enabled: false (useToggleMetadataHotkey.ts:25) and the eyedropper's target is still 'metadata' so it is enabled: false too. You called this expected in the thread and I agree it matches the button, but note it means QA step 3 fails during a generation, and pre-PR the eyedropper still fired on the hidden canvas there. Worth one line in the description.

4. Rebound keys. If a user rebinds canvas.selectColorPickerTool via custom hotkeys, it is still suppressed whenever the target is 'metadata', and a rebound viewer.toggleMetadata is still gated on isViewerPanelActive && hasMetadataViewerItem. Harmless (the canvas is hidden in the suppressed state), but the "shared key" rationale in the description does not cover it.

5. Pre-existing, out of scope, FYI: right- or middle-click on the Canvas tab header sets focus 'canvas' (DockviewTabCanvasWorkspace.tsx:31 has no button check) while dockview only activates on button === 0 (tabs.js:236-240), so focus is 'canvas' with the viewer still active and I selects the eyedropper on the hidden canvas. Pre-PR behaved the same.

Verified as claimed: listener churn is gone (useRegisteredHotkeys memoizes on [data.isEnabled, options], the text-session guard is a stable useCallback, and useHotkeys' effect deps are [_keys, memoisedOptions, enabledScopes], so re-registration happens only on a target flip); the enabled: false branch never mounts a listener; comparison mode and no-selection fall back to the eyedropper exactly as pre-PR main did (the eyedropper had no gate at all before this PR).

Attacks that failed: same-value focus write masking a Map change (needs focus already equal to the newly activated panel's region; only 'canvas' is settable out-of-band and that routes to 'colorPicker' regardless of the Map); tab switch into Canvas with the viewer restored active; dockview tab activation ordering (activates in the same pointerdown, before React's flush); drag-enter activation; every programmatic setActive path; active panel removal (no removePanel callers, tabs locked); persisted layouts predating focusRegion (params since fa72a97, v3→v4 migration resets panels); double registration of the metadata hotkey (one viewer panel per tab, one layout mounted); keydown interleaving between notify and re-render (sync-lane flush in the same task); losing the canvas text-session guard.