[WEB-2895] fix: space app priority icons#6784
Conversation
vamsikrishnamathala
requested review from
anmolsinghbhatia and
prateekshourya29
WalkthroughThis PR updates the priority component in Changes
Sequence Diagram(s)sequenceDiagram
participant Component as PriorityComponent
participant CSSUtil as cn Function
participant Icon as Icon Renderer
Component->>CSSUtil: Evaluate priority and shouldShowName
alt Priority provided
Component->>Icon: Render PriorityIcon with adjusted props
else
Component->>Icon: Render SignalHigh icon
end
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
space/core/components/issues/issue-layouts/properties/priority.tsx (2)
34-44: Improved conditional class application.Good use of the cn utility to conditionally apply classes based on priority and display mode. However, there appears to be redundant styling for the urgent priority.
The condition on line 42 (
"bg-red-600/10 border-red-600": priority === "urgent" && shouldShowName) seems unnecessary since these styles are already included inpriorityClasses.urgentwhich is applied on line 37.
46-62: Enhanced icon rendering with responsive adjustments.The conditional rendering between PriorityIcon and SignalHigh is well implemented. The detailed positioning adjustments ensure proper alignment across different priority levels.
There's a comment on line 57-58 saying "highlight the icon if priority is urgent" but no corresponding style is applied. Either add the highlighting style or remove the comment to maintain code clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
space/core/components/issues/issue-layouts/properties/priority.tsx(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (3)
space/core/components/issues/issue-layouts/properties/priority.tsx (3)
3-3: Added new SignalHigh icon for the default case.Good addition of the SignalHigh icon from lucide-react to represent the default state when no priority is set.
7-7: Appropriate imports for enhanced functionality.The additions of PriorityIcon and the cn utility function are good choices for improving icon rendering and conditional class composition.
Also applies to: 9-9
22-28: Well-structured priority styling system.The priorityClasses object provides a clear, centralized mapping of priority levels to their visual styling. This approach makes it easier to maintain consistent styling across the application.
henit-chobisa
changed the title
henit-chobisa
changed the title
henit-chobisa
changed the title
2 participants
Description
This update fixes the space app icons display. Making it consistent with the web.
Type of Change
Screenshots and Media (if applicable)
Test Scenarios
References
WEB-2895
Summary by CodeRabbit