fix: no environment alignment#7580
Conversation
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughUpdated the "No Environment" dropdown item to conditionally apply the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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.
🧹 Nitpick comments (1)
packages/bruno-app/src/components/Environments/EnvironmentSelector/EnvironmentListContent/index.js (1)
20-26: Logic and alignment fix looks correct.The conditional
dropdown-item-activeclass correctly uses the falsy check againstactiveEnvironmentUid, which aligns with the Redux slice setting it tonullwhen no environment is selected. The spacing span maintains visual alignment withColorBadgein other environment items.One minor note: per coding guidelines,
italicandopacity-50are visual styles rather than layout. These would typically belong in styled components. Consider moving them to the existing styled wrapper if consistency is a concern, but this is a low-priority nitpick given the small scope.,
♻️ Optional: Move visual styles to styled component
- <span className="italic opacity-50">No Environment</span> + <span className="no-environment-label">No Environment</span>Then in the styled component wrapper, add:
.no-environment-label { font-style: italic; opacity: 0.5; }As per coding guidelines: "Tailwind classes are used specifically for layout-based styles" and "Styled Component CSS might also change layout but Tailwind classes shouldn't define colors."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/Environments/EnvironmentSelector/EnvironmentListContent/index.js` around lines 20 - 26, The current JSX uses Tailwind utility classes "italic" and "opacity-50" inline on the span for the "No Environment" label; move these visual styles into the styled wrapper to follow the guideline that Tailwind be used for layout only. Update the EnvironmentListContent styled component (the wrapper that renders the .no-environment item) to add a class or rule (e.g., .no-environment-label) with font-style: italic and opacity: 0.5 and replace the inline "italic opacity-50" on the span in the element that reads "No Environment" (the div using activeEnvironmentUid and onEnvironmentSelect(null)) to use that class instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@packages/bruno-app/src/components/Environments/EnvironmentSelector/EnvironmentListContent/index.js`:
- Around line 20-26: The current JSX uses Tailwind utility classes "italic" and
"opacity-50" inline on the span for the "No Environment" label; move these
visual styles into the styled wrapper to follow the guideline that Tailwind be
used for layout only. Update the EnvironmentListContent styled component (the
wrapper that renders the .no-environment item) to add a class or rule (e.g.,
.no-environment-label) with font-style: italic and opacity: 0.5 and replace the
inline "italic opacity-50" on the span in the element that reads "No
Environment" (the div using activeEnvironmentUid and onEnvironmentSelect(null))
to use that class instead.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ddfbabd9-fb07-4b3b-9b8f-1fdfe473c303
📒 Files selected for processing (1)
packages/bruno-app/src/components/Environments/EnvironmentSelector/EnvironmentListContent/index.js
sid-bruno
left a comment
There was a problem hiding this comment.
Approved, waiting on JIRA for new release version tag
Description
JIRA
Fix no environment alignment in environment selector
Contribution Checklist:
Summary by CodeRabbit
Bug Fixes
Style