feat: improve cloud button discoverability and header icon visibility#654
Merged
emranemran merged 1 commit intomainfrom Mar 11, 2026
Merged
feat: improve cloud button discoverability and header icon visibility#654emranemran merged 1 commit intomainfrom
emranemran merged 1 commit intomainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdjusted Header UI text and opacity for cloud-status states and increased icon button opacity; updated an end-to-end selector to include the remote inference button. No exported API or runtime logic signature changes. (≈36 words) Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/components/Header.tsx (1)
139-166:⚠️ Potential issue | 🟡 MinorHandle
cloudDisabledbefore showing an enable CTA.If
cloudDisabledis true, this still renders “Enable Remote Inference” and the normal disconnected styling, which makes an intentionally unavailable feature look merely disconnected. That’s misleading for the exact state this copy is trying to clarify.💡 Suggested adjustment
className={`hover:opacity-80 transition-opacity h-8 gap-1.5 px-2 ${ - isConnected + cloudDisabled + ? "text-muted-foreground opacity-50" + : isConnected ? "text-green-500 opacity-100" : isConnecting ? "text-amber-400 opacity-100" : "text-muted-foreground opacity-80" }`} title={ - isConnected + cloudDisabled + ? "Remote inference unavailable" + : isConnected ? "Cloud connected" : isConnecting ? "Connecting to cloud..." : "Enable remote inference" } ... {isConnected ? "Connected" - : isConnecting - ? "Connecting..." - : "Enable Remote Inference"} + : cloudDisabled + ? "Unavailable" + : isConnecting + ? "Connecting..." + : "Enable Remote Inference"}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/Header.tsx` around lines 139 - 166, The header currently treats cloudDisabled the same as a normal disconnected state and still renders the "Enable Remote Inference" CTA; update the rendering logic in Header.tsx where isConnected/isConnecting are used (the JSX block rendering the Cloud/CloudOff icon, title attribute, className and the span text) to check cloudDisabled first and render a distinct disabled state (different text like "Remote Inference Disabled", muted styling and a disabled icon/tooltip) instead of the enable CTA; ensure the checks use the existing cloudDisabled, isConnected and isConnecting symbols so the disabled branch short-circuits the disconnected branch for title, className, icon and span text.
🧹 Nitpick comments (1)
frontend/src/components/Header.tsx (1)
169-183: The opacity transition is now a no-op on these two buttons.With
opacity-80as the base state andhover:opacity-80on hover, Line 173 and Line 182 no longer change opacity at all. If the stronger default visibility is intentional, either bump hover toopacity-100or drop the opacity transition classes.♻️ Possible cleanup
- className="hover:opacity-80 transition-opacity text-muted-foreground opacity-80 h-8 w-8" + className="hover:opacity-100 transition-opacity text-muted-foreground opacity-80 h-8 w-8" ... - className="hover:opacity-80 transition-opacity text-muted-foreground opacity-80 h-8 w-8" + className="hover:opacity-100 transition-opacity text-muted-foreground opacity-80 h-8 w-8"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/Header.tsx` around lines 169 - 183, Both Button components rendering the Plugins and Settings icons (the ones using onClick handlers setPluginsOpen and setSettingsOpen) have a no-op opacity transition because they set opacity-80 as both the base and hover state; change the hover class to a stronger value (e.g., hover:opacity-100) to make the transition visible, or remove the hover:opacity-80/transition-opacity classes if you intend static reduced opacity. Update the Button className on the Plug and Settings Buttons accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@frontend/src/components/Header.tsx`:
- Around line 139-166: The header currently treats cloudDisabled the same as a
normal disconnected state and still renders the "Enable Remote Inference" CTA;
update the rendering logic in Header.tsx where isConnected/isConnecting are used
(the JSX block rendering the Cloud/CloudOff icon, title attribute, className and
the span text) to check cloudDisabled first and render a distinct disabled state
(different text like "Remote Inference Disabled", muted styling and a disabled
icon/tooltip) instead of the enable CTA; ensure the checks use the existing
cloudDisabled, isConnected and isConnecting symbols so the disabled branch
short-circuits the disconnected branch for title, className, icon and span text.
---
Nitpick comments:
In `@frontend/src/components/Header.tsx`:
- Around line 169-183: Both Button components rendering the Plugins and Settings
icons (the ones using onClick handlers setPluginsOpen and setSettingsOpen) have
a no-op opacity transition because they set opacity-80 as both the base and
hover state; change the hover class to a stronger value (e.g.,
hover:opacity-100) to make the transition visible, or remove the
hover:opacity-80/transition-opacity classes if you intend static reduced
opacity. Update the Button className on the Plug and Settings Buttons
accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: eed58af2-10d3-4ded-a6ad-a415a2b4cb8f
📒 Files selected for processing (1)
frontend/src/components/Header.tsx
Contributor
🚀 fal.ai Preview Deployment
TestingConnect to this preview deployment by running this on your branch: 🧪 E2E tests will run automatically against this deployment. |
Contributor
✅ E2E Tests passed
Test ArtifactsCheck the workflow run for screenshots. |
Change disconnected state text from "Disconnected" to "Enable Remote Inference" so new users understand the feature is available. Bump opacity on all three header icons (cloud, plugins, settings) for better visibility. Update e2e test selector to match new title. Signed-off-by: emranemran <[email protected]>
emranemran
force-pushed
the
emran/cloud-ux-improvement
branch
from
ac2e76a to
9f7cb3b
Compare
thomshutt
reviewed
Mar 10, 2026
| size="icon" | ||
| onClick={() => setPluginsOpen(true)} | ||
| className="hover:opacity-80 transition-opacity text-muted-foreground opacity-60 h-8 w-8" | ||
| className="hover:opacity-80 transition-opacity text-muted-foreground opacity-80 h-8 w-8" |
Contributor
There was a problem hiding this comment.
Doesn't this remove the hover effect since it now has the same opacity as the regular one?
thomshutt
approved these changes
Mar 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Change disconnected state text from "Disconnected" to "Enable Remote Inference" so new users understand the feature is available. Bump opacity on all three header icons (cloud, plugins, settings) for better visibility.
Summary by CodeRabbit
Style
Tests