update(tooltip): ajust tooltip color #187
Conversation
WalkthroughThis pull request updates the tooltip formatting for various chart components. The modifications involve importing a new Changes
Sequence Diagram(s)sequenceDiagram
participant CC as Chart Component
participant TF as tooltipFormatter
participant CT as chartToken
participant DS as defendXSS
CC->>TF: Call tooltipFormatter()
TF->>CT: Import & extract color tokens
TF->>DS: Sanitize values via defendXSS
DS-->>TF: Return sanitized content
TF->>CC: Return formatted HTML tooltip
Possibly related PRs
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
src/components/BubbleChart/handleOptipn.jsOops! Something went wrong! :( ESLint: 8.56.0 ReferenceError: require is not defined in ES module scope, you can use import instead src/components/TreeMapChart/chartToken.jsOops! Something went wrong! :( ESLint: 8.56.0 ReferenceError: require is not defined in ES module scope, you can use import instead src/components/ScatterChart/handleOptipn.jsOops! Something went wrong! :( ESLint: 8.56.0 ReferenceError: require is not defined in ES module scope, you can use import instead
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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: 1
🧹 Nitpick comments (1)
src/components/BubbleChart/handleOptipn.js (1)
12-12: Consider centralizing the chartToken implementationCurrently, you're importing chartToken from local files in each chart component directory, which could lead to code duplication and maintenance issues. Consider creating a centralized token system that can be imported by all chart components.
For example, create a single token file at
src/tokens/chartToken.jsthat all components can import:// Example implementation // src/tokens/chartToken.js export default { tipSeriesNameColor: '#333333', tipNameColor: '#666666', tipValueColor: '#000000', // other token values... };And then update imports in all chart components:
-import chartToken from './chartToken'; +import chartToken from '../../tokens/chartToken';Also applies to: 13-13, 16-16
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/components/BubbleChart/handleOptipn.js(1 hunks)src/components/PolarBarChart/handleOption.js(1 hunks)src/components/ScatterChart/handleOptipn.js(2 hunks)src/components/TreeMapChart/chartToken.js(1 hunks)src/components/TreeMapChart/handleOptipn.js(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/components/TreeMapChart/chartToken.js
🔇 Additional comments (15)
src/components/TreeMapChart/handleOptipn.js (4)
12-12: Consistent token import added for stylingThe import of
chartTokenaligns with the PR objective to adjust tooltip colors across chart components.
22-22: Proper token destructuring for color valuesDestructuring the color variables from
chartTokenfollows modern JavaScript practices and improves code readability.
25-32: Tooltip styling uses token-based colors for consistencyThe implementation correctly applies the token-based color system to the tooltip elements, creating a more consistent UI.
25-27:✅ Verification successful
Verify chartToken file exists in the directory
The implementation references a
./chartTokenmodule which is being imported, but I don't see the actual file in the PR. Make sure this file exists and contains the expected color token values.Also applies to: 30-31
🏁 Script executed:
#!/bin/bash # Check if the chartToken.js file exists in the TreeMapChart directory ls -la src/components/TreeMapChart/chartToken.js 2>/dev/null || echo "chartToken.js file is missing in TreeMapChart directory" # Check the contents of the file to verify it contains the required color tokens if [ -f src/components/TreeMapChart/chartToken.js ]; then echo "Content of chartToken.js:" cat src/components/TreeMapChart/chartToken.js fiLength of output: 965
Chart token file verified and appears as expected.
- The file
src/components/TreeMapChart/chartToken.jsexists and includes the required references to color tokens.- Both code locations (lines 25-27 and 30-31) correctly leverage the token from the verified file.
src/components/ScatterChart/handleOptipn.js (4)
12-12: Consistent token import added for stylingThe import of
chartTokenaligns with the PR objective to adjust tooltip colors across chart components.
24-24: Proper token destructuring for color valuesDestructuring the color variables from
chartTokenfollows modern JavaScript practices and improves code readability.
27-29: Tooltip styling uses token-based colors for consistencyThe implementation correctly applies the token-based color system to the tooltip elements, creating a more consistent UI.
Also applies to: 32-33, 37-38
12-12:✅ Verification successful
Verify chartToken file exists in the directory
The implementation references a
./chartTokenmodule which is being imported, but I don't see the actual file in the PR. Make sure this file exists and contains the expected color token values.
🏁 Script executed:
#!/bin/bash # Check if the chartToken.js file exists in the ScatterChart directory ls -la src/components/ScatterChart/chartToken.js 2>/dev/null || echo "chartToken.js file is missing in ScatterChart directory" # Check the contents of the file to verify it contains the required color tokens if [ -f src/components/ScatterChart/chartToken.js ]; then echo "Content of chartToken.js:" cat src/components/ScatterChart/chartToken.js fiLength of output: 964
ChartToken file verified and correct
The
chartToken.jsfile in thesrc/components/ScatterChartdirectory exists and its content correctly exports the tokens usingTheme.getTokenByName(CHART_TYPE.SCATTER). No further changes are needed here.src/components/PolarBarChart/handleOption.js (3)
68-68: Proper token destructuring for color valuesDestructuring the color variables from
chartTokenfollows modern JavaScript practices and improves code readability.
73-76: Tooltip styling dimensions and colors updated for consistencyThe changes to the tooltip appearance (smaller dimensions, adjusted border radius, and token-based colors) create a more refined and consistent UI across chart components.
13-13:✅ Verification successful
Verify chartToken file exists in the directory
The implementation references a
chartTokenmodule which is already imported, but I don't see the actual file in the PR. Make sure this file exists and contains the expected color token values.
🏁 Script executed:
#!/bin/bash # Check if the chartToken.js file exists in the PolarBarChart directory ls -la src/components/PolarBarChart/chartToken.js 2>/dev/null || echo "chartToken.js file is missing in PolarBarChart directory" # Check the contents of the file to verify it contains the required color tokens if [ -f src/components/PolarBarChart/chartToken.js ]; then echo "Content of chartToken.js:" cat src/components/PolarBarChart/chartToken.js fiLength of output: 970
Verification Complete: chartToken File Valid
The file
src/components/PolarBarChart/chartToken.jshas been confirmed to exist and contains the expected implementation that imports the required token configuration and exports it correctly. No further action is required for this review comment.src/components/BubbleChart/handleOptipn.js (4)
12-12: Consistent token import added for stylingThe import of
chartTokenaligns with the PR objective to adjust tooltip colors across chart components.
16-16: Proper token destructuring for color valuesDestructuring the color variables from
chartTokenfollows modern JavaScript practices and improves code readability.
21-23: Tooltip styling uses token-based colors for consistency with other componentsThe implementation applies the token-based color system to the tooltip elements, which aligns with the changes in other chart components. The dimensions have also been adjusted for consistency (8px × 8px with 4px border radius).
Also applies to: 25-31
12-12:✅ Verification successful
Verify chartToken file exists in the directory
The implementation references a
./chartTokenmodule which is being imported, but I don't see the actual file in the PR. Make sure this file exists and contains the expected color token values.
🏁 Script executed:
#!/bin/bash # Check if the chartToken.js file exists in the BubbleChart directory ls -la src/components/BubbleChart/chartToken.js 2>/dev/null || echo "chartToken.js file is missing in BubbleChart directory" # Check the contents of the file to verify it contains the required color tokens if [ -f src/components/BubbleChart/chartToken.js ]; then echo "Content of chartToken.js:" cat src/components/BubbleChart/chartToken.js fiLength of output: 959
Chart token file verified in BubbleChart directory.
The file
src/components/BubbleChart/chartToken.jsexists and its content confirms that it correctly imports and retrieves the Bubble Chart token values. No further changes are required.
| <span style="display:inline-block;width:8px;height:8px; | ||
| margin-right:8px;border-radius:4px;border-style: solid;border-width:1px; |
There was a problem hiding this comment.
Fix missing closing quotes in style attributes
There are syntax errors in multiple places where the closing quotes are missing after the interpolated color values, which will cause the styles to not apply correctly.
Apply these fixes:
- <span style="color:${tipValueColor};>${defendXSS(name)}</span>
+ <span style="color:${tipValueColor};">${defendXSS(name)}</span>
- <span style="color:${tipValueColor};>${defendXSS(x)}</span>
+ <span style="color:${tipValueColor};">${defendXSS(x)}</span>
- <span style="color:${tipValueColor};>${defendXSS(y)}</span>
+ <span style="color:${tipValueColor};">${defendXSS(y)}</span>
- <span style="color:${tipValueColor};>${defendXSS(radius)}</span>
+ <span style="color:${tipValueColor};">${defendXSS(radius)}</span>Also applies to: 30-30, 34-35, 40-41, 46-47
Summary by CodeRabbit
New Features
Style