feat: add configurable automatic mobile board layout - #6450
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:
📝 WalkthroughWalkthroughThis PR adds an automatic two-column mobile board, separates desktop and responsive layout editing, makes layout mutations layout-aware, adds mobile-layout settings and migration behavior, and updates related tests, translations, documentation, and CI configuration. ChangesAutomatic mobile board
Sequence Diagram(s)sequenceDiagram
participant Request as Request headers
participant Viewport as MobileBoardViewportProvider
participant Client as ClientBoard
participant Mobile as MobileBoard
participant Items as BoardItemContent
Request->>Viewport: initialize mobile state
Viewport->>Client: provide viewport and settings state
Client->>Mobile: render mobile board
Mobile->>Items: render flattened grid items
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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
packages/api/src/router/board.ts (1)
685-685: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExpose
saveLayoutas an MCP tool. This router file requires new useful procedures to be published as MCP tools, andsaveLayoutis a user-facing mutation. Add a.meta({ mcp: { enabled: true, description: "..." } })before.input(...).Suggested change
- saveLayout: protectedProcedure.input(boardSaveLayoutSchema).mutation(async ({ ctx, input }) => { + saveLayout: protectedProcedure + .meta({ + mcp: { + enabled: true, + description: + "Set the desktop column count for a board's layout (recomputes item/section positions). Requires modify permission. REQUIRED: id (board ID from board_getAllBoards), columnCount (1-24)", + }, + }) + .input(boardSaveLayoutSchema) + .mutation(async ({ ctx, input }) => {As per coding guidelines: "Expose every new useful tRPC procedure as an MCP tool by adding
.meta(...)" and "Place the.meta()call before.input()".🤖 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/api/src/router/board.ts` at line 685, Expose the saveLayout procedure as an MCP tool by adding a .meta configuration with mcp.enabled set to true and a clear description before .input(boardSaveLayoutSchema). Keep the existing protectedProcedure mutation behavior unchanged.Source: Coding guidelines
apps/nextjs/src/components/board/mobile/mobile-layout.ts (1)
1-106: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMobile column count ("2") is duplicated between the layout generator and the CSS module. Both files encode the same 2-column constraint independently; changing one without the other would silently mis-size mobile items relative to the actual grid.
apps/nextjs/src/components/board/mobile/mobile-layout.ts#L1-L106:mobileColumnCount = 2clamps item widths for the mobile grid.apps/nextjs/src/components/board/mobile/mobile-board.module.css#L1-L20:grid-template-columns: repeat(2, minmax(0, 1fr));hardcodes the same column count independently.Consider exporting
mobileColumnCountand consuming it in the CSS via a CSS custom property (e.g., setstyle={{ "--mobile-column-count": mobileColumnCount }}on the grid wrapper and referencerepeat(var(--mobile-column-count), minmax(0, 1fr))), or at minimum add a comment in each file cross-referencing the other so future changes stay in sync.🤖 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 `@apps/nextjs/src/components/board/mobile/mobile-layout.ts` around lines 1 - 106, The mobile column count is duplicated between the layout generator and CSS grid. Update apps/nextjs/src/components/board/mobile/mobile-layout.ts:1-106 to expose and reuse mobileColumnCount as the single source of truth, update the grid wrapper and apps/nextjs/src/components/board/mobile/mobile-board.module.css:1-20 to consume that value through a CSS custom property, and replace the hardcoded repeat(2, ...) count; keep item width clamping aligned with the grid.
🤖 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 `@apps/nextjs/src/components/board/mobile/mobile-board.module.css`:
- Line 16: Update the Stylelint configuration’s selector-pseudo-class-no-unknown
rule to ignore the CSS Modules :global pseudo-class, preserving existing
validation for other unknown pseudo-classes. Check the existing configuration
and reuse its current rule structure rather than adding a separate override.
In `@apps/nextjs/src/components/board/mobile/mobile-board.tsx`:
- Around line 16-25: Update MobileBoard so it does not return null when
rootSection is absent; allow the existing createMobileBoardItems result to
render for boards containing only dynamic sections, while preserving any
rootSection-dependent behavior through an optional or conditional path.
In `@apps/nextjs/src/components/board/use-mobile-board.ts`:
- Around line 5-7: Update useIsMobileBoard to accept and forward a server-known
device or layout hint as Mantine’s initialValue, and configure
getInitialValueInEffect appropriately so mobile clients do not first render the
desktop board. Preserve the existing mobileBoardMediaQuery and allow the current
behavior when no hint is available.
In `@packages/api/src/router/board.ts`:
- Around line 711-750: Update saveLayout to wrap the item-layout updates,
section-layout updates, base-layout update, and legacy-layout deletion in a
single handleTransactionsAsync transaction, reusing the transaction callback
pattern from saveBoard. Ensure every database operation in this recompute flow
uses the transaction context so failures roll back the entire layout change
atomically.
---
Nitpick comments:
In `@apps/nextjs/src/components/board/mobile/mobile-layout.ts`:
- Around line 1-106: The mobile column count is duplicated between the layout
generator and CSS grid. Update
apps/nextjs/src/components/board/mobile/mobile-layout.ts:1-106 to expose and
reuse mobileColumnCount as the single source of truth, update the grid wrapper
and apps/nextjs/src/components/board/mobile/mobile-board.module.css:1-20 to
consume that value through a CSS custom property, and replace the hardcoded
repeat(2, ...) count; keep item width clamping aligned with the grid.
In `@packages/api/src/router/board.ts`:
- Line 685: Expose the saveLayout procedure as an MCP tool by adding a .meta
configuration with mcp.enabled set to true and a clear description before
.input(boardSaveLayoutSchema). Keep the existing protectedProcedure mutation
behavior unchanged.
🪄 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: 85878807-d247-4d89-aa4d-5b9049e55931
📒 Files selected for processing (15)
apps/docs/docs/management/boards/index.mdxapps/nextjs/src/app/[locale]/boards/(content)/_client.tsxapps/nextjs/src/app/[locale]/boards/(content)/_header-actions.tsxapps/nextjs/src/app/[locale]/boards/[name]/settings/_layout.tsxapps/nextjs/src/components/board/mobile/mobile-board.module.cssapps/nextjs/src/components/board/mobile/mobile-board.tsxapps/nextjs/src/components/board/mobile/mobile-layout.spec.tsapps/nextjs/src/components/board/mobile/mobile-layout.tsapps/nextjs/src/components/board/use-mobile-board.tspackages/api/src/router/board.tspackages/api/src/router/test/board.spec.tspackages/boards/src/context.tsxpackages/boards/src/edit-mode.tsxpackages/translation/src/lang/en.jsonpackages/validation/src/board.ts
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 `@apps/nextjs/src/components/board/mobile/mobile-board.tsx`:
- Around line 20-25: Update the rootSection selection to use the same ordering
as createMobileBoardItems: filter out dynamic sections, sort the remaining
sections by yOffset and then xOffset, and select the first one. Preserve the
existing mobile fallback object when no non-dynamic sections exist, and pass
this consistently to SectionProvider.
🪄 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: ad79b19f-1879-4448-9b33-c5b191beecd3
📒 Files selected for processing (6)
apps/nextjs/src/app/[locale]/boards/_layout-creator.tsxapps/nextjs/src/components/board/mobile/mobile-board.module.cssapps/nextjs/src/components/board/mobile/mobile-board.tsxapps/nextjs/src/components/board/mobile/mobile-layout.tsapps/nextjs/src/components/board/use-mobile-board.tsxpackages/api/src/router/board.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/nextjs/src/components/board/mobile/mobile-board.module.css
- apps/nextjs/src/components/board/mobile/mobile-layout.ts
- packages/api/src/router/board.ts
Dokploy Preview Deployment
|
3f3ba61 to
7bcfdd6
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/nextjs/src/app/[locale]/boards/(content)/_header-actions.tsx (1)
56-75: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
!isMobile/hidden={isMobile}are dead code after the early mobile return.Since
BoardContentHeaderActionsalready returns<MobileMoreMenu/>whenisMobileis true (L63-65),isMobileis guaranteedfalsefor the rest of the render. That makesisEditMode && !isMobile && <AddMenu/>(L73) equivalent to justisEditMode, andhidden={isMobile}on<EditModeMenu/>(L75) always resolves tohidden={false}— thehiddenprop's actual guard logic inEditModeMenu(L207-219) is unreachable from this call site. This is confusing for anyone reading the code without tracing the control flow.♻️ Suggested simplification (if `EditModeMenu`'s `hidden` isn't needed elsewhere)
- {isEditMode && !isMobile && <AddMenu />} + {isEditMode && <AddMenu />} - <EditModeMenu demoReadOnly={demoReadOnly} hidden={isMobile} /> + <EditModeMenu demoReadOnly={demoReadOnly} />🤖 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 `@apps/nextjs/src/app/`[locale]/boards/(content)/_header-actions.tsx around lines 56 - 75, Remove the redundant !isMobile condition from the AddMenu render and remove the always-false hidden={isMobile} prop from EditModeMenu in BoardContentHeaderActions, since the earlier mobile return guarantees the remaining branch is non-mobile.
🤖 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 `@apps/nextjs/src/app/`[locale]/boards/(content)/_header-actions.tsx:
- Around line 341-345: Update the sign-out handler around signOut in the onClick
callback to handle rejected promises with a catch path. Ensure failures do not
produce an unhandled rejection and still perform the logoutUrl-or-/auth/login
redirect, while preserving the existing redirect behavior on successful
sign-out.
---
Nitpick comments:
In `@apps/nextjs/src/app/`[locale]/boards/(content)/_header-actions.tsx:
- Around line 56-75: Remove the redundant !isMobile condition from the AddMenu
render and remove the always-false hidden={isMobile} prop from EditModeMenu in
BoardContentHeaderActions, since the earlier mobile return guarantees the
remaining branch is non-mobile.
🪄 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: 14180e4c-2e4a-42ad-b69d-89ded02659de
📒 Files selected for processing (46)
apps/docs/docs/management/boards/index.mdxapps/nextjs/src/app/[locale]/boards/(content)/_creator.tsxapps/nextjs/src/app/[locale]/boards/(content)/_header-actions.tsxapps/nextjs/src/app/[locale]/boards/_board-switcher-logo.tsxapps/nextjs/src/app/[locale]/boards/_header-actions.tsxapps/nextjs/src/app/[locale]/boards/_layout-creator.tsxapps/nextjs/src/app/[locale]/boards/layout.tsxapps/nextjs/src/components/board/items/item-content.tsxapps/nextjs/src/components/board/mobile/deferred-mobile-item.tsxapps/nextjs/src/components/board/mobile/mobile-board.module.cssapps/nextjs/src/components/board/mobile/mobile-board.tsxapps/nextjs/src/components/board/mobile/mobile-layout.spec.tsapps/nextjs/src/components/board/mobile/mobile-layout.tsapps/nextjs/src/components/board/mobile/mobile-presentation.spec.tsapps/nextjs/src/components/board/mobile/mobile-presentation.tsapps/nextjs/src/components/board/mobile/mobile-widget-actions.tsxapps/nextjs/src/components/board/use-mobile-board.spec.tsapps/nextjs/src/components/board/use-mobile-board.tsxapps/nextjs/src/components/layout/header.tsxapps/nextjs/src/components/layout/header/button.tsxapps/nextjs/src/components/layout/header/responsive-user.tsxapps/nextjs/src/components/layout/header/search.tsxapps/nextjs/src/components/layout/header/user.tsxapps/nextjs/src/components/layout/shell.tsxpackages/translation/src/lang/en.jsonpackages/widgets/src/app/index.tspackages/widgets/src/calendar/component.tsxpackages/widgets/src/calendar/index.tspackages/widgets/src/clock/index.tspackages/widgets/src/common/mobile-summary.tsxpackages/widgets/src/definition.tspackages/widgets/src/downloads/component.tsxpackages/widgets/src/downloads/index.tspackages/widgets/src/index.tsxpackages/widgets/src/media-missing/component.tsxpackages/widgets/src/media-missing/index.tspackages/widgets/src/media-releases/component.tsxpackages/widgets/src/media-releases/index.tspackages/widgets/src/media-requests/list/component.tsxpackages/widgets/src/media-requests/list/index.tspackages/widgets/src/media-transcoding/component.tsxpackages/widgets/src/media-transcoding/index.tspackages/widgets/src/notifications/component.tsxpackages/widgets/src/notifications/index.tspackages/widgets/src/smart-home/entity-state/index.tspackages/widgets/src/smart-home/execute-automation/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/nextjs/src/app/[locale]/boards/_layout-creator.tsx
- apps/docs/docs/management/boards/index.mdx
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Let me know when U want me to test 🙂 |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (4)
apps/nextjs/src/components/board/mobile/mobile-dialog-stack.spec.ts (1)
29-34: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThis test passes vacuously.
root.querySelectorAll(...)never returnsrootitself, so the assertion holds even if thedialog !== currentDialogexclusion were deleted. Nest the details dialog inside a shared root (with a sibling non-dialog) so the exclusion logic is actually exercised.🤖 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 `@apps/nextjs/src/components/board/mobile/mobile-dialog-stack.spec.ts` around lines 29 - 34, Update the test “does not treat the current details dialog as nested” to render the details dialog inside a shared root alongside a non-dialog sibling, then call hasOpenDialogOutside with that root and details dialog. Keep the assertion false so the test specifically exercises exclusion of the current dialog rather than relying on querySelectorAll omitting the root.packages/widgets/src/system-resources/component.tsx (1)
88-92: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider labelling the direction arrows for assistive tech.
↓/↑carry the only download/upload semantics here and screen readers announce them inconsistently (or as "down arrow"). A localized prefix (e.g.t("card.network.down")) or a visually-hidden label would make the mobile summary self-describing.🤖 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/system-resources/component.tsx` around lines 88 - 92, Update the network value and description in the component’s system-resources card to provide localized download/upload text for assistive technologies instead of relying solely on the ↓ and ↑ symbols. Reuse the existing translation function t with appropriate network direction keys, while preserving the current formatted rates and fallback behavior.apps/nextjs/src/components/board/mobile/mobile-dialog-stack.ts (1)
35-43: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueObserver fires a document-wide query on every subtree mutation.
subtree: true+childList: trueondocument.bodymeans any DOM churn inside the open details widget (charts, lists, live data) triggers a fulldocument.querySelectorAll('[role="dialog"]'). ScopingchildListobservation or batchingupdatein a microtask/rAF would keep this off the mutation hot path.🤖 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 `@apps/nextjs/src/components/board/mobile/mobile-dialog-stack.ts` around lines 35 - 43, Reduce the mutation overhead in the observer setup around the mobile dialog stack’s `MutationObserver` by batching `update` calls with a microtask or animation frame, or by narrowing child-list observation to the relevant dialog container. Preserve updates for dialog additions, removals, and relevant attribute changes while preventing every document-wide subtree mutation from triggering an immediate `document.querySelectorAll('[role="dialog"]')`.apps/nextjs/src/components/board/mobile/deferred-mobile-item.tsx (1)
24-67: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winSolid deferred-mount/unmount logic; consider debouncing the per-item resize listener.
The eager / mount-once / continuous-unmount branches are all correctly guarded. One nit: every mounted
DeferredMobileItemregisters its ownresizelistener that re-runsobserve()(potentially recreating theIntersectionObserver) on every resize tick. On a board with many widgets, rapid resize events (e.g. orientation change) could trigger many synchronous re-observations at once. Debouncingobserve(or coordinating recalculation from a single parent listener) would reduce this cost.🤖 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 `@apps/nextjs/src/components/board/mobile/deferred-mobile-item.tsx` around lines 24 - 67, Debounce the per-item resize handling in the useEffect hook so rapid resize events do not synchronously recreate the IntersectionObserver through observe. Preserve the existing eager, mount-once, offscreen-unmount, cleanup, and root-margin recalculation behavior, including cancelling any pending debounce during effect cleanup.
🤖 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 `@apps/docs/docs/management/boards/index.mdx`:
- Line 43: In the public-board description, update the misspelled word “fiends”
to “friends” while preserving the rest of the sentence.
In `@apps/nextjs/src/components/board/mobile/mobile-board.module.css`:
- Around line 9-19: Replace the deprecated clip declaration in .boardTitle with
the equivalent clip-path declaration, preserving the existing visually-hidden
behavior and all other accessibility styles unchanged.
In `@packages/widgets/src/beszel/_shared/mobile-summary.tsx`:
- Around line 21-29: In packages/widgets/src/beszel/_shared/mobile-summary.tsx
lines 21-29, update the error throw in the mobile summary query flow to occur
only when results.length === 0; retain failed-result handling and render cached
results with isStale when usable results exist. In
packages/widgets/src/beszel-alerts/component.tsx lines 73-78, likewise gate the
alertsError throw on alerts results being empty so cached alerts continue
rendering during failed background refetches.
In `@packages/widgets/src/notebook/component.tsx`:
- Around line 19-20: Update the HTML-stripping replacements in the notebook
component to allow optional whitespace between the closing script/style tag
names and the closing angle bracket, while preserving case-insensitive matching
and removal of the entire element contents.
In `@packages/widgets/src/system-disks/component.tsx`:
- Around line 160-161: Update the temperature computation in the system disk
component to use a nullish availability check for smartItem?.temperature, so a
valid 0°C value is formatted and displayed while null or undefined remains
omitted.
In `@packages/widgets/src/weather/component.tsx`:
- Around line 59-63: Update getPreferredUnit usage in the current-weather,
weekly current-weather, and forecast-day accessible labels so a temperature
value of 0 is passed through rather than treated as missing; apply the same
zero-preserving change at all three listed sites in
packages/widgets/src/weather/component.tsx.
---
Nitpick comments:
In `@apps/nextjs/src/components/board/mobile/deferred-mobile-item.tsx`:
- Around line 24-67: Debounce the per-item resize handling in the useEffect hook
so rapid resize events do not synchronously recreate the IntersectionObserver
through observe. Preserve the existing eager, mount-once, offscreen-unmount,
cleanup, and root-margin recalculation behavior, including cancelling any
pending debounce during effect cleanup.
In `@apps/nextjs/src/components/board/mobile/mobile-dialog-stack.spec.ts`:
- Around line 29-34: Update the test “does not treat the current details dialog
as nested” to render the details dialog inside a shared root alongside a
non-dialog sibling, then call hasOpenDialogOutside with that root and details
dialog. Keep the assertion false so the test specifically exercises exclusion of
the current dialog rather than relying on querySelectorAll omitting the root.
In `@apps/nextjs/src/components/board/mobile/mobile-dialog-stack.ts`:
- Around line 35-43: Reduce the mutation overhead in the observer setup around
the mobile dialog stack’s `MutationObserver` by batching `update` calls with a
microtask or animation frame, or by narrowing child-list observation to the
relevant dialog container. Preserve updates for dialog additions, removals, and
relevant attribute changes while preventing every document-wide subtree mutation
from triggering an immediate `document.querySelectorAll('[role="dialog"]')`.
In `@packages/widgets/src/system-resources/component.tsx`:
- Around line 88-92: Update the network value and description in the component’s
system-resources card to provide localized download/upload text for assistive
technologies instead of relying solely on the ↓ and ↑ symbols. Reuse the
existing translation function t with appropriate network direction keys, while
preserving the current formatted rates and fallback behavior.
🪄 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: 23e23d9d-aa82-4a8a-8d99-1cf06fde046c
📒 Files selected for processing (116)
apps/docs/docs/management/boards/index.mdxapps/nextjs/src/app/[locale]/boards/(content)/_creator.tsxapps/nextjs/src/app/[locale]/boards/(content)/_header-actions.tsxapps/nextjs/src/app/[locale]/boards/(content)/_prefetch-policy.spec.tsapps/nextjs/src/app/[locale]/boards/(content)/_prefetch-policy.tsapps/nextjs/src/app/[locale]/boards/_board-switcher-logo.tsxapps/nextjs/src/components/board/items/item-content.tsxapps/nextjs/src/components/board/mobile/deferred-mobile-item.spec.tsapps/nextjs/src/components/board/mobile/deferred-mobile-item.tsxapps/nextjs/src/components/board/mobile/mobile-board.module.cssapps/nextjs/src/components/board/mobile/mobile-board.tsxapps/nextjs/src/components/board/mobile/mobile-dialog-stack.spec.tsapps/nextjs/src/components/board/mobile/mobile-dialog-stack.tsapps/nextjs/src/components/board/mobile/mobile-layout.spec.tsapps/nextjs/src/components/board/mobile/mobile-layout.tsapps/nextjs/src/components/board/mobile/mobile-presentation.spec.tsapps/nextjs/src/components/board/mobile/mobile-presentation.tsapps/nextjs/src/components/board/mobile/mobile-section-navigation.spec.tsapps/nextjs/src/components/board/mobile/mobile-section-navigation.tsapps/nextjs/src/components/board/mobile/mobile-widget-actions.tsxapps/nextjs/src/components/layout/header/search.tsxe2e/mobile-board.spec.tspackages/api/src/router/board.tspackages/api/src/router/test/board.spec.tspackages/api/src/router/widgets/calendar.tspackages/api/src/router/widgets/downloads.tspackages/api/src/router/widgets/health-monitoring.tspackages/api/src/router/widgets/media-release.tspackages/api/src/router/widgets/media-requests.tspackages/api/src/router/widgets/media-server.tspackages/api/src/router/widgets/notifications.tspackages/api/src/router/widgets/uptime-kuma.tspackages/api/src/settle-integrations.spec.tspackages/api/src/settle-integrations.tspackages/translation/src/lang/en.jsonpackages/widgets/src/anchor-note/index.tspackages/widgets/src/app/component.tsxpackages/widgets/src/archive-team-warrior/index.tspackages/widgets/src/audio-stats/index.tspackages/widgets/src/bazarr/index.tspackages/widgets/src/beszel-alerts/component.tsxpackages/widgets/src/beszel-alerts/index.tspackages/widgets/src/beszel-system-grid/component.tsxpackages/widgets/src/beszel-system-grid/index.tspackages/widgets/src/beszel-system-stats/component.tsxpackages/widgets/src/beszel-system-stats/index.tspackages/widgets/src/beszel-system-table/component.tsxpackages/widgets/src/beszel-system-table/index.tspackages/widgets/src/beszel/_shared/mobile-summary.tsxpackages/widgets/src/bookmarks/index.tsxpackages/widgets/src/calendar/component.tsxpackages/widgets/src/calendar/index.tspackages/widgets/src/common/mobile-summary.tsxpackages/widgets/src/coolify/applications-section.tsxpackages/widgets/src/coolify/component.tsxpackages/widgets/src/coolify/index.tspackages/widgets/src/coolify/instance-card.tsxpackages/widgets/src/coolify/resource-row.tsxpackages/widgets/src/coolify/servers-section.tsxpackages/widgets/src/coolify/services-section.tsxpackages/widgets/src/coolify/single-instance-layout.tsxpackages/widgets/src/custom-api/component.tsxpackages/widgets/src/custom-api/index.tspackages/widgets/src/custom-api/jsx-interactive-components.tsxpackages/widgets/src/definition.tspackages/widgets/src/dns-hole/controls/TimerModal.tsxpackages/widgets/src/dns-hole/controls/component.tsxpackages/widgets/src/dns-hole/controls/index.tspackages/widgets/src/docker/component.tsxpackages/widgets/src/docker/index.tspackages/widgets/src/downloads/component.tsxpackages/widgets/src/downloads/index.tspackages/widgets/src/firewall/index.tspackages/widgets/src/health-monitoring/component.tsxpackages/widgets/src/health-monitoring/index.tspackages/widgets/src/health-monitoring/system-health.tsxpackages/widgets/src/iframe/component.tsxpackages/widgets/src/iframe/index.tspackages/widgets/src/indexer-manager/index.tspackages/widgets/src/media-missing/component.tsxpackages/widgets/src/media-missing/index.tspackages/widgets/src/media-releases/component.tsxpackages/widgets/src/media-requests/list/component.tsxpackages/widgets/src/media-requests/list/index.tspackages/widgets/src/media-requests/stats/index.tspackages/widgets/src/media-server/component.tsxpackages/widgets/src/media-server/index.tspackages/widgets/src/media-transcoding/component.tsxpackages/widgets/src/media-transcoding/index.tspackages/widgets/src/notebook/component.tsxpackages/widgets/src/notebook/index.tspackages/widgets/src/notebook/notebook.tsxpackages/widgets/src/notifications/component.tsxpackages/widgets/src/notifications/index.tspackages/widgets/src/paperless-ngx/index.tspackages/widgets/src/patchmon/index.tspackages/widgets/src/releases/index.tspackages/widgets/src/rssFeed/component.tsxpackages/widgets/src/rssFeed/index.tspackages/widgets/src/smart-home/entity-state/index.tspackages/widgets/src/speedtest-tracker/index.tspackages/widgets/src/stocks/index.tspackages/widgets/src/system-disks/component.tsxpackages/widgets/src/system-disks/index.tspackages/widgets/src/system-resources/component.tsxpackages/widgets/src/system-resources/index.tspackages/widgets/src/timetable/index.tspackages/widgets/src/tracearr/index.tspackages/widgets/src/traefik/index.tspackages/widgets/src/umami/index.tspackages/widgets/src/ups/index.tspackages/widgets/src/uptime-kuma/component.tsxpackages/widgets/src/uptime-kuma/index.tspackages/widgets/src/vpn/index.tspackages/widgets/src/weather/component.tsxpackages/widgets/src/weather/index.ts
🚧 Files skipped from review as they are similar to previous changes (19)
- apps/nextjs/src/app/[locale]/boards/_board-switcher-logo.tsx
- packages/widgets/src/media-transcoding/index.ts
- packages/widgets/src/calendar/index.ts
- apps/nextjs/src/components/board/mobile/mobile-layout.spec.ts
- packages/widgets/src/media-requests/list/index.ts
- apps/nextjs/src/components/layout/header/search.tsx
- packages/widgets/src/downloads/index.ts
- packages/widgets/src/notifications/index.ts
- packages/widgets/src/media-releases/component.tsx
- packages/widgets/src/smart-home/entity-state/index.ts
- packages/widgets/src/media-requests/list/component.tsx
- packages/widgets/src/definition.ts
- packages/widgets/src/media-missing/index.ts
- packages/widgets/src/media-missing/component.tsx
- apps/nextjs/src/components/board/mobile/mobile-board.tsx
- packages/widgets/src/calendar/component.tsx
- packages/widgets/src/notifications/component.tsx
- apps/nextjs/src/components/board/mobile/mobile-layout.ts
- apps/nextjs/src/app/[locale]/boards/(content)/_header-actions.tsx
|
@coderabbitai review |
✅ Action performedReview finished.
|
890a9c8 to
7959c20
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (13)
apps/nextjs/src/components/board/use-mobile-board.spec.ts (1)
77-85: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMove the skeleton test into its own
describeand assert the positive branch. It currently sits underresolveIsAutomaticMobileBoard, and only thefalsecase is covered — the automatic + unresolved case that actually gates the skeleton render is untested.♻️ Proposed refactor
- test("does not show the tablet resolution skeleton in legacy mode", () => { - expect( - shouldShowMobileBoardViewportSkeleton({ - enableAutomaticMobileLayout: false, - isResolved: false, - }), - ).toBe(false); - }); -}); +}); + +describe("shouldShowMobileBoardViewportSkeleton", () => { + test("does not show the tablet resolution skeleton in legacy mode", () => { + expect(shouldShowMobileBoardViewportSkeleton({ enableAutomaticMobileLayout: false, isResolved: false })).toBe( + false, + ); + }); + + test("shows the skeleton while an automatic mobile viewport is unresolved", () => { + expect(shouldShowMobileBoardViewportSkeleton({ enableAutomaticMobileLayout: true, isResolved: false })).toBe(true); + }); +});🤖 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 `@apps/nextjs/src/components/board/use-mobile-board.spec.ts` around lines 77 - 85, Move the shouldShowMobileBoardViewportSkeleton test out of the resolveIsAutomaticMobileBoard describe block into a dedicated describe. Add coverage for enableAutomaticMobileLayout true with isResolved false and assert that the skeleton is shown, while preserving the existing legacy-mode assertion.apps/nextjs/src/components/board/mobile/mobile-presentation.ts (1)
18-19: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueGate
unmountWhenOffscreenon the resolvedeagervalue, not just the explicit flag. Wheneagercomes from theapp/bookmarksdefault, an explicitunmountWhenOffscreen: truestill wins, producing an eagerly mounted item that unmounts on scroll.♻️ Proposed refactor
- return { + const eager = mobile?.eager ?? (item.kind === "app" || item.kind === "bookmarks"); + + return { width: mobile?.width ?? (item.kind === "app" ? 1 : 2), height: mobile?.height ?? (item.kind === "app" || usesGenericSummary ? 1 : Math.max(1, Math.min(item.height, 3))), displayMode: supportsCompactSummary || usesGenericSummary ? ("mobileSummary" as const) : ("default" as const), supportsDetails, usesGenericSummary, - eager: mobile?.eager ?? (item.kind === "app" || item.kind === "bookmarks"), - unmountWhenOffscreen: mobile?.eager === true ? false : (mobile?.unmountWhenOffscreen ?? false), + eager, + unmountWhenOffscreen: eager ? false : (mobile?.unmountWhenOffscreen ?? false), };🤖 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 `@apps/nextjs/src/components/board/mobile/mobile-presentation.ts` around lines 18 - 19, Update the mobile presentation options so unmountWhenOffscreen is disabled whenever the resolved eager value is true, including when eager uses the app/bookmarks default. Base the condition on the resolved eager expression or value rather than only mobile?.eager === true, while preserving the existing fallback behavior for non-eager items.apps/nextjs/src/components/board/mobile/mobile-viewport-observer.spec.ts (1)
39-47: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider resetting the observer module between tests.
mobile-viewport-observerkeepsobserver/subscriptions/resize-listener state at module scope, so these tests only pass because each one happens to fully unsubscribe. Addingvi.resetModules()with a dynamic import (or an exported reset for tests) would make the suite robust to future tests that leave a subscription open.🤖 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 `@apps/nextjs/src/components/board/mobile/mobile-viewport-observer.spec.ts` around lines 39 - 47, Reset the module-scoped observer state between tests in the mobile viewport observer spec by using vi.resetModules() and dynamically importing mobile-viewport-observer for each test, or by invoking an exported test reset hook. Ensure observer, subscriptions, and resize-listener state is freshly initialized before every test while preserving the existing IntersectionObserverMock setup.packages/boards/src/context.spec.ts (1)
16-36: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider covering the undefined-viewport and no-layout paths.
getCurrentLayout(board, false, undefined)(SSR path, falls back to desktop) and theBoard must have a layoutthrow are both untested branches of the new logic.🤖 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/boards/src/context.spec.ts` around lines 16 - 36, Extend the “board layout selection” tests to cover getCurrentLayout with legacy mode and an undefined viewport, asserting it falls back to the desktop “large” layout. Also add a test for getCurrentLayout when the board has no layouts, asserting it throws the “Board must have a layout” error.apps/nextjs/src/components/board/use-mobile-board.tsx (1)
76-89: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
hasResolvedClientWidthis really a "has mounted" flag.It is set unconditionally on mount and never reflects width resolution; renaming it (e.g.
hasMounted) would make the tablet gate inresolveMobileBoardViewporteasier to reason about.🤖 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 `@apps/nextjs/src/components/board/use-mobile-board.tsx` around lines 76 - 89, Rename hasResolvedClientWidth and its setter to hasMounted and setHasMounted throughout the component, including the useEffect and resolveMobileBoardViewport call. Preserve the existing mount-only initialization and pass the renamed flag to the viewport resolver.apps/nextjs/src/app/[locale]/boards/(content)/_header-actions.tsx (1)
67-79: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
isMobilegating below the early return is dead code.Line 67 already returns for mobile, so
!isMobileon line 77 is always true andhidden={isMobile}on line 79 is alwaysfalse— which also makes the entirehiddenbranch inEditModeMenu(lines 212, 221) unreachable. Either drop thehiddenprop or keepEditModeMenurendered on mobile if hotkey suppression was the goal.♻️ Suggested simplification
- {isEditMode && !isMobile && <AddMenu />} + {isEditMode && <AddMenu />} - <EditModeMenu demoReadOnly={demoReadOnly} hidden={isMobile} /> + <EditModeMenu demoReadOnly={demoReadOnly} />🤖 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 `@apps/nextjs/src/app/`[locale]/boards/(content)/_header-actions.tsx around lines 67 - 79, Remove the dead mobile gating from the desktop-only render path in the header actions component: update the isEditMode AddMenu condition to omit !isMobile and remove the always-false hidden={isMobile} prop from EditModeMenu. Keep the existing early mobile return and its MobileMoreMenu behavior unchanged.apps/nextjs/src/components/board/mobile/deferred-mobile-item.tsx (1)
27-57: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winKeep the viewport callback out of the observer dependencies.
MobileBoardItempasses the raw state setter (setIsNearViewport) asonNearViewportChange; each render changes state and recreates theIntersectionObserver. Use a ref-backed callback and exclude it from the effect dependencies so the observer lifecycle depends only oneagerandunmountWhenOffscreen.🤖 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 `@apps/nextjs/src/components/board/mobile/deferred-mobile-item.tsx` around lines 27 - 57, The useEffect in DeferredMobileItem recreates its viewport observer because onNearViewportChange changes identity on each render. Store the latest callback in a ref-backed stable handler, invoke that handler from the observer callback, and remove onNearViewportChange from the effect dependency array so the observer lifecycle depends only on eager and unmountWhenOffscreen.packages/db/queries/server-setting.ts (2)
46-75: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRedundant double-parse of the persisted "board" setting.
persistedSettingsis already parsed generically for"board"inside thereduce(line 57), thenpersistedBoardSettings.valueis parsed again separately (line 67) just to type-checkenableAutomaticMobileLayout. Capturing the parsed board value once (e.g., from within the reduce, or hoisting the board lookup) would avoid parsing the same JSON string twice per call.♻️ Example consolidation
- const mergedSettings = defaultServerSettingsKeys.reduce((acc, settingKey) => { - const setting = settings.find((candidate) => candidate.settingKey === settingKey); - if (!setting) { - // Typescript is not happy because the key is a union and it does not know that they are the same - acc[settingKey] = { ...defaultServerSettings[settingKey] } as never; - return acc; - } - - const persistedSettings = parse<Record<string, unknown>>(setting.value); - acc[settingKey] = { - ...defaultServerSettings[settingKey], - ...persistedSettings, - } as never; - return acc; - }, {} as ServerSettings); - - const persistedBoardSettings = settings.find((setting) => setting.settingKey === "board"); - const parsedBoardSettings = persistedBoardSettings - ? parse<Partial<ServerSettings["board"]>>(persistedBoardSettings.value) - : {}; - - if (typeof parsedBoardSettings.enableAutomaticMobileLayout !== "boolean") { + let parsedBoardSettings: Partial<ServerSettings["board"]> = {}; + const mergedSettings = defaultServerSettingsKeys.reduce((acc, settingKey) => { + const setting = settings.find((candidate) => candidate.settingKey === settingKey); + if (!setting) { + acc[settingKey] = { ...defaultServerSettings[settingKey] } as never; + return acc; + } + + const persistedSettings = parse<Record<string, unknown>>(setting.value); + if (settingKey === "board") parsedBoardSettings = persistedSettings as Partial<ServerSettings["board"]>; + acc[settingKey] = { + ...defaultServerSettings[settingKey], + ...persistedSettings, + } as never; + return acc; + }, {} as ServerSettings); + + if (typeof parsedBoardSettings.enableAutomaticMobileLayout !== "boolean") { mergedSettings.board.enableAutomaticMobileLayout = await getAutomaticMobileLayoutUpgradeDefaultAsync(db); }🤖 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/db/queries/server-setting.ts` around lines 46 - 75, Update getServerSettingsAsync to reuse the already parsed board settings from the defaultServerSettingsKeys reduce when checking enableAutomaticMobileLayout, instead of parsing persistedBoardSettings.value again. Preserve the existing fallback to getAutomaticMobileLayoutUpgradeDefaultAsync when the reused value is not boolean.
16-32: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider a test for the duplicate-canonical-layout branch.
The
nextCount > 1per-board counter path (defending against duplicate "Base"/breakpoint-0 rows for one board) isn't exercised by any of the providedseed.spec.tscases — every negative test currently hits thename !== "Base" || breakpoint !== 0check first. A small added case with two{ name: "Base", breakpoint: 0 }rows for the sameboardIdwould close this gap for a heuristic that gates a server-wide rendering mode.🤖 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/db/queries/server-setting.ts` around lines 16 - 32, The test suite should cover the duplicate canonical-layout branch in shouldEnableAutomaticMobileLayoutForUpgrade by adding a case with two Base layouts at breakpoint 0 sharing the same boardId, and assert that it returns false.packages/widgets/src/media-requests/stats/component.tsx (1)
107-119: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMobile summary description hardcodes word order instead of using an ICU placeholder.
${pending} ${t("titles.stats.pending")}bakes English word order into the component. Other locales can't reorder the number relative to the label, unlike every other interpolated string inen.json(e.g.footer.updated,remaining).🛠️ Suggested approach
- description={`${pending} ${t("titles.stats.pending")}`} + description={t("titles.stats.pendingCount", { pending })}Add
"pendingCount": "{pending} pending approvals"(or similar) toen.jsonso translators control word order.🤖 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/media-requests/stats/component.tsx` around lines 107 - 119, The mobile summary description in the displayMode === "mobileSummary" branch should use a translated ICU message instead of concatenating pending and the localized label. Add a pending-count translation key with a {pending} placeholder to the existing English translations, then pass the pending value through that key in the WidgetMobileSummary description so locales can control word order.packages/api/src/settle-integrations.ts (1)
22-28: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
getIntegrationQueryProvenancesilently undercounts failures if afallbackis combined with it.
failedIntegrationCountis derived fromselectedIntegrationCount - results.length. This is accurate today because no caller passesfallback, butsettleIntegrationQueries'sfallbackoption backfills failed results into the same array, makingresults.length === integrations.lengtheven when integrations failed — silently zeroing outfailedIntegrationCountfor any future caller that combines both options.🛠️ Suggested doc guard
export const getIntegrationQueryProvenance = ( selectedIntegrationCount: number, results: readonly { isStale?: boolean }[], ): IntegrationQueryProvenance => ({ + // NOTE: assumes `results` came from settleIntegrationQueries without a `fallback` option. + // A `fallback` backfills failed integrations into `results`, which would undercount failures here. failedIntegrationCount: Math.max(0, selectedIntegrationCount - results.length), staleIntegrationCount: results.filter((result) => result.isStale === true).length, });🤖 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/api/src/settle-integrations.ts` around lines 22 - 28, Update getIntegrationQueryProvenance and its callers so failedIntegrationCount is based on the original failed-query information rather than results.length when settleIntegrationQueries uses fallback. Preserve staleIntegrationCount from the returned results, and add a guard in the relevant API or documentation to prevent combining fallback with provenance calculation unless failure counts remain available.e2e/mobile-board.spec.ts (1)
164-182: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueScenario selection keyed on pixel widths is fragile.
viewport.width === 390/=== 320silently disables the reduced-motion, zoom-overflow, and interaction/RTL scenarios if a viewport size is ever tweaked. Put explicit flags on the viewport descriptors instead.♻️ Suggested restructure
- { name: "small phone", width: 320, height: 568, userAgent: devices["iPhone 13"].userAgent, phone: true }, - { name: "phone", width: 390, height: 844, userAgent: devices["iPhone 13"].userAgent, phone: true }, + { + name: "small phone", + width: 320, + height: 568, + userAgent: iPhoneUserAgent, + phone: true, + checksZoomOverflow: true, + }, + { + name: "phone", + width: 390, + height: 844, + userAgent: iPhoneUserAgent, + phone: true, + reducedMotion: true, + checksInteractions: true, + },then branch on
viewport.checksZoomOverflow/viewport.checksInteractions/viewport.reducedMotion.Also applies to: 191-191, 237-237, 251-251
🤖 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 `@e2e/mobile-board.spec.ts` around lines 164 - 182, Add explicit scenario flags to each mobileViewports descriptor for reduced-motion, zoom-overflow, and interaction/RTL checks, then update the affected scenario branches to use viewport.reducedMotion, viewport.checksZoomOverflow, and viewport.checksInteractions instead of hard-coded width comparisons. Preserve the current scenario assignments for each viewport while removing the fragile pixel-width coupling.packages/api/src/router/test/serverSettings.spec.ts (1)
133-153: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBackfill assertion can't fail for the wrong reason it's guarding.
On an empty DB the upgrade default resolves to the same value as
defaultServerSettings.board.enableAutomaticMobileLayout, so this test passes even if the backfill branch ingetServerSettingByKeyAsync(packages/db/queries/server-setting.ts:77-108) were removed. Seed a board with a saved responsive layout first so the upgrade default flips, then assert the flipped value — that actually exercises the legacy path.🤖 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/api/src/router/test/serverSettings.spec.ts` around lines 133 - 153, The test “getBoardSettings should backfill the automatic layout default for legacy values” currently cannot distinguish the backfill behavior from the normal default. Seed the database with a saved responsive layout before inserting the legacy board settings, then assert that getBoardSettings returns the flipped enableAutomaticMobileLayout value while preserving the other default board settings.
🤖 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 `@apps/docs/docs/management/api/index.mdx`:
- Around line 83-88: Remove the UI-only saved responsive layouts bullet from the
OpenAPI/MCP endpoint list near the `boards` and `settings` API reference
guidance. Keep the endpoint list limited to API-supported capabilities, and
preserve the note explaining that saved responsive layouts are intentionally not
exposed as OpenAPI or MCP operations.
In `@apps/docs/docs/management/boards/index.mdx`:
- Line 273: Update the sentence describing the global home board to refer to
people as “users who,” replacing “users that” while preserving the meaning and
surrounding wording.
- Line 37: Update the sentence in the board documentation to use the hyphenated
compound adjective “drag-and-drop” instead of “drag and drop,” without changing
the surrounding wording.
In `@apps/nextjs/src/app/`[locale]/boards/(content)/_header-actions.tsx:
- Around line 553-580: Update handleClick to bypass leave confirmation and
preserve normal browser behavior for same-document hash-only links matched by
anchorSelector. Detect destinations whose URL has no pathname or search change
and only a hash fragment, returning before preventDefault(); keep confirmation
and navigation handling unchanged for actual page transitions.
In `@packages/api/src/router/serverSettings.ts`:
- Around line 105-119: The saveSettings mutation currently uses a non-atomic
findFirst-then-insert/update sequence that can race under concurrent writes.
Replace this branch and the equivalent logic in updateBoardSettings with a
shared dialect-aware Drizzle upsert helper using onConflictDoUpdate for
PostgreSQL/SQLite and the MySQL equivalent, preserving the existing setting-key
conflict behavior and merged next value.
In `@packages/boards/src/edit-mode.tsx`:
- Around line 15-28: Update requestBoardEditAction so both execution paths
handle rejected promises from BoardEditAction through the application’s existing
error-reporting mechanism instead of discarding them with void action().
Preserve the current SSR and cancellable-event behavior, and reuse the
established error-reporting symbol rather than introducing new handling.
In `@packages/modals/src/index.tsx`:
- Around line 127-131: The OpenModalOptions.onClose callback is declared but not
invoked when a modal closes. In packages/modals/src/index.tsx:127-131, update
the ActiveModal close flow, such as handleCloseModal or closeModal, to retrieve
and invoke the stored onClose callback for every dismissal path. In
packages/modals/src/confirm-modal.tsx:19, keep onClose options-only with no
direct component change, unless explicitly forwarding innerProps.onClose is
required by the chosen provider implementation.
In `@packages/request-handler/src/lib/request-handler.ts`:
- Line 8: Remove the duplicate RequestResult<TData> type alias declaration,
retaining only one definition used by the request-handler module so TypeScript
no longer reports a duplicate identifier.
- Around line 27-29: Update invalidateCache and the request/inflight resolution
logic to prevent pre-invalidation promises from writing cache entries or
deleting newer inflight requests. Track the cache generation or exact promise
identity, and only apply completion updates when they still belong to the
current generation/request. Add a regression test covering invalidate → refetch
→ old request resolves.
In `@packages/widgets/src/common/query-state.tsx`:
- Around line 40-73: Update WidgetDataState so the warning is accessible on
touch devices: either render the translated warning label inline, or configure
the Tooltip to respond to touch and focus while making the Center trigger
keyboard-focusable. Preserve the existing warning visibility condition and
styling, and ensure users on mobile can discover the stale/failed-data
indicator.
In `@packages/widgets/src/media-missing/component.tsx`:
- Around line 40-46: Update the media-missing widget’s mobile and desktop
rendering to detect partial integration failures with hasWidgetDataWarning using
expectedIntegrationCount and receivedIntegrationCount, rather than relying only
on error. Wrap the desktop tabbed view in WidgetDataState, and apply the same
warning state to the mobile summary while preserving the existing loading and
all-failures error behavior.
In `@packages/widgets/src/rssFeed/component.tsx`:
- Around line 57-65: Sanitize feedEntry.description before it reaches the
dangerouslySetInnerHTML sink in the RSS widget render. Update the description
rendering in the component to use an established sanitizer or safely re-encode
script-capable markup, while preserving the existing hideDescription, Text
props, and text content behavior.
---
Nitpick comments:
In `@apps/nextjs/src/app/`[locale]/boards/(content)/_header-actions.tsx:
- Around line 67-79: Remove the dead mobile gating from the desktop-only render
path in the header actions component: update the isEditMode AddMenu condition to
omit !isMobile and remove the always-false hidden={isMobile} prop from
EditModeMenu. Keep the existing early mobile return and its MobileMoreMenu
behavior unchanged.
In `@apps/nextjs/src/components/board/mobile/deferred-mobile-item.tsx`:
- Around line 27-57: The useEffect in DeferredMobileItem recreates its viewport
observer because onNearViewportChange changes identity on each render. Store the
latest callback in a ref-backed stable handler, invoke that handler from the
observer callback, and remove onNearViewportChange from the effect dependency
array so the observer lifecycle depends only on eager and unmountWhenOffscreen.
In `@apps/nextjs/src/components/board/mobile/mobile-presentation.ts`:
- Around line 18-19: Update the mobile presentation options so
unmountWhenOffscreen is disabled whenever the resolved eager value is true,
including when eager uses the app/bookmarks default. Base the condition on the
resolved eager expression or value rather than only mobile?.eager === true,
while preserving the existing fallback behavior for non-eager items.
In `@apps/nextjs/src/components/board/mobile/mobile-viewport-observer.spec.ts`:
- Around line 39-47: Reset the module-scoped observer state between tests in the
mobile viewport observer spec by using vi.resetModules() and dynamically
importing mobile-viewport-observer for each test, or by invoking an exported
test reset hook. Ensure observer, subscriptions, and resize-listener state is
freshly initialized before every test while preserving the existing
IntersectionObserverMock setup.
In `@apps/nextjs/src/components/board/use-mobile-board.spec.ts`:
- Around line 77-85: Move the shouldShowMobileBoardViewportSkeleton test out of
the resolveIsAutomaticMobileBoard describe block into a dedicated describe. Add
coverage for enableAutomaticMobileLayout true with isResolved false and assert
that the skeleton is shown, while preserving the existing legacy-mode assertion.
In `@apps/nextjs/src/components/board/use-mobile-board.tsx`:
- Around line 76-89: Rename hasResolvedClientWidth and its setter to hasMounted
and setHasMounted throughout the component, including the useEffect and
resolveMobileBoardViewport call. Preserve the existing mount-only initialization
and pass the renamed flag to the viewport resolver.
In `@e2e/mobile-board.spec.ts`:
- Around line 164-182: Add explicit scenario flags to each mobileViewports
descriptor for reduced-motion, zoom-overflow, and interaction/RTL checks, then
update the affected scenario branches to use viewport.reducedMotion,
viewport.checksZoomOverflow, and viewport.checksInteractions instead of
hard-coded width comparisons. Preserve the current scenario assignments for each
viewport while removing the fragile pixel-width coupling.
In `@packages/api/src/router/test/serverSettings.spec.ts`:
- Around line 133-153: The test “getBoardSettings should backfill the automatic
layout default for legacy values” currently cannot distinguish the backfill
behavior from the normal default. Seed the database with a saved responsive
layout before inserting the legacy board settings, then assert that
getBoardSettings returns the flipped enableAutomaticMobileLayout value while
preserving the other default board settings.
In `@packages/api/src/settle-integrations.ts`:
- Around line 22-28: Update getIntegrationQueryProvenance and its callers so
failedIntegrationCount is based on the original failed-query information rather
than results.length when settleIntegrationQueries uses fallback. Preserve
staleIntegrationCount from the returned results, and add a guard in the relevant
API or documentation to prevent combining fallback with provenance calculation
unless failure counts remain available.
In `@packages/boards/src/context.spec.ts`:
- Around line 16-36: Extend the “board layout selection” tests to cover
getCurrentLayout with legacy mode and an undefined viewport, asserting it falls
back to the desktop “large” layout. Also add a test for getCurrentLayout when
the board has no layouts, asserting it throws the “Board must have a layout”
error.
In `@packages/db/queries/server-setting.ts`:
- Around line 46-75: Update getServerSettingsAsync to reuse the already parsed
board settings from the defaultServerSettingsKeys reduce when checking
enableAutomaticMobileLayout, instead of parsing persistedBoardSettings.value
again. Preserve the existing fallback to
getAutomaticMobileLayoutUpgradeDefaultAsync when the reused value is not
boolean.
- Around line 16-32: The test suite should cover the duplicate canonical-layout
branch in shouldEnableAutomaticMobileLayoutForUpgrade by adding a case with two
Base layouts at breakpoint 0 sharing the same boardId, and assert that it
returns false.
In `@packages/widgets/src/media-requests/stats/component.tsx`:
- Around line 107-119: The mobile summary description in the displayMode ===
"mobileSummary" branch should use a translated ICU message instead of
concatenating pending and the localized label. Add a pending-count translation
key with a {pending} placeholder to the existing English translations, then pass
the pending value through that key in the WidgetMobileSummary description so
locales can control word order.
🪄 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: 0c82b604-38c2-4034-a8be-047d17880948
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (200)
apps/docs/docs/management/api/index.mdxapps/docs/docs/management/boards/index.mdxapps/docs/docs/management/settings/index.mdxapps/nextjs/src/app/[locale]/boards/(content)/_client.tsxapps/nextjs/src/app/[locale]/boards/(content)/_creator.tsxapps/nextjs/src/app/[locale]/boards/(content)/_header-actions.tsxapps/nextjs/src/app/[locale]/boards/(content)/_prefetch-policy.spec.tsapps/nextjs/src/app/[locale]/boards/(content)/_prefetch-policy.tsapps/nextjs/src/app/[locale]/boards/[name]/settings/_layout.tsxapps/nextjs/src/app/[locale]/boards/[name]/settings/page.tsxapps/nextjs/src/app/[locale]/boards/_board-switcher-logo.tsxapps/nextjs/src/app/[locale]/boards/_header-actions.tsxapps/nextjs/src/app/[locale]/boards/_layout-creator.tsxapps/nextjs/src/app/[locale]/boards/layout.tsxapps/nextjs/src/app/[locale]/layout.tsxapps/nextjs/src/app/[locale]/manage/settings/_components/board-settings-form.tsxapps/nextjs/src/app/[locale]/manage/settings/_components/common-form.tsxapps/nextjs/src/components/board/items/actions/create-item.tsapps/nextjs/src/components/board/items/actions/empty-position.tsapps/nextjs/src/components/board/items/actions/move-and-resize-item.tsapps/nextjs/src/components/board/items/actions/move-item-to-section.tsapps/nextjs/src/components/board/items/actions/test/create-item.spec.tsapps/nextjs/src/components/board/items/actions/test/empty-position.spec.tsapps/nextjs/src/components/board/items/actions/test/move-and-resize-item.spec.tsapps/nextjs/src/components/board/items/actions/test/move-item-to-section.spec.tsapps/nextjs/src/components/board/items/item-content.tsxapps/nextjs/src/components/board/mobile/deferred-mobile-item.spec.tsapps/nextjs/src/components/board/mobile/deferred-mobile-item.tsxapps/nextjs/src/components/board/mobile/mobile-board.module.cssapps/nextjs/src/components/board/mobile/mobile-board.tsxapps/nextjs/src/components/board/mobile/mobile-dialog-stack.spec.tsapps/nextjs/src/components/board/mobile/mobile-dialog-stack.tsapps/nextjs/src/components/board/mobile/mobile-layout.spec.tsapps/nextjs/src/components/board/mobile/mobile-layout.tsapps/nextjs/src/components/board/mobile/mobile-presentation.spec.tsapps/nextjs/src/components/board/mobile/mobile-presentation.tsapps/nextjs/src/components/board/mobile/mobile-section-navigation.spec.tsapps/nextjs/src/components/board/mobile/mobile-section-navigation.tsapps/nextjs/src/components/board/mobile/mobile-viewport-observer.spec.tsapps/nextjs/src/components/board/mobile/mobile-viewport-observer.tsapps/nextjs/src/components/board/mobile/mobile-widget-actions.tsxapps/nextjs/src/components/board/sections/category/category-menu-actions.tsxapps/nextjs/src/components/board/sections/dynamic-section.tsxapps/nextjs/src/components/board/sections/gridstack/gridstack.tsxapps/nextjs/src/components/board/sections/gridstack/use-gridstack.tsapps/nextjs/src/components/board/sections/section-actions.tsxapps/nextjs/src/components/board/use-mobile-board.spec.tsapps/nextjs/src/components/board/use-mobile-board.tsxapps/nextjs/src/components/language/current-language-combobox.tsxapps/nextjs/src/components/layout/header.tsxapps/nextjs/src/components/layout/header/button.tsxapps/nextjs/src/components/layout/header/responsive-user.tsxapps/nextjs/src/components/layout/header/search.tsxapps/nextjs/src/components/layout/header/user.tsxapps/nextjs/src/components/layout/shell.tsxapps/nextjs/src/components/user-avatar-menu.tsxe2e/mobile-board.spec.tspackages/api/src/router/board.tspackages/api/src/router/serverSettings.tspackages/api/src/router/test/board.spec.tspackages/api/src/router/test/serverSettings.spec.tspackages/api/src/router/widgets/beszel.tspackages/api/src/router/widgets/calendar.tspackages/api/src/router/widgets/downloads.tspackages/api/src/router/widgets/health-monitoring.tspackages/api/src/router/widgets/media-release.tspackages/api/src/router/widgets/media-requests.tspackages/api/src/router/widgets/media-server.tspackages/api/src/router/widgets/notifications.tspackages/api/src/router/widgets/uptime-kuma.tspackages/api/src/settle-integrations.spec.tspackages/api/src/settle-integrations.tspackages/api/src/test/mcp.spec.tspackages/boards/package.jsonpackages/boards/src/context.spec.tspackages/boards/src/context.tsxpackages/boards/src/edit-mode.spec.tspackages/boards/src/edit-mode.tsxpackages/db/migrations/seed.spec.tspackages/db/migrations/seed.tspackages/db/queries/server-setting.tspackages/modals/src/confirm-modal.tsxpackages/modals/src/index.tsxpackages/request-handler/src/lib/request-handler.spec.tspackages/request-handler/src/lib/request-handler.tspackages/request-handler/src/media-release.tspackages/server-settings/src/index.tspackages/settings/src/creator.tspackages/spotlight/package.jsonpackages/spotlight/src/preferences/use-user-preference.tspackages/translation/src/lang/en.jsonpackages/ui/src/index.tspackages/ui/src/theme.tspackages/ui/src/theme/layers.tspackages/ui/src/theme/modal.spec.tspackages/ui/src/theme/modal.tspackages/validation/src/board.tspackages/widgets/src/anchor-note/index.tspackages/widgets/src/app/component.tsxpackages/widgets/src/app/index.tspackages/widgets/src/archive-team-warrior/index.tspackages/widgets/src/audio-stats/index.tspackages/widgets/src/bazarr/index.tspackages/widgets/src/beszel-alerts/component.tsxpackages/widgets/src/beszel-alerts/index.tspackages/widgets/src/beszel-system-grid/component.tsxpackages/widgets/src/beszel-system-grid/index.tspackages/widgets/src/beszel-system-stats/component.tsxpackages/widgets/src/beszel-system-stats/index.tspackages/widgets/src/beszel-system-table/component.tsxpackages/widgets/src/beszel-system-table/index.tspackages/widgets/src/beszel/_shared/error-indicator.tsxpackages/widgets/src/beszel/_shared/mobile-summary.tsxpackages/widgets/src/beszel/_shared/query-status.spec.tspackages/widgets/src/beszel/_shared/query-status.tspackages/widgets/src/beszel/_shared/stats-view.tsxpackages/widgets/src/bookmarks/component.tsxpackages/widgets/src/bookmarks/index.tsxpackages/widgets/src/calendar/component.tsxpackages/widgets/src/calendar/index.tspackages/widgets/src/clock/index.tspackages/widgets/src/common/mobile-summary.tsxpackages/widgets/src/common/query-state.spec.tspackages/widgets/src/common/query-state.tsxpackages/widgets/src/coolify/applications-section.tsxpackages/widgets/src/coolify/component.tsxpackages/widgets/src/coolify/index.tspackages/widgets/src/coolify/instance-card.tsxpackages/widgets/src/coolify/resource-row.tsxpackages/widgets/src/coolify/servers-section.tsxpackages/widgets/src/coolify/services-section.tsxpackages/widgets/src/coolify/single-instance-layout.tsxpackages/widgets/src/custom-api/component.tsxpackages/widgets/src/custom-api/index.tspackages/widgets/src/custom-api/jsx-interactive-components.tsxpackages/widgets/src/definition.tspackages/widgets/src/dns-hole/controls/TimerModal.tsxpackages/widgets/src/dns-hole/controls/component.tsxpackages/widgets/src/dns-hole/controls/index.tspackages/widgets/src/dns-hole/summary/index.tspackages/widgets/src/docker/component.tsxpackages/widgets/src/docker/index.tspackages/widgets/src/downloads/component.tsxpackages/widgets/src/downloads/index.tspackages/widgets/src/filter-storage-volumes.spec.tspackages/widgets/src/filter-storage-volumes.tspackages/widgets/src/firewall/index.tspackages/widgets/src/health-monitoring/component.tsxpackages/widgets/src/health-monitoring/index.tspackages/widgets/src/health-monitoring/system-health.tsxpackages/widgets/src/iframe/component.tsxpackages/widgets/src/iframe/index.tspackages/widgets/src/index.tsxpackages/widgets/src/indexer-manager/index.tspackages/widgets/src/media-missing/component.tsxpackages/widgets/src/media-missing/index.tspackages/widgets/src/media-releases/component.tsxpackages/widgets/src/media-releases/index.tspackages/widgets/src/media-requests/list/component.tsxpackages/widgets/src/media-requests/list/index.tspackages/widgets/src/media-requests/stats/component.tsxpackages/widgets/src/media-requests/stats/index.tspackages/widgets/src/media-server/component.tsxpackages/widgets/src/media-server/index.tspackages/widgets/src/media-transcoding/component.tsxpackages/widgets/src/media-transcoding/index.tspackages/widgets/src/notebook/component.tsxpackages/widgets/src/notebook/index.tspackages/widgets/src/notebook/mobile-summary.spec.tspackages/widgets/src/notebook/mobile-summary.tspackages/widgets/src/notebook/notebook.tsxpackages/widgets/src/notifications/component.tsxpackages/widgets/src/notifications/index.tspackages/widgets/src/paperless-ngx/index.tspackages/widgets/src/patchmon/index.tspackages/widgets/src/releases/index.tspackages/widgets/src/rssFeed/component.tsxpackages/widgets/src/rssFeed/index.tspackages/widgets/src/smart-home/entity-state/index.tspackages/widgets/src/smart-home/execute-automation/index.tspackages/widgets/src/speedtest-tracker/index.tspackages/widgets/src/stocks/index.tspackages/widgets/src/system-disks/component.tsxpackages/widgets/src/system-disks/index.tspackages/widgets/src/system-disks/mobile-summary.spec.tspackages/widgets/src/system-disks/mobile-summary.tspackages/widgets/src/system-resources/component.tsxpackages/widgets/src/system-resources/index.tspackages/widgets/src/timetable/index.tspackages/widgets/src/tracearr/index.tspackages/widgets/src/traefik/index.tspackages/widgets/src/umami/index.tspackages/widgets/src/ups/index.tspackages/widgets/src/uptime-kuma/component.tsxpackages/widgets/src/uptime-kuma/index.tspackages/widgets/src/vpn/index.tspackages/widgets/src/weather/component.tsxpackages/widgets/src/weather/index.tspackages/widgets/src/weather/temperature.spec.tspackages/widgets/src/weather/temperature.ts
🚧 Files skipped from review as they are similar to previous changes (114)
- apps/nextjs/src/components/board/mobile/deferred-mobile-item.spec.ts
- apps/nextjs/src/components/layout/header/responsive-user.tsx
- packages/widgets/src/weather/temperature.spec.ts
- packages/widgets/src/beszel/_shared/query-status.ts
- packages/widgets/src/calendar/index.ts
- packages/widgets/src/system-disks/mobile-summary.spec.ts
- packages/widgets/src/custom-api/index.ts
- packages/widgets/src/audio-stats/index.ts
- packages/widgets/src/beszel-system-stats/index.ts
- packages/widgets/src/beszel/_shared/query-status.spec.ts
- packages/widgets/src/beszel-system-grid/index.ts
- packages/api/src/router/widgets/uptime-kuma.ts
- apps/nextjs/src/components/layout/header/search.tsx
- packages/widgets/src/notifications/index.ts
- apps/nextjs/src/components/board/mobile/mobile-dialog-stack.spec.ts
- packages/widgets/src/ups/index.ts
- packages/widgets/src/media-server/index.ts
- packages/widgets/src/dns-hole/controls/index.ts
- packages/widgets/src/firewall/index.ts
- packages/widgets/src/weather/index.ts
- packages/widgets/src/releases/index.ts
- packages/widgets/src/system-disks/index.ts
- packages/widgets/src/system-disks/mobile-summary.ts
- packages/widgets/src/coolify/applications-section.tsx
- packages/widgets/src/downloads/index.ts
- packages/widgets/src/bazarr/index.ts
- apps/nextjs/src/components/board/mobile/mobile-section-navigation.ts
- apps/nextjs/src/components/layout/header/user.tsx
- packages/widgets/src/notebook/index.ts
- packages/widgets/src/media-transcoding/index.ts
- apps/nextjs/src/components/board/mobile/mobile-section-navigation.spec.ts
- apps/nextjs/src/app/[locale]/boards/layout.tsx
- packages/widgets/src/notebook/mobile-summary.spec.ts
- packages/widgets/src/docker/index.ts
- packages/widgets/src/system-resources/index.ts
- packages/widgets/src/bookmarks/index.tsx
- packages/api/src/router/widgets/beszel.ts
- packages/widgets/src/media-releases/index.ts
- packages/widgets/src/tracearr/index.ts
- packages/widgets/src/vpn/index.ts
- packages/widgets/src/indexer-manager/index.ts
- packages/widgets/src/coolify/instance-card.tsx
- packages/widgets/src/coolify/services-section.tsx
- packages/widgets/src/beszel-system-table/index.ts
- packages/widgets/src/umami/index.ts
- packages/widgets/src/notebook/mobile-summary.ts
- packages/widgets/src/iframe/index.ts
- packages/widgets/src/anchor-note/index.ts
- packages/widgets/src/paperless-ngx/index.ts
- packages/widgets/src/index.tsx
- packages/api/src/settle-integrations.spec.ts
- apps/nextjs/src/app/[locale]/boards/(content)/_prefetch-policy.spec.ts
- apps/nextjs/src/app/[locale]/boards/_header-actions.tsx
- packages/widgets/src/media-requests/stats/index.ts
- packages/widgets/src/app/index.ts
- packages/widgets/src/rssFeed/index.ts
- packages/widgets/src/beszel/_shared/mobile-summary.tsx
- packages/widgets/src/common/mobile-summary.tsx
- apps/nextjs/src/components/board/mobile/mobile-dialog-stack.ts
- apps/nextjs/src/components/layout/header/button.tsx
- apps/nextjs/src/app/[locale]/boards/(content)/_prefetch-policy.ts
- packages/widgets/src/weather/temperature.ts
- packages/widgets/src/uptime-kuma/index.ts
- apps/nextjs/src/app/[locale]/boards/_board-switcher-logo.tsx
- packages/widgets/src/filter-storage-volumes.spec.ts
- packages/widgets/src/media-missing/index.ts
- packages/widgets/src/notebook/notebook.tsx
- packages/widgets/src/iframe/component.tsx
- packages/widgets/src/timetable/index.ts
- packages/widgets/src/smart-home/entity-state/index.ts
- packages/widgets/src/clock/index.ts
- packages/widgets/src/traefik/index.ts
- apps/nextjs/src/app/[locale]/boards/(content)/_client.tsx
- packages/widgets/src/coolify/index.ts
- packages/widgets/src/media-releases/component.tsx
- packages/widgets/src/app/component.tsx
- packages/widgets/src/coolify/single-instance-layout.tsx
- packages/api/src/router/widgets/media-server.ts
- packages/widgets/src/beszel-alerts/index.ts
- packages/widgets/src/dns-hole/controls/TimerModal.tsx
- packages/widgets/src/coolify/component.tsx
- packages/widgets/src/beszel/_shared/stats-view.tsx
- packages/widgets/src/patchmon/index.ts
- packages/widgets/src/coolify/resource-row.tsx
- packages/widgets/src/smart-home/execute-automation/index.ts
- packages/widgets/src/health-monitoring/system-health.tsx
- apps/nextjs/src/app/[locale]/boards/(content)/_creator.tsx
- packages/widgets/src/custom-api/jsx-interactive-components.tsx
- packages/widgets/src/media-requests/list/index.ts
- packages/widgets/src/beszel-system-grid/component.tsx
- packages/api/src/router/widgets/downloads.ts
- packages/widgets/src/stocks/index.ts
- packages/widgets/src/beszel/_shared/error-indicator.tsx
- packages/api/src/router/widgets/calendar.ts
- packages/widgets/src/media-transcoding/component.tsx
- apps/nextjs/src/components/layout/header.tsx
- packages/widgets/src/dns-hole/controls/component.tsx
- apps/nextjs/src/components/board/mobile/mobile-layout.ts
- packages/widgets/src/filter-storage-volumes.ts
- packages/widgets/src/beszel-system-table/component.tsx
- packages/widgets/src/beszel-alerts/component.tsx
- packages/widgets/src/custom-api/component.tsx
- packages/widgets/src/notifications/component.tsx
- apps/nextjs/src/components/board/mobile/mobile-layout.spec.ts
- apps/nextjs/src/components/board/mobile/mobile-board.tsx
- packages/widgets/src/beszel-system-stats/component.tsx
- packages/widgets/src/coolify/servers-section.tsx
- packages/widgets/src/media-server/component.tsx
- packages/widgets/src/health-monitoring/component.tsx
- packages/widgets/src/media-requests/list/component.tsx
- packages/widgets/src/notebook/component.tsx
- packages/widgets/src/system-disks/component.tsx
- packages/widgets/src/downloads/component.tsx
- apps/nextjs/src/components/board/items/item-content.tsx
This reverts commit d0b9348.
This reverts commit 7959c20.
This reverts commit cf83e0d.
This reverts commit a540f07.
This reverts commit 7ce1081.
This reverts commit 6dbfbb1.
This reverts commit 1039bb1.
This reverts commit fa3d037.
This reverts commit 4bd0d8e.
This reverts commit 315520b.
This reverts commit 9f0969e.
This reverts commit 7c0e98c.
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/api/src/router/serverSettings.ts (1)
58-58: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winExplain how MCP callers obtain board IDs.
Line 58 defines which board IDs are valid, but not where callers retrieve them. Name the relevant public-board listing tool/API and its
idfield in the description.As per coding guidelines, MCP descriptions must explain “how to obtain required IDs.”
🤖 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/api/src/router/serverSettings.ts` at line 58, Update the MCP description near the global board defaults definition to explain that callers can obtain valid board IDs from the public-board listing tool/API, using each returned board’s id field. Preserve the existing validation and optional-field details.Source: Coding guidelines
apps/nextjs/src/app/[locale]/boards/(content)/_header-actions.tsx (2)
190-203: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winEdit toggle disappears mid-edit if the viewport crosses the mobile breakpoint.
hiddencomes fromuseIsMobileBoard(), whose media query re-evaluates live after mount. If a user starts editing on a desktop-sized viewport and then resizes below the breakpoint,hiddenflips totrue, hiding the save/discard button whileisEditMode(and any unsaved changes) is still active — the user is stuck until they resize back. Consider keeping the button visible wheneverisEditModeistrue, regardless ofhidden.♻️ Suggested tweak
- if (hidden) return null; + if (hidden && !isEditMode) return null;🤖 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 `@apps/nextjs/src/app/`[locale]/boards/(content)/_header-actions.tsx around lines 190 - 203, Update the visibility guard in the header actions around the `isEditMode` and `hidden` checks so the edit toggle remains rendered whenever `isEditMode` is true, even if `hidden` changes after a mobile breakpoint resize. Preserve the existing hidden behavior when not editing and keep the `toggle` callback’s save, discard, and open behavior unchanged.
245-303: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAnchor listeners are attached once and miss dynamically-rendered links.
document.querySelectorAll(anchorSelector)runs once per effect invocation (keyed only onisDirty), and listeners are bound only to the anchors that exist in the DOM at that moment. None of theMenucomponents in this file (SelectBoardsMenu,AddMenu's target, etc.) passkeepMounted, so per Mantine's default behavior theirMenu.Dropdowncontent — including the board-switcherMenu.Itemlinks — isn't in the DOM until first opened, and can be unmounted again after closing. Clicking any such link therefore bypassesopenConfirmModalentirely and navigates away viarouter.push, silently discarding unsaved edits.Prefer restoring event delegation on
documentwithevent.target.closest(anchorSelector)so newly-rendered anchors are always covered without needing to re-query the DOM.Static analysis also flags the effect's missing deps (
openConfirmModal,t,router); closures can go stale if these change whileisDirtyremainstrue.🛡️ Suggested delegation-based fix
const handleClick = (event: Event) => { - const target = (event.target as HTMLElement).closest("a"); + const target = (event.target as HTMLElement).closest<HTMLAnchorElement>(anchorSelector); if (!target) { - console.warn("No anchor element found for click event", event); return; } event.preventDefault(); ... }; ... - const anchors = document.querySelectorAll(anchorSelector); - anchors.forEach((link) => { - link.addEventListener("click", handleClick); - }); + document.addEventListener("click", handleClick); window.addEventListener("popstate", handlePopState); window.addEventListener("beforeunload", handleBeforeUnload); return () => { - anchors.forEach((link) => { - link.removeEventListener("click", handleClick); - }); + document.removeEventListener("click", handleClick); window.removeEventListener("popstate", handlePopState); window.removeEventListener("beforeunload", handleBeforeUnload); }; - }, [isDirty]); + }, [isDirty, openConfirmModal, t, router]);🤖 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 `@apps/nextjs/src/app/`[locale]/boards/(content)/_header-actions.tsx around lines 245 - 303, Update usePreventLeaveWithDirty to delegate anchor clicks from document instead of querying and binding the current anchors, using event.target.closest(anchorSelector) so dynamically mounted menu links are intercepted. Register and clean up the document listener consistently, and include openConfirmModal, t, and router in the effect dependencies so the handler does not retain stale closures.Source: Linters/SAST tools
🧹 Nitpick comments (1)
apps/nextjs/src/components/board/mobile/mobile-board.module.css (1)
5-5: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded
/2mirrorsmobileColumnCountbut isn't tied to it.
grid-auto-rowsdivides the viewport width by a literal2to approximate one column's width, duplicating themobileColumnCount = 2constant used elsewhere. If that constant ever changes, this CSS silently desyncs.♻️ Suggested tweak
- grid-auto-rows: min(calc((100vw - 42px) / 2), 12rem); + grid-auto-rows: min(calc((100vw - 42px) / var(--mobile-column-count)), 12rem);🤖 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 `@apps/nextjs/src/components/board/mobile/mobile-board.module.css` at line 5, Update the mobile board grid sizing so the row calculation derives its column-width divisor from the existing mobileColumnCount configuration rather than a hardcoded 2. Keep the current spacing and 12rem maximum behavior unchanged, and ensure the CSS remains synchronized when the column count changes.
🤖 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 `@apps/nextjs/src/app/`[locale]/boards/_layout-creator.tsx:
- Around line 67-69: Update the initialIsMobile calculation in the layout
creator to treat both "mobile" and "tablet" user-agent device types as mobile,
matching useIsMobileBoard() and preventing tablet requests from initially
rendering the desktop board.
---
Outside diff comments:
In `@apps/nextjs/src/app/`[locale]/boards/(content)/_header-actions.tsx:
- Around line 190-203: Update the visibility guard in the header actions around
the `isEditMode` and `hidden` checks so the edit toggle remains rendered
whenever `isEditMode` is true, even if `hidden` changes after a mobile
breakpoint resize. Preserve the existing hidden behavior when not editing and
keep the `toggle` callback’s save, discard, and open behavior unchanged.
- Around line 245-303: Update usePreventLeaveWithDirty to delegate anchor clicks
from document instead of querying and binding the current anchors, using
event.target.closest(anchorSelector) so dynamically mounted menu links are
intercepted. Register and clean up the document listener consistently, and
include openConfirmModal, t, and router in the effect dependencies so the
handler does not retain stale closures.
In `@packages/api/src/router/serverSettings.ts`:
- Line 58: Update the MCP description near the global board defaults definition
to explain that callers can obtain valid board IDs from the public-board listing
tool/API, using each returned board’s id field. Preserve the existing validation
and optional-field details.
---
Nitpick comments:
In `@apps/nextjs/src/components/board/mobile/mobile-board.module.css`:
- Line 5: Update the mobile board grid sizing so the row calculation derives its
column-width divisor from the existing mobileColumnCount configuration rather
than a hardcoded 2. Keep the current spacing and 12rem maximum behavior
unchanged, and ensure the CSS remains synchronized when the column count
changes.
🪄 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: 79bfb641-5b74-4334-87fb-e3f11ea64df8
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (21)
.github/workflows/ci.ymlapps/docs/docs/management/api/index.mdxapps/docs/docs/management/boards/index.mdxapps/docs/docs/management/settings/index.mdxapps/nextjs/src/app/[locale]/boards/(content)/_header-actions.tsxapps/nextjs/src/app/[locale]/boards/_layout-creator.tsxapps/nextjs/src/components/board/mobile/mobile-board.module.cssapps/nextjs/src/components/board/mobile/mobile-board.tsxapps/nextjs/src/components/board/mobile/mobile-layout.spec.tsapps/nextjs/src/components/board/mobile/mobile-layout.tsapps/nextjs/src/components/board/use-mobile-board.tsxe2e/mobile-board.spec.tspackages/api/src/router/serverSettings.tspackages/api/src/router/test/serverSettings.spec.tspackages/boards/src/context.spec.tspackages/boards/src/edit-mode.tsxpackages/db/collection.tspackages/db/migrations/seed.spec.tspackages/db/queries/server-setting.tspackages/db/test/schema.spec.tspackages/translation/src/lang/en.json
💤 Files with no reviewable changes (1)
- packages/boards/src/edit-mode.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/boards/src/context.spec.ts
- packages/db/queries/server-setting.ts
- packages/db/migrations/seed.spec.ts
|
Feel free to try it @Nerdy-Technician |
Summary
Validation
Summary by CodeRabbit
New Features
Bug Fixes
Documentation