Skip to content

SY-4174: Make windowKey Optional in Nav Drawer Actions#2313

Merged
emilbon99 merged 5 commits into
rcfrom
sy-4174-make-windowkey-optional-in-nav-drawer-actions
May 13, 2026
Merged

SY-4174: Make windowKey Optional in Nav Drawer Actions#2313
emilbon99 merged 5 commits into
rcfrom
sy-4174-make-windowkey-optional-in-nav-drawer-actions

Conversation

@emilbon99
Copy link
Copy Markdown
Contributor

@emilbon99 emilbon99 commented May 12, 2026

Issue Pull Request

Linear Issue

SY-4174

Description

Removes the boilerplate of selecting windowKey at every call site that dispatches setNavDrawerVisible. A new injectNavDrawerWindowKey layout middleware reads the current window key from drift state and injects it into the action payload when the caller omits one. The reducer still throws if a windowKey is missing by the time it runs, so the invariant is preserved.

Cleans up the useSelectWindowKey() lookup, the as string cast, and the corresponding useCallback dep at five consumers: arc/editor/graph/Editor.tsx, lineplot/LinePlot.tsx, log/Log.tsx, schematic/Schematic.tsx, and table/Table.tsx. Call sites that intentionally target a non-current window (e.g. useNavDrawer.ts, nav/useTriggers.ts, layouts/nav/Menu.tsx) keep their explicit windowKey and are unaffected.

Basic Readiness

  • I have performed a self-review of my code.
  • I have added relevant, automated tests to cover the changes.
  • I have updated documentation to reflect the changes.

Greptile Summary

This PR eliminates repetitive useSelectWindowKey() boilerplate from five layout renderers by introducing an injectNavDrawerWindowKey Redux middleware that reads the current window key from Drift state and injects it into setNavDrawerVisible actions that omit one. The reducer retains its guard and throws if a windowKey still arrives missing.

  • console/src/layout/middleware.ts: New injectNavDrawerWindowKey middleware added as the first entry in MIDDLEWARE; it short-circuits on matching actions that already carry a windowKey.
  • console/src/layout/slice.ts: SetNavDrawerVisiblePayload.windowKey made optional (?); reducer throws UnexpectedError if the key is absent after middleware runs.
  • console/src/layout/slice.spec.ts: Comprehensive new test file covering the reducer, nav drawer actions, mosaic operations, and workspace management (no middleware integration test included).

Confidence Score: 5/5

Safe to merge; the middleware correctly injects window key for the five consumers and call sites that already carry an explicit windowKey are untouched.

The change is well-scoped: a single middleware intercepts one action type, the reducer retains its invariant guard, and five consumers are simplified in a uniform way. Call sites that intentionally target non-current windows keep their explicit windowKey and are unaffected.

No files require special attention beyond the optional middleware integration test gap noted in slice.spec.ts.

Important Files Changed

Filename Overview
console/src/layout/middleware.ts Adds injectNavDrawerWindowKey middleware; correctly short-circuits on actions with a windowKey already set and injects from Drift state otherwise.
console/src/layout/slice.ts windowKey made optional in SetNavDrawerVisiblePayload; reducer guard preserved via UnexpectedError throw.
console/src/layout/slice.spec.ts New comprehensive test file covering slice reducer; does not include an integration test for the injectNavDrawerWindowKey middleware itself.
console/src/arc/editor/graph/Editor.tsx Removed useSelectWindowKey hook and simplified handleDoubleClick callback and its dependency array.
console/src/lineplot/LinePlot.tsx Removed useSelectWindowKey hook and windowKey from handleDoubleClick dependency array.
console/src/log/Log.tsx Removed useSelectWindowKey hook and simplified handleDoubleClick callback.
console/src/schematic/Schematic.tsx Removed useSelectWindowKey hook and simplified handleDoubleClick callback and its dependency array.
console/src/table/Table.tsx Removed useSelectWindowKey hook; handleDoubleClick dep array unchanged at [canEdit] (pre-existing).

Sequence Diagram

sequenceDiagram
    participant C as Component (e.g. LinePlot)
    participant M as injectNavDrawerWindowKey Middleware
    participant D as Drift State
    participant R as Layout Reducer

    C->>M: dispatch setNavDrawerVisible without windowKey
    M->>M: Check if windowKey is absent
    M->>D: selectWindowKey from store state
    D-->>M: returns current windowKey
    M->>R: forward action with injected windowKey
    R->>R: Update nav drawer state

    note over M,R: If Drift returns null, the unmodified action is forwarded and the reducer throws UnexpectedError
Loading

Reviews (5): Last reviewed commit: "Use exported action creators directly in..." | Re-trigger Greptile

Make `windowKey` optional on `setNavDrawerVisible`. A new layout
middleware fills it in from drift state when the call site omits it,
so consumers no longer need to call `useSelectWindowKey()` and
thread it through `useCallback` deps. Drops the boilerplate from
Editor, LinePlot, Log, Schematic, and Table. The reducer still
throws if a windowKey isn't present at the time it runs.
Comment thread console/src/layout/middleware.ts Outdated
@codecov
Copy link
Copy Markdown

codecov Bot commented May 12, 2026

Codecov Report

❌ Patch coverage is 21.42857% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.07%. Comparing base (1214e6c) to head (97aaa8b).
⚠️ Report is 2 commits behind head on rc.

Files with missing lines Patch % Lines
console/src/layout/middleware.ts 14.28% 4 Missing and 2 partials ⚠️
console/src/arc/editor/graph/Editor.tsx 0.00% 1 Missing ⚠️
console/src/lineplot/LinePlot.tsx 0.00% 1 Missing ⚠️
console/src/log/Log.tsx 0.00% 1 Missing ⚠️
console/src/schematic/Schematic.tsx 0.00% 1 Missing ⚠️
console/src/table/Table.tsx 0.00% 1 Missing ⚠️

❌ Your patch check has failed because the patch coverage (21.42%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##               rc    #2313      +/-   ##
==========================================
+ Coverage   64.97%   65.07%   +0.10%     
==========================================
  Files        2603     2603              
  Lines      113946   113826     -120     
  Branches     8399     8374      -25     
==========================================
+ Hits        74035    74073      +38     
+ Misses      33774    33640     -134     
+ Partials     6137     6113      -24     
Flag Coverage Δ
console 23.08% <21.42%> (+1.32%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@emilbon99 emilbon99 merged commit 1cca8c2 into rc May 13, 2026
25 of 27 checks passed
@emilbon99 emilbon99 deleted the sy-4174-make-windowkey-optional-in-nav-drawer-actions branch May 13, 2026 17:23
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