Repository navigation
Fix treemap tooltip text color across light and dark themes - #10035
prateek601 wants to merge 5 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request fixes the treemap tooltip text color across light and dark themes by applying the correct theme color to the text span. It also updates the test wrappers to support dark theme configuration and adds tests to verify the tooltip text color in both themes. The review feedback suggests extracting a duplicated theme configuration block in the test wrappers into a helper function to adhere to the DRY principle.
| theme: themeFor( | ||
| isDarkTheme: false, | ||
| isDarkTheme: isDarkTheme, | ||
| ideTheme: IdeTheme(), | ||
| theme: ThemeData(useMaterial3: true, colorScheme: lightColorScheme), | ||
| theme: ThemeData( | ||
| useMaterial3: true, | ||
| colorScheme: isDarkTheme ? darkColorScheme : lightColorScheme, | ||
| ), | ||
| ), |
There was a problem hiding this comment.
[CONCERN] The theme configuration block is duplicated between wrap and wrapSimple. Consider extracting this into a private helper function to adhere to the DRY principle and improve maintainability.
You can define the helper function at the top level of the file:
ThemeData _testTheme({required bool isDarkTheme}) {
return themeFor(
isDarkTheme: isDarkTheme,
ideTheme: IdeTheme(),
theme: ThemeData(
useMaterial3: true,
colorScheme: isDarkTheme ? darkColorScheme : lightColorScheme,
),
);
} theme: _testTheme(isDarkTheme: isDarkTheme),References
- Use shared primitives and components rather than recreating them from scratch. Flag duplicated code blocks and suggest extracting them. (link)
… a helper function to adhere to the DRY principle
Issue 1 : Currently tooltip text color for the nodes under the title bar shows white color text on the white background tooltip in dark mode on the Diff tab of App Size Screen. Rendering white text on white background of tooltip is not clearly visible as can be seen in screenshot.
Issue 2: Also in the light theme the tooltip renders greyish background and currently black color text is shown on the tooltip on the Analysis tab on the App Size screen for the nodes under the title bar (screenshot attached for reference) which is less visible as well. This is also being fixed in this PR.
This PR fixes both the above mentioned issues. Now the DevToolsToolTip richMessage parameter explicitly passes the color parameter in the displayTextSpan method which shows the appropriate tooltip color in light and dark theme modes.
Earlier since color was not explicitly passed, the tooltip text color was defaulted to black in Analysis tab and white in Diff tab
Before

in dark theme --> Diff tab --> tooltip text color is not visible
After

tooltip text color is visible
Before

in light theme --> Analysis tab --> black tooltip text color is not giving proper contrast (less visible)
After

tooltip text color is clearly visible
Pre-launch Checklist
General checklist
///).Issues checklist
contributions-welcomeorgood-first-issuelabel.contributions-welcomeorgood-first-issuelabel. I understand this means my PR might take longer to be reviewed.Tests checklist
AI-tooling checklist
Feature-change checklist
release-notes-not-requiredlabel or left a comment requesting the label be added.packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md.If you need help, consider asking for help on Discord.