feat: connect settings page updated for responsive webgui #1585
Conversation
WalkthroughUpdates Connect.page to handle Flash Backup visibility based on user/connection state and standardize button layout with a loading indicator. Refactors SettingsGrid.vue to use class-based responsive grid utilities instead of custom CSS and media queries. No changes to exported/public APIs or business logic. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant P as Connect.page
participant C as Connect Plugin/Cloud Status
U->>P: Open Connect page
P->>C: Check registration & cloud connection
alt Not registered
P-->>U: Show "Disabled until you have signed in"
else Not cloud-connected
P-->>U: Show "Disabled until connected to Unraid Connect Cloud"
else Connected
P-->>U: Show loading indicator
P-->>U: Render Flash Backup form and controls
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Claude finished @zackspear's task —— View job Code Review - PR #1585
|
|
🚀 Storybook has been deployed to staging: https://unraid-ui-storybook-staging.unraid-workers.workers.dev |
|
it gucci |
|
🚀 Storybook has been deployed to staging: https://unraid-ui-storybook-staging.unraid-workers.workers.dev |
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.my.servers/Connect.page (1)
456-463: Fix JavaScript syntax error (stray character after brace)There is a stray "i" character after a closing brace, which will break all subsequent JS execution on this page.
- }i + }
🧹 Nitpick comments (3)
unraid-ui/src/components/layout/SettingsGrid.vue (1)
11-16: Optional: Avoid passing class via props; rely on Vue’s class mergeVue automatically merges the parent’s class attribute onto the root element. Passing class through props is unnecessary and may confuse readers.
Within this template block, remove props.class from cn(...). Also remove the defineProps typing for class.
Diff within these lines:
- 'grid grid-cols-1 md:grid-cols-[35%_1fr] md:[&>*:nth-child(odd)]:text-end gap-2 items-baseline md:gap-x-3 md:gap-y-6', - props.class + 'grid grid-cols-1 md:grid-cols-[35%_1fr] md:[&>*:nth-child(odd)]:text-end gap-2 items-baseline md:gap-x-3 md:gap-y-6'Outside these lines (script setup), remove the props typing entirely:
<script setup lang="ts"> import { cn } from '@/lib/utils'; // no defineProps needed; Vue will merge parent classes into the root element </script>plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.my.servers/Connect.page (2)
495-500: Standardize capitalization: “Flash Backup”Other strings use “Flash Backup” (capital B). Align these labels for consistency.
-_(Flash backup)_: +_(Flash Backup)_:- _(Flash backup)_: + _(Flash Backup)_:
502-502: Add aria-live to announce loading/state changes for screen readersflashbackuptext dynamically changes content. Make it accessible with aria-live/role.
-: <span id='flashbackuptext'><span class='blue p0'>_(Loading)_ <i class="fa fa-spinner fa-spin" aria-hidden="true"></i></span></span> +: <span id='flashbackuptext' aria-live="polite" role="status"><span class='blue p0'>_(Loading)_ <i class="fa fa-spinner fa-spin" aria-hidden="true"></i></span></span>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.my.servers/Connect.page(2 hunks)unraid-ui/src/components/layout/SettingsGrid.vue(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Avoid unnecessary or obvious comments; only add comments when needed for clarity
Files:
unraid-ui/src/components/layout/SettingsGrid.vue
**/components/**/*.vue
📄 CodeRabbit Inference Engine (.cursor/rules/web-testing-rules.mdc)
Some Vue files may require explicit imports like ref or computed due to Nuxt auto-imports not applying in tests
Files:
unraid-ui/src/components/layout/SettingsGrid.vue
🔇 Additional comments (3)
unraid-ui/src/components/layout/SettingsGrid.vue (1)
11-16: Confirm Tailwind support for text-end and arbitrary variant selectorThe utility text-end and the arbitrary variant selector [&>*:nth-child(odd)] require Tailwind versions that support logical alignments and arbitrary variants. If your Tailwind version lacks text-end, use text-right plus an RTL override.
Fallback (if needed):
- Replace text-end with text-right and add an RTL override:
md:[&>:nth-child(odd)]:text-right rtl:md:[&>:nth-child(odd)]:text-leftplugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.my.servers/Connect.page (2)
518-523: Ensure buttons-spaced class exists and is loaded on this pageYou’ve wrapped multiple button groups in span.buttons-spaced for consistent spacing. Confirm that:
- .buttons-spaced is defined in a stylesheet loaded by this page (not only in the SPA bundle).
- The class applies appropriate layout (e.g., inline-flex with gap) across classic/legacy pages.
If not guaranteed, add a minimal local style or ensure the global stylesheet is included to avoid layout regressions.
Also applies to: 525-529, 532-535, 541-545, 551-554, 560-564, 567-571
502-502: LGTM: Loading indicator and placeholderInitializing flashbackuptext with a loading spinner is a UX improvement and aligns with the subsequent Nchan-fed state updates.
| :class=" | ||
| cn( | ||
| 'grid grid-cols-1 md:grid-cols-[35%_1fr] [&>*:nth-child(odd)]:text-end gap-2 items-baseline md:gap-x-3 md:gap-y-6', | ||
| props.class | ||
| ) | ||
| " |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Scope odd-child text alignment to md+ to avoid awkward mobile layout
Right-aligning odd children on small screens can make single-column layouts look off (labels become right-aligned above inputs). Limit this alignment to md and above.
- 'grid grid-cols-1 md:grid-cols-[35%_1fr] [&>*:nth-child(odd)]:text-end gap-2 items-baseline md:gap-x-3 md:gap-y-6',
+ 'grid grid-cols-1 md:grid-cols-[35%_1fr] md:[&>*:nth-child(odd)]:text-end gap-2 items-baseline md:gap-x-3 md:gap-y-6',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| :class=" | |
| cn( | |
| 'grid grid-cols-1 md:grid-cols-[35%_1fr] [&>*:nth-child(odd)]:text-end gap-2 items-baseline md:gap-x-3 md:gap-y-6', | |
| props.class | |
| ) | |
| " | |
| :class=" | |
| cn( | |
| 'grid grid-cols-1 md:grid-cols-[35%_1fr] md:[&>*:nth-child(odd)]:text-end gap-2 items-baseline md:gap-x-3 md:gap-y-6', | |
| props.class | |
| ) | |
| " |
🤖 Prompt for AI Agents
In unraid-ui/src/components/layout/SettingsGrid.vue around lines 11 to 16, the
utility that right-aligns odd children is applying on all screen sizes; change
the selector so it only applies at md and above. Replace the class token
[&>*:nth-child(odd)]:text-end with md:[&>*:nth-child(odd)]:text-end so
single-column (mobile) layouts keep default left alignment while preserving
right-align for md+ breakpoints.
Summary by CodeRabbit