-
Notifications
You must be signed in to change notification settings - Fork 5.1k
[Jetcaster] Reverted colors #1583
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @AjeshRPai, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request focuses on updating the color palette within the Color.kt
file to enhance the design system's light and dark themes. The changes aim to improve visual consistency and accessibility across various contrast levels by adjusting primary, secondary, tertiary, and surface element colors.
Highlights
- Light Theme Color Updates: Modified primary, secondary, tertiary, and surface-related colors for the light theme to improve contrast and alignment with the design system. This includes changes to
onPrimaryLight
,primaryContainerLight
,secondaryLight
,tertiaryLight
,errorLight
, and varioussurfaceLight
variants. - Medium Contrast Light Theme Refinements: Adjusted medium-contrast colors for primary, secondary, tertiary, and surface elements in the light theme to enhance readability and balance. Specific changes were made to
primaryLightMediumContrast
,secondaryLightMediumContrast
,tertiaryLightMediumContrast
, andsurfaceLightMediumContrast
variants. - Dark Theme Color Revisions: Updated primary, secondary, tertiary, and surface-related colors for the dark theme to improve contrast and consistency. This involved revisions to
primaryDark
,onPrimaryDark
,secondaryDark
,tertiaryDark
,surfaceDark
, andonSurfaceDark
. - High Contrast Dark Theme Enhancements: Optimized high-contrast colors for primary, secondary, tertiary, and surface elements in the dark theme to improve accessibility. This includes updates to
primaryDarkHighContrast
,secondaryDarkHighContrast
, andtertiaryDarkHighContrast
.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request updates the color palette for the Jetcaster design system. While the goal is to improve visual consistency and accessibility, my review has identified several critical and high-severity accessibility issues related to color contrast. Multiple color pairs have contrast ratios that fall well below WCAG AA standards, which would make UI elements unreadable. This is especially concerning for the 'High Contrast' themes, where some colors do not provide adequate contrast. I've left detailed comments on each of these issues. Please address them to ensure the app is accessible to all users.
val primaryDark = Color(0xFFFF792C) | ||
val onPrimaryDark = Color(0xFFFFECE0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The contrast ratio between onPrimaryDark
(#FFFFECE0
) and primaryDark
(#FFFF792C
) is approximately 1.55:1, which is far below the minimum WCAG AA requirement of 4.5:1 for normal text (or 3:1 for large text). This will make text or icons using these colors unreadable. Please correct the value of onPrimaryDark
to ensure sufficient contrast.
val secondaryContainerDarkMediumContrast = Color(0xFFB18B3D) | ||
val onSecondaryContainerDarkMediumContrast = Color(0xFF664800) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
val tertiaryContainerDarkMediumContrast = Color(0xFFC3824A) | ||
val onTertiaryContainerDarkMediumContrast = Color(0xFF663600) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The contrast ratio between onTertiaryContainerDarkMediumContrast
(#FF663600
) and tertiaryContainerDarkMediumContrast
(#FFC3824A
) is approximately 2.15:1, which does not meet minimum accessibility requirements. Note that onTertiaryContainerDarkMediumContrast
has the same value as onTertiaryDarkMediumContrast
, which is likely an error. Please adjust the colors to provide sufficient contrast.
val tertiaryContainerLightMediumContrast = Color(0xFF8F606C) | ||
val onTertiaryContainerLightMediumContrast = Color(0xFFFFFFFF) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The contrast ratio between onTertiaryContainerLightMediumContrast
(#FFFFFF
) and tertiaryContainerLightMediumContrast
(#8F606C
) is approximately 4.2:1. This is below the WCAG AA requirement of 4.5:1 for normal text. While it passes for large text, it could be an accessibility issue for smaller UI elements or text. Please verify if this is the intended contrast level or if a color with higher contrast should be used.
val primaryContainerDarkHighContrast = Color(0xFFFFBC9C) | ||
val onPrimaryContainerDarkHighContrast = Color(0xFF662500) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The contrast ratio between onPrimaryContainerDarkHighContrast
(#FF662500
) and primaryContainerDarkHighContrast
(#FFFFBC9C
) is approximately 3.03:1. For a 'High Contrast' theme, this is very low. It does not meet WCAG AA standards for normal text (4.5:1), and barely meets it for large text (3:1). High contrast themes should aim for much higher contrast ratios, typically 7:1 or more.
val secondaryContainerDarkHighContrast = Color(0xFFF0C470) | ||
val onSecondaryContainerDarkHighContrast = Color(0xFF664800) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The contrast ratio between onSecondaryContainerDarkHighContrast
(#FF664800
) and secondaryContainerDarkHighContrast
(#FFF0C470
) is approximately 3.01:1. This is insufficient for a 'High Contrast' theme and fails to meet WCAG AA guidelines for normal text. Please select colors that provide a significantly higher contrast ratio.
val secondaryLight = Color(0xFFFFBD1D) | ||
val onSecondaryLight = Color(0xFF664800) | ||
val secondaryContainerLight = Color(0xFFFFEECC) | ||
val onSecondaryContainerLight = Color(0xFF664800) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The value for onSecondaryContainerLight
is the same as onSecondaryLight
(line 25). In other color groups (like primary and tertiary) and in the dark theme, the on...
and on...Container
colors are distinct. Is this intentional? If this is a copy-paste error, it should be corrected to maintain consistency and avoid potential display issues.
val secondaryContainerDark = Color(0xFFF0C470) | ||
val onSecondaryContainerDark = Color(0xFF704700) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The contrast ratio between onSecondaryContainerDark
(#FF704700
) and secondaryContainerDark
(#FFF0C470
) is approximately 3.01:1. This meets the minimum WCAG AA requirement for large text (3:1), but it's on the very edge and fails for normal-sized text (which requires 4.5:1). Consider slightly adjusting one of the colors to improve readability, especially if it's used for smaller text elements.
val tertiaryDark = Color(0xFFFF8F0F) | ||
val onTertiaryDark = Color(0xFF663600) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding @MagicalMeghan for reviewing this when it's ready, as she's most recently touched these files |
This pull request updates the color palette in the
Color.kt
file to improve the design system's light and dark themes. The changes include adjustments to color values for primary, secondary, tertiary, and surface elements across different contrast levels. These updates aim to enhance visual consistency and accessibility.Light Theme Updates:
onPrimaryLight
,primaryContainerLight
, andonSecondaryContainerLight
, to provide better contrast and alignment with the design system.surfaceDimLight
,surfaceBrightLight
, andsurfaceContainerLight
to create a more cohesive palette.Medium Contrast Updates:
primaryLightMediumContrast
andonSecondaryContainerLightMediumContrast
, to improve readability and accessibility. [1] [2]surfaceContainerLightMediumContrast
andsurfaceBrightLightMediumContrast
for a more balanced appearance.Dark Theme Updates:
primaryDark
,onPrimaryDark
, andtertiaryContainerDark
, to enhance contrast and usability.surfaceDark
andonSurfaceDark
for improved consistency in the dark theme.High Contrast Updates:
primaryDarkHighContrast
andonTertiaryContainerDarkHighContrast
, to optimize accessibility for users requiring higher contrast.surfaceContainerDarkHighContrast
for better visual hierarchy.