feat(widgets): persist Beszel and Docker table layouts - #6490
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughChangesWidget table layout and interaction updates
Sequence Diagram(s)sequenceDiagram
participant User
participant WidgetTable
participant usePersistedTableLayout
participant WidgetOptionsMutation
User->>WidgetTable: Reorders or resizes columns
WidgetTable->>usePersistedTableLayout: Reports layout change
usePersistedTableLayout->>WidgetOptionsMutation: Saves layout after debounce
WidgetOptionsMutation-->>WidgetTable: Returns save success or failure
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review Triggered by Codex (OpenAI) to obtain the requested preview review for this draft PR. |
|
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/widgets/src/docker/component.tsx (1)
364-400: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winEvery drag/resize step triggers a
setOptions+ server mutation.
columnsWidthupdates while resizing produce onesaveItemOptionscall per change. Consider debouncingpersistLayout(e.g. 300–500ms trailing) to avoid a burst of writes per interaction.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/widgets/src/docker/component.tsx` around lines 364 - 400, Debounce the persistLayout call in the layout-change useEffect so rapid columnsWidth updates during resizing, and order updates during dragging, produce only a trailing save after roughly 300–500ms of inactivity. Preserve the existing layout-building and change-detection logic, and ensure the debounce is cancelled or cleaned up when dependencies change or the component unmounts.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/widgets/src/docker/component.tsx`:
- Around line 57-89: Extract the duplicated table-layout parsing, hydration, and
persistence logic into a shared module/hook parameterized by accessors,
store-key prefix, columns, options, and setOptions. In
packages/widgets/src/docker/component.tsx lines 57-89, move parseColumnOrder and
parseColumnWidths into the shared module; in
packages/widgets/src/beszel-system-table/component.tsx lines 92-124, import
those shared parsers and retain local re-exports only if required by specs.
Replace the effects at packages/widgets/src/beszel-system-table/component.tsx
lines 395-451 and packages/widgets/src/docker/component.tsx lines 344-400 with
the shared hook.
- Around line 149-176: Update the column definitions for the cpuUsage and
memoryUsage accessors in the container columns configuration to include
sortable: sortingEnabled, matching the existing name, state, and host columns so
their headers support sorting and display sort indicators.
---
Nitpick comments:
In `@packages/widgets/src/docker/component.tsx`:
- Around line 364-400: Debounce the persistLayout call in the layout-change
useEffect so rapid columnsWidth updates during resizing, and order updates
during dragging, produce only a trailing save after roughly 300–500ms of
inactivity. Preserve the existing layout-building and change-detection logic,
and ensure the debounce is cancelled or cleaned up when dependencies change or
the component unmounts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b6a5ed5e-70fb-4924-8778-c5e3fbcc61d8
📒 Files selected for processing (11)
apps/docs/docs/widgets/beszel-system-table/index.mdxapps/docs/docs/widgets/docker-containers/index.mdxapps/docs/docs/widgets/docker-containers/index.tspackages/translation/src/lang/en.jsonpackages/widgets/src/beszel-system-table/component.spec.tspackages/widgets/src/beszel-system-table/component.tsxpackages/widgets/src/beszel-system-table/index.tspackages/widgets/src/docker/component.spec.tspackages/widgets/src/docker/component.tsxpackages/widgets/src/docker/index.tspackages/widgets/src/docker/styles.css
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/widgets/src/beszel-system-table/component.tsx (1)
341-341: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNarrow the
columnsmemo dependencies to the flags it reads.The memo depends on the whole
optionsobject.columnsfeedsvisibleAccessors,visibleAccessorSet, andrestoredOrderinsideusePersistedTableLayout. Ifoptionsreceives a new identity on each render, those derived values change identity too, and the hydration effect inpackages/widgets/src/common/use-persisted-table-layout.tsat Lines 87-104 never completes. Listing the individual flags removes that coupling.♻️ Proposed dependency change
- }, [options, t, size]); + }, [ + options.showCpu, + options.showMemory, + options.showDisk, + options.showGpu, + options.showLoadAvg, + options.showNet, + options.showTemp, + options.showBattery, + options.showServices, + options.showUptime, + options.showAgent, + t, + size, + ]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/widgets/src/beszel-system-table/component.tsx` at line 341, Update the columns useMemo dependency list to remove the whole options object and include only the individual option flags read by the columns computation. Keep t and size, and ensure every referenced options flag remains represented so derived layout values retain stable identity when unrelated options change.packages/widgets/src/common/use-persisted-table-layout.ts (1)
136-146: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueHandle all accessor keys in each
columnsWidthentry.
columnsWidthentries are typed as{ [x: string]: string | number; }, and reading onlyObject.keys(entry)[0]silently drops any additional accessor keys. UseObject.entries(entry)so every valid accessor preserves its width.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/widgets/src/common/use-persisted-table-layout.ts` around lines 136 - 146, Update the columnsWidth processing loop to iterate over every accessor-width pair with Object.entries(entry), rather than selecting only Object.keys(entry)[0]. Preserve the existing visible-accessor filtering and numeric/px width validation while assigning each valid width to widthMap.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/widgets/src/common/use-persisted-table-layout.ts`:
- Around line 87-104: Update the hydration logic in the useEffect to determine
completion by value rather than relying on the requestAnimationFrame callback.
Compare the current table column order and visible widths with restoredOrder and
savedWidths, using value-based comparisons for both order and widths, and set
hydrated.current accordingly while preserving storeKey reset behavior.
---
Nitpick comments:
In `@packages/widgets/src/beszel-system-table/component.tsx`:
- Line 341: Update the columns useMemo dependency list to remove the whole
options object and include only the individual option flags read by the columns
computation. Keep t and size, and ensure every referenced options flag remains
represented so derived layout values retain stable identity when unrelated
options change.
In `@packages/widgets/src/common/use-persisted-table-layout.ts`:
- Around line 136-146: Update the columnsWidth processing loop to iterate over
every accessor-width pair with Object.entries(entry), rather than selecting only
Object.keys(entry)[0]. Preserve the existing visible-accessor filtering and
numeric/px width validation while assigning each valid width to widthMap.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 687ac5ed-1d9a-4dd9-b53a-0adbbbbfb5f0
📒 Files selected for processing (5)
packages/widgets/src/beszel-system-table/component.spec.tspackages/widgets/src/beszel-system-table/component.tsxpackages/widgets/src/common/use-persisted-table-layout.tspackages/widgets/src/docker/component.spec.tspackages/widgets/src/docker/component.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/widgets/src/docker/component.spec.ts
- packages/widgets/src/beszel-system-table/component.spec.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/widgets/src/common/use-persisted-table-layout.ts (1)
148-186: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winFlush the pending layout on unmount instead of discarding it.
The unmount cleanup at lines 150-155 only clears
saveTimeout.current. If the component unmounts (e.g., the widget is removed, the board is navigated away from) within the 350 ms debounce window after a column reorder or resize,pendingLayout.currentis never sent toonLayoutChangeand the user's last layout edit is silently lost.Call
onLayoutChange(pendingLayout.current)in the cleanup when there is a pending layout, before clearing the timeout.🛠️ Proposed fix to flush pending layout on unmount
useEffect( - () => () => { - window.clearTimeout(saveTimeout.current); - }, + () => () => { + window.clearTimeout(saveTimeout.current); + if (Object.keys(pendingLayout.current).length > 0) { + onLayoutChange(pendingLayout.current); + pendingLayout.current = {}; + } + }, [], );Note: if
onLayoutChangeis not stable across renders, add it to this effect's dependency array or capture it in a ref to avoid a stale closure.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/widgets/src/common/use-persisted-table-layout.ts` around lines 148 - 186, Update the unmount cleanup associated with pendingLayout and saveTimeout to call onLayoutChange(pendingLayout.current) when a pending layout exists, then clear the timeout and reset the pending layout. Ensure the cleanup uses the current onLayoutChange callback by including it in the effect dependencies or capturing it in a ref.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/widgets/src/common/use-persisted-table-layout.ts`:
- Around line 148-186: Update the unmount cleanup associated with pendingLayout
and saveTimeout to call onLayoutChange(pendingLayout.current) when a pending
layout exists, then clear the timeout and reset the pending layout. Ensure the
cleanup uses the current onLayoutChange callback by including it in the effect
dependencies or capturing it in a ref.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e41d3187-94a6-4edc-badc-eb26dd296aa3
📒 Files selected for processing (2)
packages/widgets/src/beszel-system-table/component.tsxpackages/widgets/src/common/use-persisted-table-layout.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/widgets/src/beszel-system-table/component.tsx
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/widgets/src/docker/component.tsx (1)
238-251: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the permission-gated layout-persist callback into a shared helper. Both widgets duplicate the identical
setOptions+ permission-gatedsaveItemOptionsglue that feedsusePersistedTableLayout'sonLayoutChange. The shared hook already consolidated the parsing/hydration/debounce logic; this remaining glue should move alongside it (e.g., acceptsetOptions,saveItemOptions,hasChangeAccess,boardId,itemIdas hook inputs, or expose a smallusePersistLayoutOptionshelper) so a future fix to the gating logic lands once instead of per widget.
packages/widgets/src/docker/component.tsx#L238-L251: replace the localpersistLayoutuseCallbackwith the shared helper.packages/widgets/src/downloads/component.tsx#L341-L347: replace the localpersistOptionuseCallbackwith the same shared helper.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/widgets/src/docker/component.tsx` around lines 238 - 251, Extract the duplicated permission-gated layout persistence logic into a shared helper alongside usePersistedTableLayout, accepting the existing setOptions, saveItemOptions, hasChangeAccess, boardId, and itemId inputs as needed. Replace the local persistLayout callback in packages/widgets/src/docker/component.tsx lines 238-251 and the local persistOption callback in packages/widgets/src/downloads/component.tsx lines 341-347 with this helper, preserving both widgets’ current option updates and gated saves.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/widgets/src/docker/component.tsx`:
- Around line 238-251: Extract the duplicated permission-gated layout
persistence logic into a shared helper alongside usePersistedTableLayout,
accepting the existing setOptions, saveItemOptions, hasChangeAccess, boardId,
and itemId inputs as needed. Replace the local persistLayout callback in
packages/widgets/src/docker/component.tsx lines 238-251 and the local
persistOption callback in packages/widgets/src/downloads/component.tsx lines
341-347 with this helper, preserving both widgets’ current option updates and
gated saves.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d2927172-a74c-4547-af89-c19410be24b3
📒 Files selected for processing (10)
apps/docs/docs/widgets/beszel-system-table/index.mdxapps/docs/docs/widgets/docker-containers/index.mdxapps/docs/docs/widgets/downloads/index.mdxpackages/widgets/src/beszel-system-table/component.tsxpackages/widgets/src/beszel-system-table/styles.csspackages/widgets/src/common/homarr-data-table.csspackages/widgets/src/common/homarr-data-table.tsxpackages/widgets/src/docker/component.tsxpackages/widgets/src/downloads/component.tsxpackages/widgets/src/downloads/styles.css
💤 Files with no reviewable changes (2)
- packages/widgets/src/downloads/styles.css
- packages/widgets/src/beszel-system-table/styles.css
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/docs/docs/widgets/docker-containers/index.mdx
- apps/docs/docs/widgets/beszel-system-table/index.mdx
- packages/widgets/src/beszel-system-table/component.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/widgets/src/common/use-persisted-table-layout.ts`:
- Around line 173-181: Update the pending-layout state and flush logic around
flushPendingLayout so each debounced batch retains the originating
onLayoutChange callback and storeKey. When itemId/storeKey changes, flush the
existing batch through its stored callback before replacing it, and ensure the
timeout callback uses the batch’s stored callback rather than the latest
onLayoutChangeRef.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2e2057c4-ead9-4f2c-91dd-93d32091f726
📒 Files selected for processing (5)
apps/docs/docs/widgets/docker-containers/index.mdxpackages/widgets/src/beszel-system-table/component.tsxpackages/widgets/src/common/use-persisted-table-layout.tspackages/widgets/src/docker/component.tsxpackages/widgets/src/downloads/component.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/docs/docs/widgets/docker-containers/index.mdx
- packages/widgets/src/docker/component.tsx
- packages/widgets/src/beszel-system-table/component.tsx
- packages/widgets/src/downloads/component.tsx
0242496 to
d4c66ce
Compare
# Conflicts: # packages/integrations/src/navidrome/test/navidrome-integration.spec.ts
1 participant
Summary
Validation
pnpm exec vitest run packages/widgets/src/beszel-system-table/component.spec.ts packages/widgets/src/docker/component.spec.ts packages/widgets/src/test/translation.spec.ts --coverage.enabled=false(558 tests passed)pnpm turbo typecheck --filter=@homarr/widgets --filter=@homarr/docspnpm turbo build --filter=@homarr/docspnpm --filter @homarr/widgets lintpnpm --filter @homarr/docs lintSummary by CodeRabbit
New Features
Improvements