Skip to content

Conversation

@elibosley
Copy link
Member

@elibosley elibosley commented Aug 13, 2025

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.

Summary by CodeRabbit

  • Refactor
    • Consolidated common UI and form components behind single import entry points, simplifying usage and making components easier to discover. No functional changes.
  • Style
    • Refined API key input appearance for a cleaner, less intrusive look while preserving readability and existing controls (copy, visibility toggle, read-only).
  • Chores
    • Streamlined component import paths to improve developer experience and consistency across the app. No user-facing behavior changes.

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.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Aug 13, 2025

Walkthrough

Adds 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

Cohort / File(s) Summary of Changes
UI Barrel Exports — Common
unraid-ui/src/components/common/index.ts
New barrel file re-exporting: accordion, badge, button, dialog, dropdown-menu, loading, popover, scroll-area, sheet, stepper, tabs, toast, tooltip.
UI Barrel Exports — Form
unraid-ui/src/components/form/index.ts
New barrel file re-exporting: combobox, input, label, lightswitch, number, select, switch.
Web UI Style Tweak — ApiKey
web/components/ApiKey/ApiKeyManager.vue
Removed bg and border classes from API key input; no functional changes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

Suggested reviewers

  • mdatelle
  • zackspear
  • pujitm

Poem

In the garden of imports, I hop with glee,
Two barrels bloom, as tidy as can be.
A key field sheds its border coat—how light!
I twitch my nose at cleaner sight.
Thump-thump—ship it swift, by moonlit night. 🐇✨

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/api-key-theme-issue

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@claude
Copy link

claude bot commented Aug 13, 2025

Claude finished @elibosley's task —— View job


Code Review - API Key Theme Fix

Todo List:

  • Read the changed file to understand the specific diff
  • Analyze the diff for critical issues (bugs, security, breaking changes, data loss)
  • Provide review feedback

Review of Changes

Analyzed the exact diff in web/components/ApiKey/ApiKeyManager.vue:172:

- 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 (bg-gray-50 and border-gray-200) from the Input component, which was causing the white-on-white text issue in dark theme. This is a safe styling fix that allows the Input component's default theming to take effect properly.

The modification:

  • Does not introduce any logic errors or security vulnerabilities
  • Does not break any API contracts or function signatures
  • Does not pose any data loss risks
  • Appropriately fixes the reported theming issue

@github-actions
Copy link
Contributor

🚀 Storybook has been deployed to staging: https://unraid-ui-storybook-staging.unraid-workers.workers.dev

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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-700 pairing 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

📥 Commits

Reviewing files that changed from the base of the PR and between 27dbfde and 93d0ea9.

📒 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.vue
  • unraid-ui/src/components/form/index.ts
  • unraid-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.ts
  • unraid-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.

Comment on lines 1 to 13
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';
Copy link
Contributor

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.

Suggested change
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.

Comment on lines 1 to 7
export * from './combobox';
export * from './input';
export * from './label';
export * from './lightswitch';
export * from './number';
export * from './select';
export * from './switch';
Copy link
Contributor

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.

Suggested change
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.

@pujitm pujitm merged commit b321687 into main Aug 13, 2025
11 checks passed
@pujitm pujitm deleted the fix/api-key-theme-issue branch August 13, 2025 18:43
@github-actions
Copy link
Contributor

🚀 Storybook has been deployed to staging: https://unraid-ui-storybook-staging.unraid-workers.workers.dev

@github-actions
Copy link
Contributor

This plugin has been deployed to Cloudflare R2 and is available for testing.
Download it at this URL:

https://preview.dl.unraid.net/unraid-api/tag/PR1584/dynamix.unraid.net.plg

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants