Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Alternating colors look nice in tables when embedded in IDE #8498

Merged
merged 7 commits into from
Nov 5, 2024

Conversation

elliette
Copy link
Member

@elliette elliette commented Oct 31, 2024

Fixes #8466
Work towards #8230

Alternating table row colors now match the IDE theme.

DevTools light theme:
Screenshot 2024-10-31 at 12 29 49 PM

DevTools dark theme:

Screenshot 2024-10-31 at 12 29 35 PM

Custom IDE themes:
Screenshot 2024-10-30 at 5 39 56 PM

Screenshot 2024-10-30 at 5 40 14 PM

primaryTextTheme: _devToolsTextTheme(theme, theme.primaryTextTheme),
textTheme: _devToolsTextTheme(theme, theme.textTheme),
colorScheme: theme.colorScheme.copyWith(
surface: backgroundColor,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we really want to override the surface color specified by the darkColorScheme and lightColorScheme definitions?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the darkColorScheme and lightColorScheme, the surface color is the same as this backgroundColor. This makes sure that the surface color for IDE themes matches the IDE's background color.

@elliette elliette requested a review from a team as a code owner October 31, 2024 19:32
@elliette elliette requested review from bkonyi and removed request for a team October 31, 2024 19:32
Comment on lines 608 to 610
Color darken({int percent = 10}) {
assert(1 <= percent && percent <= 100);
final p = percent / 100;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we make these methods take a value between 0.0 and 1.0 instead of having to / 100 to get the percentage? This is similar to flutter's opacity APIs for example.

@@ -599,3 +603,23 @@ ButtonStyle _generateButtonStyle({
}
return buttonStyle;
}

extension on Color {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see we already have these utilities in devtools_app: https://github.com/flutter/devtools/blob/master/packages/devtools_app/lib/src/shared/common_widgets.dart/#L944

Instead of creating new utility methods here, can we move the existing ones to devtools_app_shared: https://github.com/flutter/devtools/blob/master/packages/devtools_app_shared/lib/src/ui/ui_utils.dart/#L1

@@ -146,3 +146,33 @@ extension StringUtilities on String {
return true;
}
}

/// Utility extension methods to the [Color] class.
extension ColorExtension on Color {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you move this to ui/ui_utils instead? That way we don't have to cross reference utils/ imports from the ui/theme.dart file. We try to keep things separated in this package since it is published and also for BUILD rules in g3.

@elliette elliette merged commit 2184bba into flutter:master Nov 5, 2024
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Property details do not match IDE theme
2 participants