feat: add sidebar width persistence and restoration #6278
Conversation
- Implemented saving of left sidebar width to user preferences in the Redux store. - Added functionality to restore sidebar width from preferences on application load. - Updated preferences schema to include leftSidebarWidth with validation. - Set default leftSidebarWidth in the preferences to 222.
jeffbowen
requested review from
bijin-bruno,
helloanoop,
lohit-bruno and
naman-bruno
as code owners
WalkthroughPersist sidebar width: dragging the app sidebar now persists the width to the persisted UI snapshot via a new Redux thunk and IPC; startup/hydration flows were extended to read and restore the saved sidebar width into app state. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Sidebar as Sidebar Component
participant Redux as Renderer Redux (thunks/slices)
participant IPC as Renderer<->Main IPC (ipcRenderer/ipcMain)
participant Snapshot as Electron UiStateSnapshot Store
User->>Sidebar: drag resize -> mouseup
Sidebar->>Redux: dispatch saveSidebarWidth(asideWidth)
Redux->>IPC: ipcRenderer.invoke('renderer:save-ui-state-snapshot', { type: "SIDEBAR", data: { width } })
IPC->>Snapshot: updateSidebarWidth(width) / persist
Snapshot-->>IPC: persist success
IPC-->>Redux: resolve thunk
Note over Snapshot,Redux: On app start / renderer readiness
Snapshot->>IPC: main:hydrate-app-with-ui-state-snapshot({ sidebar: { width }, collections: { ... } })
IPC->>Redux: hydrate actions (updateLeftSidebarWidth, collection hydrate)
Redux->>Sidebar: state updated -> re-render with persisted width
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
packages/bruno-electron/src/store/preferences.js (1)
41-44: Layout default and validation look consistent; consider centralizing boundsThe new
layout.leftSidebarWidthdefault (222) and schema range (221–600) align with the UI’s min/max and the app slice initial state. This is good for keeping persisted values sane.To avoid future drift, it might be worth documenting or centralizing these bounds so any change to sidebar limits is updated in both the Electron schema and the React/Redux side together.
Also applies to: 88-91
packages/bruno-app/src/providers/ReduxStore/slices/app.js (1)
144-163: Persistence flow is correct; you can reusesavePreferencesto avoid duplicationThe thunk correctly patches
layout.leftSidebarWidth, persists via IPC, and syncsstate.app.preferences, so behavior is sound.Since the logic closely matches
savePreferences, you could simplify to:export const saveSidebarWidth = (leftSidebarWidth) => (dispatch, getState) => { const currentPreferences = getState().app.preferences; const updatedPreferences = { ...currentPreferences, layout: { ...currentPreferences.layout, leftSidebarWidth } }; return dispatch(savePreferences(updatedPreferences)); };This keeps all IPC + validation handling in
savePreferencesand reduces maintenance surface.packages/bruno-app/src/providers/App/useIpcEvents.js (1)
6-8: Startup restoration works; consider a stricter check than truthinessImporting
updateLeftSidebarWidthand restoring it onmain:load-preferencescorrectly wires persisted layout into the UI state.To make this a bit more robust against future changes (e.g., if
0ornullbecomes meaningful), you might prefer an explicit check over a truthy test:if (val?.layout && val.layout.leftSidebarWidth != null) { dispatch(updateLeftSidebarWidth({ leftSidebarWidth: val.layout.leftSidebarWidth })); }This still ignores
null/undefinedbut doesn’t silently couple behavior to “truthy”.Also applies to: 164-171
packages/bruno-app/src/components/Sidebar/index.js (1)
7-7: Drag-end persistence is wired correctly; optional optimization for redundant savesImporting
saveSidebarWidthand dispatching it onhandleMouseUpafterupdateLeftSidebarWidthcleanly persists the final, clamped width and matches the intended UX.If you want to reduce unnecessary preference writes, you could guard the save:
if (asideWidth !== leftSidebarWidth) { dispatch(saveSidebarWidth(asideWidth)); }Not required for correctness, but it avoids IPC + disk I/O when the width hasn’t actually changed.
Also applies to: 35-51
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/bruno-app/src/components/Sidebar/index.js(2 hunks)packages/bruno-app/src/providers/App/useIpcEvents.js(2 hunks)packages/bruno-app/src/providers/ReduxStore/slices/app.js(1 hunks)packages/bruno-electron/src/store/preferences.js(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single parameters
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses. Keep 'em tight
Space before and after the arrow in arrow functions.() => {}is good
No space between function name and parentheses.func()notfunc ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Avoid single line abstractions where all that's being done is increasing the call stack with one additional function
Add in meaningful comments instead of obvious ones where complex code flow is explained properly
Files:
packages/bruno-app/src/providers/ReduxStore/slices/app.jspackages/bruno-app/src/components/Sidebar/index.jspackages/bruno-electron/src/store/preferences.jspackages/bruno-app/src/providers/App/useIpcEvents.js
🧬 Code graph analysis (1)
packages/bruno-app/src/components/Sidebar/index.js (1)
packages/bruno-app/src/providers/ReduxStore/slices/app.js (2)
saveSidebarWidth(144-163)saveSidebarWidth(144-163)
- reuse savePreferences to avoid duplication - consider a stricter check than truthiness on main:load-preferences - only saveSidebarWidth if it's changed TODO: Consider centralizing sidebar width/bounds. > To avoid future drift, it might be worth documenting or centralizing these bounds so any change to sidebar limits is updated in both the Electron schema and the React/Redux side together. This is outside the scope of this change as other values also do not use constants. They should match `MIN_LEFT_SIDEBAR_WIDTH` / `MAX_LEFT_SIDEBAR_WIDTH` defined in `packages/bruno-app/src/components/Sidebar/index.js` though.
|
Hey @jeffbowen Thanks for the PR. We'd want to save the state of the sidebar width in ui-snapshot-store Can you update your PR to add it there so that the final store structure looks like this. {
collections: //existing
sidebar: {
width: <sidebarWidth>
}
} |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/bruno-electron/src/ipc/preferences.js (1)
31-40: Consider reusing UiStateSnapshotStore instance and aligning error log with existing styleThe sidebar hydration block is functionally fine, but two minor improvements:
You’re instantiating
new UiStateSnapshot()inside therenderer:readyhandler every time. Given this is a simpleelectron-storewrapper it’s not a big deal, but you could instantiate it once at module scope and reuse it to avoid repeated construction.The error message
'Error loading UI state snapshot'is good; consider including the stack or using a consistent prefix format (similar to the global-environments catch above) if you want logs to be easy to grep.No change required if you’re happy with the current pattern; just something to keep in mind.
packages/bruno-electron/src/app/collection-watcher.js (1)
656-665: Tighten “no collections snapshot” handling when hydrating UI stateThis wiring looks consistent with the renderer’s
hydrateCollectionWithUiStateSnapshotconsumer, but there are two small polish points:
- When
collectionSnapshotStateis falsy, you currently send{ collections: {} }. On the renderer side,{}is truthy and triggers hydration logic with nopathname. It’s harmless but slightly wasteful. You could instead skip sending in that case:- const uiSnapshotState = { collections: collectionSnapshotState || {} }; - win.webContents.send('main:hydrate-app-with-ui-state-snapshot', uiSnapshotState); + if (collectionSnapshotState) { + const uiSnapshotState = { collections: collectionSnapshotState }; + win.webContents.send('main:hydrate-app-with-ui-state-snapshot', uiSnapshotState); + }
- As with
preferences.js, you might eventually want a sharedUiStateSnapshotStoreinstance instead of instantiating per call, though it’s not performance‑critical here.Functionally this is fine; above is optional cleanup.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/bruno-app/src/providers/App/useIpcEvents.js(1 hunks)packages/bruno-app/src/providers/ReduxStore/slices/app.js(1 hunks)packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js(2 hunks)packages/bruno-electron/src/app/collection-watcher.js(1 hunks)packages/bruno-electron/src/ipc/preferences.js(2 hunks)packages/bruno-electron/src/store/ui-state-snapshot.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/bruno-app/src/providers/ReduxStore/slices/app.js
- packages/bruno-app/src/providers/App/useIpcEvents.js
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single parameters
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses. Keep 'em tight
Space before and after the arrow in arrow functions.() => {}is good
No space between function name and parentheses.func()notfunc ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Avoid single line abstractions where all that's being done is increasing the call stack with one additional function
Add in meaningful comments instead of obvious ones where complex code flow is explained properly
Files:
packages/bruno-electron/src/store/ui-state-snapshot.jspackages/bruno-electron/src/app/collection-watcher.jspackages/bruno-app/src/providers/ReduxStore/slices/collections/actions.jspackages/bruno-electron/src/ipc/preferences.js
🧬 Code graph analysis (1)
packages/bruno-electron/src/ipc/preferences.js (1)
packages/bruno-electron/src/app/collection-watcher.js (14)
UiStateSnapshot(27-27)require(5-11)require(12-18)require(19-19)require(21-21)require(22-22)require(23-23)require(24-24)require(25-25)require(28-28)require(29-29)require(30-30)UiStateSnapshotStore(660-660)uiSnapshotState(663-663)
🪛 Biome (2.1.2)
packages/bruno-electron/src/store/ui-state-snapshot.js
[error] 68-68: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 72-72: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
packages/bruno-app/src/providers/ReduxStore/slices/collections/actions.js
Show resolved
Hide resolved
|
@helloanoop Gotcha. I moved to using ui-state-snapshot to store the sidebar width. To keep this pull request limited to just one feature, I avoided messing with existing collections stuff and tried to mirror the way collections ui-state-snapshot already works. But it seems like there could be a separate PR down the road to unify them a bit. I was unable to get the sidebar width to update from Tested and working. |
|
Looks like #6264 introduced some merge conflicts. |
2 participants
Description
Fixes #635
Contribution Checklist:
Note: Keeping the PR small and focused helps make it easier to review and merge. If you have multiple changes you want to make, please consider submitting them as separate pull requests.
Publishing to New Package Managers
Please see here for more information.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.