-
Couldn't load subscription status.
- Fork 11
fix(theme): API key white text on white background #1584
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
When generating an API key with the black theme active, the key was displayed with white text on a white background. This was caused by hardcoded light-theme classes (`bg-gray-50` and `border-gray-200`) on the `Input` component that displays the key. This change removes the hardcoded background and border color classes, allowing the themed styles to be applied correctly.
WalkthroughAdds two barrel index files to re-export common and form UI components, consolidating import paths. Updates ApiKeyManager.vue styling by removing background and border classes from the API key input without altering functionality. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 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 @elibosley's task —— View job Code Review - API Key Theme FixTodo List:
Review of ChangesAnalyzed the exact diff in - class="w-full font-mono text-base px-2 py-1 bg-gray-50 border border-gray-200 rounded pr-10"
+ class="w-full font-mono text-base px-2 py-1 rounded pr-10"✅ No critical issues found in changes The change correctly removes hardcoded light-theme classes ( The modification:
|
|
🚀 Storybook has been deployed to staging: https://unraid-ui-storybook-staging.unraid-workers.workers.dev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (2)
web/components/ApiKey/ApiKeyManager.vue (2)
176-181: Use theme tokens for the reveal button color; avoid hardcoded grays to ensure dark theme compatibility.The
text-gray-500 hover:text-gray-700pairing is light-theme biased. Prefer semantic tokens to adapt across themes.Apply this diff:
- <button + <button type="button" - class="absolute inset-y-0 right-2 flex items-center px-1 text-gray-500 hover:text-gray-700" + class="absolute inset-y-0 right-2 flex items-center px-1 text-muted-foreground hover:text-foreground" tabindex="-1" @click="toggleShowKey(key.id)" >
41-41: Remove stray console.log.Leftover debug logging will clutter the console.
Apply this diff:
- console.log(createdKey.value);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
unraid-ui/src/components/common/index.ts(1 hunks)unraid-ui/src/components/form/index.ts(1 hunks)web/components/ApiKey/ApiKeyManager.vue(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Avoid unnecessary or obvious comments; only add comments when needed for clarity
Files:
web/components/ApiKey/ApiKeyManager.vueunraid-ui/src/components/form/index.tsunraid-ui/src/components/common/index.ts
**/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:
web/components/ApiKey/ApiKeyManager.vue
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
TypeScript source files must use import specifiers with .js extensions for ESM compatibility
Files:
unraid-ui/src/components/form/index.tsunraid-ui/src/components/common/index.ts
🧠 Learnings (1)
📚 Learning: 2025-04-02T21:21:29.168Z
Learnt from: elibosley
PR: unraid/api#1308
File: unraid-ui/src/components/common/loading/Error.vue:2-2
Timestamp: 2025-04-02T21:21:29.168Z
Learning: Components in the unraid-ui folder require explicit imports and are not autoloaded, unlike other parts of the project that may use Nuxt.js autoloading features.
Applied to files:
unraid-ui/src/components/common/index.ts
🔇 Additional comments (1)
web/components/ApiKey/ApiKeyManager.vue (1)
172-172: Removing hardcoded bg/border classes from Input resolves the theme bug.Letting the Input inherit theme styles fixes the white-on-white issue as intended.
| export * from './accordion'; | ||
| export * from './badge'; | ||
| export * from './button'; | ||
| export * from './dialog'; | ||
| export * from './dropdown-menu'; | ||
| export * from './loading'; | ||
| export * from './popover'; | ||
| export * from './scroll-area'; | ||
| export * from './sheet'; | ||
| export * from './stepper'; | ||
| export * from './tabs'; | ||
| export * from './toast'; | ||
| export * from './tooltip'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Use .js extensions in re-export specifiers for ESM compatibility.
Per the repo’s guidelines for TypeScript ESM, relative specifiers should include .js. This avoids runtime resolution issues in Node ESM and bundlers that do not auto-resolve directory indexes.
Apply this diff:
-export * from './accordion';
-export * from './badge';
-export * from './button';
-export * from './dialog';
-export * from './dropdown-menu';
-export * from './loading';
-export * from './popover';
-export * from './scroll-area';
-export * from './sheet';
-export * from './stepper';
-export * from './tabs';
-export * from './toast';
-export * from './tooltip';
+export * from './accordion/index.js';
+export * from './badge/index.js';
+export * from './button/index.js';
+export * from './dialog/index.js';
+export * from './dropdown-menu/index.js';
+export * from './loading/index.js';
+export * from './popover/index.js';
+export * from './scroll-area/index.js';
+export * from './sheet/index.js';
+export * from './stepper/index.js';
+export * from './tabs/index.js';
+export * from './toast/index.js';
+export * from './tooltip/index.js';📝 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.
| export * from './accordion'; | |
| export * from './badge'; | |
| export * from './button'; | |
| export * from './dialog'; | |
| export * from './dropdown-menu'; | |
| export * from './loading'; | |
| export * from './popover'; | |
| export * from './scroll-area'; | |
| export * from './sheet'; | |
| export * from './stepper'; | |
| export * from './tabs'; | |
| export * from './toast'; | |
| export * from './tooltip'; | |
| export * from './accordion/index.js'; | |
| export * from './badge/index.js'; | |
| export * from './button/index.js'; | |
| export * from './dialog/index.js'; | |
| export * from './dropdown-menu/index.js'; | |
| export * from './loading/index.js'; | |
| export * from './popover/index.js'; | |
| export * from './scroll-area/index.js'; | |
| export * from './sheet/index.js'; | |
| export * from './stepper/index.js'; | |
| export * from './tabs/index.js'; | |
| export * from './toast/index.js'; | |
| export * from './tooltip/index.js'; |
🤖 Prompt for AI Agents
In unraid-ui/src/components/common/index.ts around lines 1 to 13, the re-export
specifiers lack .js extensions which breaks Node ESM/bundlers that don't resolve
directory indexes; update each export to use explicit .js file extensions (e.g.
export * from './accordion.js'; export * from './badge.js'; etc.) for all listed
modules so the relative ESM specifiers resolve at runtime.
| export * from './combobox'; | ||
| export * from './input'; | ||
| export * from './label'; | ||
| export * from './lightswitch'; | ||
| export * from './number'; | ||
| export * from './select'; | ||
| export * from './switch'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Align re-export specifiers with ESM by adding .js extensions.
Same rationale as the common barrel: ensures correct runtime resolution and adheres to the project’s ESM guideline.
Apply this diff:
-export * from './combobox';
-export * from './input';
-export * from './label';
-export * from './lightswitch';
-export * from './number';
-export * from './select';
-export * from './switch';
+export * from './combobox/index.js';
+export * from './input/index.js';
+export * from './label/index.js';
+export * from './lightswitch/index.js';
+export * from './number/index.js';
+export * from './select/index.js';
+export * from './switch/index.js';📝 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.
| export * from './combobox'; | |
| export * from './input'; | |
| export * from './label'; | |
| export * from './lightswitch'; | |
| export * from './number'; | |
| export * from './select'; | |
| export * from './switch'; | |
| export * from './combobox/index.js'; | |
| export * from './input/index.js'; | |
| export * from './label/index.js'; | |
| export * from './lightswitch/index.js'; | |
| export * from './number/index.js'; | |
| export * from './select/index.js'; | |
| export * from './switch/index.js'; |
🤖 Prompt for AI Agents
In unraid-ui/src/components/form/index.ts lines 1-7, the module re-exports use
bare specifiers; update each export to include the .js extension to match the
project's ESM runtime resolution rules (e.g., export * from './combobox.js';
export * from './input.js'; etc.), ensuring all six re-export lines reference
the corresponding .js files.
|
🚀 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. |
When generating an API key with the black theme active, the key was displayed with white text on a white background. This was caused by hardcoded light-theme classes (
bg-gray-50andborder-gray-200) on theInputcomponent that displays the key.This change removes the hardcoded background and border color classes, allowing the themed styles to be applied correctly.
Summary by CodeRabbit