feat: Custom colors in info panel #2799
Conversation
|
🚀 Thanks for opening this pull request! ❌ Please fill out all fields with a placeholder |
Uffizzi Ephemeral Environment
|
📝 WalkthroughWalkthroughThe changes introduce dynamic styling for the AggregationPanel component by allowing custom font and background colors to be set per segment title, key-value item, and button. This is achieved by passing inline CSS custom properties and updating SCSS to use these variables with fallbacks to existing color values. Changes
Sequence Diagram(s)sequenceDiagram
participant ParentComponent
participant AggregationPanel
participant Segment
participant KeyValueElement
participant ButtonElement
ParentComponent->>AggregationPanel: Passes segments/items with color properties
AggregationPanel->>Segment: Renders segment with title, passing titleFontColor/titleBackgroundColor
Segment->>AggregationPanel: Renders title with inline CSS variables
AggregationPanel->>KeyValueElement: Renders item with fontColor/backgroundColor
KeyValueElement->>KeyValueElement: Applies inline CSS variables for styling
AggregationPanel->>ButtonElement: Renders button with fontColor/backgroundColor/onhoverBackgroundColor
ButtonElement->>ButtonElement: Applies inline CSS variables for styling
Assessment against linked issues
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (4)
src/components/AggregationPanel/AggregationPanel.js (2)
50-50: Sanitize segment color properties.
Ensure thatsegment.titleFontColorandsegment.titleBackgroundColorare validated and conform to acceptable CSS color formats to avoid invalid styles or potential injection.
49-49: Avoid using array index as React key.
Usingindexforkeycan lead to rendering issues when segments reorder. If each segment has a unique identifier (e.g.,segment.id), prefer that to maintain stable keys.src/components/AggregationPanel/AggregationPanelComponents.js (2)
13-13: Validate and sanitize item color props.
Ensureitem.fontColoranditem.backgroundColorare validated against valid CSS color formats to prevent invalid or malicious values.
92-92: Consider memoizing style object and sanitizing hover color.
To avoid recreating the style object on each render and to validateitem.onhoverBackgroundColor, you could memoize the style viauseMemoand ensure the hover color prop is sanitized.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/AggregationPanel/AggregationPanel.js(1 hunks)src/components/AggregationPanel/AggregationPanel.scss(4 hunks)src/components/AggregationPanel/AggregationPanelComponents.js(2 hunks)
🔇 Additional comments (6)
src/components/AggregationPanel/AggregationPanel.js (1)
50-50: Inline CSS variables applied correctly.
Thestyleprop properly injects--fontColorand--backgroundColorfrom the segment object, enabling dynamic theming with sensible fallbacks.src/components/AggregationPanel/AggregationPanel.scss (3)
8-9: CSS variables for heading integrated correctly.
The.headingrules now usevar(--backgroundColor, $blue)andvar(--fontColor, $white)to allow dynamic overrides while preserving legacy fallbacks.
27-28: CSS variables for key-value items integrated correctly.
The.keyValueblock leveragesvar(--backgroundColor, inherit)andvar(--fontColor, inherit)to support item-level theming without altering default styles.
71-75: CSS variables for button styling integrated correctly.
Button background, font color, and hover background now use--backgroundColor,--fontColor, and--onhoverBackgroundColorwith proper fallbacks, aligning SCSS with component-level theming.Also applies to: 83-83
src/components/AggregationPanel/AggregationPanelComponents.js (2)
13-13: Inline CSS variables in KeyValueElement applied correctly.
Thestyleprop on the.keyValuecontainer correctly injects--fontColorand--backgroundColorfromitem, enabling dynamic theming.
92-92: Inline CSS variables in ButtonElement applied correctly.
The button element now uses--fontColor,--backgroundColor, and--onhoverBackgroundColorfromitem, enabling consistent dynamic styling.
|
Hey @mtrezza, I have implemented this enhancement. Now, if we pass the fontColor and backgroundColor as described, the panel reflects them correctly. This works for the title, key-value items, and the button. |
|
@vardhan0604 Thanks for opening this PR. This feature has already been added in #2788, but the PR was not linked to the issue. The feature is also documented in the README. Unfortunately it can always happen that an issue is open but obsolete. I recommend to check the current state of the dashboard, though code, README and posting in the issue itself before working on it. |
|
@coderabbitai you should have checked the previous PRs and README of the repository to realize that this feature has been implemented recently, and then adviced the PR author. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 23562 @mtrezza You're absolutely right, and I apologize for the oversight. I should have checked for recently merged PRs and the README to identify that this feature was already implemented. PR #2788 (merged on May 6, 2025) added support for custom CSS styling in the info panel through properties like Thank you for the feedback. I'll be more thorough in checking for recently implemented features before providing walkthroughs on future PRs. 🐰 Rabbit looks apologetic with droopy ears |
New Pull Request Checklist
Issue Description
fixes #2644
Closes: FILL_THIS_OUT
Approach
Summary by CodeRabbit
New Features
Style