Update theme color tokens#434
Conversation
| @@ -23,15 +23,21 @@ const defaultTheme = { | |||
| 'brand-text-950': '#172554', | |||
There was a problem hiding this comment.
What: Verify that all color tokens used in the UI have appropriate contrast ratios, especially for new tokens like 'background-error' and 'text-error'.
Why: Lack of proper contrast can lead to accessibility issues for users with visual impairments. Ensuring that color contrasts meet accessibility standards is crucial for an inclusive design.
How: Use a tool or get a reviewer to check the contrast ratios of all new color tokens against the expected background colors. Adjust any color values that do not meet the recommended WCAG guidelines.
| @@ -42,30 +48,39 @@ const defaultTheme = { | |||
| 'field-background-disabled': '#F9FAFB', | |||
There was a problem hiding this comment.
What: Consider defining a clear and consistent naming convention for tokens. Some new tokens introduced (like 'background-error', 'background-success') differ slightly from existing naming styles.
Why: A consistent naming convention enhances readability and makes maintenance easier. It helps other developers to navigate and use the codebase more effectively.
How: Adhere to a specific convention such as BEM or a similar methodology for naming. Ensure that all tokens fit this convention.
| @@ -97,15 +112,17 @@ const defaultTheme = { | |||
| 'button-secondary': '#1F2937', | |||
| 'button-secondary-hover': '#374151', | |||
There was a problem hiding this comment.
What: Be cautious with hardcoded color values and ensure they align with design guidelines from Figma and do not introduce discrepancies.
Why: Colors should be driven by design specifications to maintain visual consistency and brand alignment. Mistakes in color representation can lead to inconsistency in the UI.
How: Cross-reference the updated colors with the design specifications in Figma and consider documenting color usage and reasoning within comments or external documentation.
| @@ -114,19 +131,20 @@ const defaultTheme = { | |||
| 'focus-error-border': '#FECACA', | |||
There was a problem hiding this comment.
What: Evaluate the performance implications of loading large theme configuration files. This change adds several colors which might increase load times slightly.
Why: Though these changes are likely not impactful by themselves, inefficient handling of such assets could affect the overall performance of applications that rely heavily on theme configurations.
How: Consider lazy loading styles or breaking down larger configuration files if performance metrics indicate a significant load time impact.
f004a85 to
97f736e
Compare
Summary
src/theme/default-config.jsWhat Changed
background-error,background-tertiary,border-brand,text-brand,button-active,focus-ring,badge-background-brand,tab-background-secondary, and table background tokensborder-interactive,focus, andtext-inversein place so existing usages continue to workValidation
node --check src/theme/default-config.js