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

feat(theme): [button] add dark themes, modify other theme variables a… #2898

Merged
merged 1 commit into from
Feb 14, 2025

Conversation

MomoPoppy
Copy link
Collaborator

@MomoPoppy MomoPoppy commented Feb 13, 2025

…nd component level variables

PR

PR Checklist

Please check if your PR fulfills the following requirements:

  • The commit message follows our Commit Message Guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Summary by CodeRabbit

  • New Features

    • Introduced comprehensive dark theme support that refreshes the look and feel across the app.
  • Style Enhancements

    • Updated styling by migrating from fixed color values to dynamic, variable‐based theming.
    • Improved color consistency and visual cues for interactive elements such as buttons, dropdowns, modals, calendars, and tooltips for a more cohesive user experience.

Copy link

coderabbitai bot commented Feb 13, 2025

Walkthrough

The changes replace hardcoded color values with CSS variable references across a wide range of files. This update standardizes the theming across the project by updating component-specific styling (e.g., buttons, dropdowns, tooltips) and introducing new variables for dark mode support. Numerous LESS files now refer to newly defined variables, and a new dark theme configuration is added. No modifications affect public APIs or component logic.

Changes

File(s) Change Summary
examples/sites/src/views/layout/layout.vue Updated element styling: replaced hardcoded colors with CSS variable references for text and icon colors.
packages/theme/src/base/aurora-theme.less, packages/theme/src/base/dark-theme.less, packages/theme/src/base/index.less, packages/theme/src/base/old-theme.less, packages/theme/src/base/vars.less Introduced and updated several CSS custom properties; added new variables for dark mode support and standardized theme definitions; added new import for dark-theme configuration.
packages/theme/src/{autocomplete, badge, button-group, button, calendar-view, carousel-item, cascader{-node,}, crop, date-panel, date-table, dialog-box, drawer, dropdown(-menu, /dropdown), grid, guide, link, modal, month-table, nav-menu, notify, popover, progress, select-dropdown, slider, steps, switch, tabs, tag, time-panel, time-spinner, tooltip, tree-menu, upload-list, user-contact}/vars.less Updated component-specific LESS files by replacing static color values with CSS variable references across backgrounds, texts, icons, borders, and spacing; new sizing and spacing variables added for certain components.

Possibly related PRs

Suggested labels

enhancement

Suggested reviewers

  • zzcr

Poem

I’m a bunny with a hopping beat,
Watching colors change and themes complete.
Gone are the hardcoded hues of old,
New CSS variables shine bold and gold!
With every change, I jump and cheer—
A rabbit’s delight when styling’s clear!


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions github-actions bot added the enhancement New feature or request (功能增强) label Feb 13, 2025
Copy link

  • Walkthrough: This PR introduces dark themes for buttons and modifies various theme variables across multiple components. It includes changes to color variables, background colors, and component-level variables to support the new dark theme.
  • Changes:
    Files Summary
    examples/sites/src/views/layout/layout.vue Updated color and fill properties to use theme variables.
    packages/theme/src/autocomplete/vars.less, packages/theme/src/badge/vars.less Modified background and text color variables.
    packages/theme/src/base/aurora-theme.less, packages/theme/src/base/dark-theme.less Added and adjusted color variables for dark theme support.
    packages/theme/src/base/index.less Included dark theme import.
    packages/theme/src/button-group/vars.less, packages/theme/src/button/vars.less Updated button text and icon color variables.
    packages/theme/src/calendar-view/vars.less, packages/theme/src/carousel-item/vars.less Adjusted text color variables.
    packages/theme/src/cascader-node/vars.less, packages/theme/src/cascader/vars.less Changed background and icon color variables.
    packages/theme/src/crop/vars.less, packages/theme/src/date-panel/vars.less Updated background color variables.
    packages/theme/src/date-table/vars.less, packages/theme/src/dialog-box/vars.less Modified text and background color variables.
    packages/theme/src/drawer/vars.less, packages/theme/src/dropdown-menu/vars.less Adjusted background color variables.
    packages/theme/src/dropdown/vars.less, packages/theme/src/grid/vars.less Updated icon and background color variables.
    packages/theme/src/guide/vars.less, packages/theme/src/link/vars.less Changed text color variables.
    packages/theme/src/modal/vars.less, packages/theme/src/month-table/vars.less Modified background and text color variables.
    packages/theme/src/nav-menu/index.less, packages/theme/src/nav-menu/vars.less Updated background color variables.
    packages/theme/src/notify/vars.less, packages/theme/src/popover/vars.less Adjusted background color variables.
    packages/theme/src/progress/vars.less, packages/theme/src/select-dropdown/vars.less Modified text and background color variables.
    packages/theme/src/slider/vars.less, packages/theme/src/steps/vars.less Updated background and icon color variables.
    packages/theme/src/switch/vars.less, packages/theme/src/tabs/vars.less Changed text and background color variables.
    packages/theme/src/tag/vars.less, packages/theme/src/time-panel/vars.less Adjusted background color variables.
    packages/theme/src/time-spinner/vars.less, packages/theme/src/tooltip/vars.less Modified text and background color variables.
    packages/theme/src/tree-menu/vars.less, packages/theme/src/upload-list/vars.less Updated text and icon color variables.
    packages/theme/src/user-contact/vars.less Changed text color variables.

/* 1.1品牌色 */

/* 品牌主色 */
--tv-base-color-brand: #B3B3B3; // 主色黑/链接颜色

Choose a reason for hiding this comment

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

Ensure that the new color variables for the dark theme are correctly applied across all components to maintain consistency in the theme.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🔭 Outside diff range comments (1)
packages/theme/src/popover/vars.less (1)

1-1: 💡 Verification agent

🧩 Analysis chain

Consider standardizing fallback values across theme files.

Several components are transitioning from --tv-color-bg-secondary to --tv-color-bg-2, but the approach to fallback values is inconsistent. Some components include fallbacks (e.g., dropdown menu) while others don't (e.g., popover). Consider standardizing this approach across all theme files.

Let's check for consistency in fallback values across theme files:


🏁 Script executed:

#!/bin/bash
# Search for color variable declarations and their fallback patterns
rg --type=less "tv-color-bg-2.*" packages/theme/src/

Length of output: 1459


Standardize Fallback Values for Theme Colors

Several theme files are transitioning to use --tv-color-bg-2, but the fallback values are applied inconsistently. For example:

  • Files like autocomplete/vars.less, cascader/vars.less, date-panel/vars.less, dropdown-menu/vars.less, select-dropdown/vars.less, and time-panel/vars.less include an explicit fallback (e.g., var(--tv-color-bg-2, #ffffff)).
  • Other files such as popover/vars.less, modal/vars.less, notify/vars.less, and tooltip/vars.less reference --tv-color-bg-2 without any fallback.

Action:
Please review all theme files and standardize the fallback approach. This may involve updating files like packages/theme/src/popover/vars.less to use an explicit fallback value (e.g., var(--tv-color-bg-2, #ffffff)), ensuring consistent behavior across themes. Additionally, verify that any intentionally commented-out dark theme imports or related settings maintain architectural consistency.

🧹 Nitpick comments (6)
packages/theme/src/popover/vars.less (1)

15-15: Add fallback value for background color.

While the change to --tv-color-bg-2 aligns with the new theme system, consider adding a fallback value for better backward compatibility, similar to other components like the dropdown menu.

Apply this diff to add the fallback:

-  --tv-Popover-bg-color: var(--tv-color-bg-2);
+  --tv-Popover-bg-color: var(--tv-color-bg-2, #ffffff);
packages/theme/src/dialog-box/vars.less (1)

15-15: Add fallback color for backward compatibility.

While the change from --tv-color-bg-secondary to --tv-color-bg-3 aligns with the new system, consider adding a fallback color for better backward compatibility.

---tv-DialogBox-bg-color: var(--tv-color-bg-3);
+--tv-DialogBox-bg-color: var(--tv-color-bg-3, #ffffff);
packages/theme/src/base/aurora-theme.less (1)

271-282: LGTM! Comprehensive dark theme support added.

The new color variables provide extensive support for dark themes across different UI elements. Consider adding documentation to explain the usage of these variables in dark mode contexts.

Add JSDoc-style comments to document the intended usage of each dark theme variable, for example:

 // -- 暗色新增 --
+/**
+ * Text color for control elements in dark mode
+ * @example
+ * Used for left navigation text and primary icon buttons
+ */
 --tv-color-text-control: #333;

Also applies to: 295-298, 337-347

packages/theme/src/base/old-theme.less (1)

297-307: Consider consolidating theme files to reduce duplication.

The dark theme variables are duplicated across both aurora-theme.less and old-theme.less. Consider:

  1. Creating a shared dark theme module
  2. Extending it in both themes with theme-specific overrides

Example structure:

// dark-theme-base.less
@dark-theme-vars: {
  --tv-color-text-control: var(--tv-base-color-common-8);
  // ... other shared dark theme variables
}

// aurora-theme.less
@import 'dark-theme-base.less';
:root {
  @dark-theme-vars();
  // Aurora-specific overrides
}

// old-theme.less
@import 'dark-theme-base.less';
:root {
  @dark-theme-vars();
  // Old theme-specific overrides
}

Also applies to: 321-324, 365-374, 391-395

packages/theme/src/base/vars.less (1)

299-310: Consider using color variables for better maintainability.

The dark theme variables are well-structured, but some color values are hardcoded. Consider defining these as base color variables for better maintainability and consistency.

Example:

-  --tv-color-text-control: var(--tv-base-color-common-11); // #191919
+  --tv-dark-text-control: var(--tv-base-color-common-11);
+  --tv-color-text-control: var(--tv-dark-text-control);

This approach:

  • Makes it easier to update theme colors
  • Maintains clear separation between base colors and semantic colors
  • Improves maintainability
packages/theme/src/base/dark-theme.less (1)

375-386: Consider reorganizing dark theme specific variables.

The dark theme specific variables could be better organized by grouping related variables together and using consistent naming patterns.

Consider this organization:

// Dark theme specific backgrounds
-// -- 暗色新增 -- 
-  --tv-color-bg-2: var(--tv-base-color-common-9);
-  --tv-color-bg-3: var(--tv-base-color-common-11);
-  --tv-color-bg-4: var(--tv-base-color-common-6);
+/* Dark theme backgrounds */
+  --tv-dark-bg-panel: var(--tv-base-color-common-9);      // Dropdown panels, tooltips
+  --tv-dark-bg-container: var(--tv-base-color-common-11); // Navigation, panels
+  --tv-dark-bg-accent: var(--tv-base-color-common-6);     // Timeline dots

// Dark theme specific states
-  --tv-color-bg-progressbar: var(--tv-base-color-success-6);
-  --tv-color-bg-active-emphasize-1: var(--tv-base-color-common-9);
+/* Dark theme states */
+  --tv-dark-bg-progress: var(--tv-base-color-success-6);  // Progress indicators
+  --tv-dark-bg-selected: var(--tv-base-color-common-9);   // Selected items
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 13b5a25 and b44cbb3.

📒 Files selected for processing (43)
  • examples/sites/src/views/layout/layout.vue (3 hunks)
  • packages/theme/src/autocomplete/vars.less (1 hunks)
  • packages/theme/src/badge/vars.less (1 hunks)
  • packages/theme/src/base/aurora-theme.less (16 hunks)
  • packages/theme/src/base/dark-theme.less (1 hunks)
  • packages/theme/src/base/index.less (1 hunks)
  • packages/theme/src/base/old-theme.less (15 hunks)
  • packages/theme/src/base/vars.less (7 hunks)
  • packages/theme/src/button-group/vars.less (1 hunks)
  • packages/theme/src/button/vars.less (8 hunks)
  • packages/theme/src/calendar-view/vars.less (1 hunks)
  • packages/theme/src/carousel-item/vars.less (1 hunks)
  • packages/theme/src/cascader-node/vars.less (1 hunks)
  • packages/theme/src/cascader/vars.less (1 hunks)
  • packages/theme/src/crop/vars.less (1 hunks)
  • packages/theme/src/date-panel/vars.less (1 hunks)
  • packages/theme/src/date-table/vars.less (1 hunks)
  • packages/theme/src/dialog-box/vars.less (1 hunks)
  • packages/theme/src/drawer/vars.less (1 hunks)
  • packages/theme/src/dropdown-menu/vars.less (1 hunks)
  • packages/theme/src/dropdown/vars.less (1 hunks)
  • packages/theme/src/grid/vars.less (1 hunks)
  • packages/theme/src/guide/vars.less (1 hunks)
  • packages/theme/src/link/vars.less (1 hunks)
  • packages/theme/src/modal/vars.less (2 hunks)
  • packages/theme/src/month-table/vars.less (1 hunks)
  • packages/theme/src/nav-menu/index.less (1 hunks)
  • packages/theme/src/nav-menu/vars.less (2 hunks)
  • packages/theme/src/notify/vars.less (1 hunks)
  • packages/theme/src/popover/vars.less (1 hunks)
  • packages/theme/src/progress/vars.less (1 hunks)
  • packages/theme/src/select-dropdown/vars.less (1 hunks)
  • packages/theme/src/slider/vars.less (1 hunks)
  • packages/theme/src/steps/vars.less (4 hunks)
  • packages/theme/src/switch/vars.less (3 hunks)
  • packages/theme/src/tabs/vars.less (3 hunks)
  • packages/theme/src/tag/vars.less (1 hunks)
  • packages/theme/src/time-panel/vars.less (1 hunks)
  • packages/theme/src/time-spinner/vars.less (1 hunks)
  • packages/theme/src/tooltip/vars.less (2 hunks)
  • packages/theme/src/tree-menu/vars.less (2 hunks)
  • packages/theme/src/upload-list/vars.less (1 hunks)
  • packages/theme/src/user-contact/vars.less (1 hunks)
✅ Files skipped from review due to trivial changes (3)
  • packages/theme/src/drawer/vars.less
  • packages/theme/src/cascader-node/vars.less
  • examples/sites/src/views/layout/layout.vue
🔇 Additional comments (44)
packages/theme/src/crop/vars.less (1)

17-17: LGTM! Appropriate variable selection for crop center background.

The change from --tv-color-bg-control to --tv-color-bg-control-unactive better reflects the semantic meaning of the crop center's background state.

packages/theme/src/carousel-item/vars.less (1)

15-15: LGTM! Improved color variable semantics.

Changing from --tv-color-text-white to --tv-color-text-inverse improves theme flexibility and better supports dark mode by using semantic color naming instead of a specific color.

packages/theme/src/dropdown-menu/vars.less (1)

17-17: LGTM! Consistent background color variable usage.

The change from --tv-color-bg-secondary to --tv-color-bg-2 aligns with the new theme system while maintaining backward compatibility through the fallback value.

packages/theme/src/user-contact/vars.less (1)

27-27: LGTM! Semantic improvement in color variable.

The change from --tv-color-text-white to --tv-color-text-inverse improves theme flexibility by using semantic color naming.

packages/theme/src/progress/vars.less (1)

22-22: LGTM! Consistent color variable update with fallback.

The change to --tv-color-text-inverse maintains backward compatibility with the #ffffff fallback while improving theme support.

packages/theme/src/autocomplete/vars.less (1)

30-30: LGTM! Standardized background color variable.

The change from --tv-color-bg-secondary to --tv-color-bg-2 aligns with the standardized background color levels while maintaining the fallback.

packages/theme/src/badge/vars.less (1)

23-24: LGTM! Consistent text color updates.

Both badge text and link text colors now use --tv-color-text-inverse, maintaining consistency and improving theme support.

Also applies to: 27-27

packages/theme/src/time-spinner/vars.less (1)

41-41:

❓ Verification inconclusive

Verify hover state visibility with the new color.

The change from --tv-color-bg-hover-1 to --tv-color-bg-hover-3 might affect the hover state visibility. Please ensure the new hover state provides sufficient visual feedback in both light and dark themes.


🌐 Web query:

What is the recommended contrast ratio for hover states in UI components?

💡 Result:

For UI components' hover states, WCAG 2.1 Success Criterion 1.4.11 (Non-text Contrast) requires a minimum 3:1 contrast ratio between the hover state indicator and adjacent colors when either:

  1. The hover effect provides critical visual information needed to identify interactive elements
  2. The component lacks other distinguishing indicators (like text labels or positional context)

Key exceptions:

  • If other clear identifiers exist (e.g., button text labels with sufficient contrast), supplementary hover effects don't need separate 3:1 compliance[3][7]
  • Native browser default focus/hover indicators are exempt[3]

Implementation best practices:

  • Use subtle animations (200ms duration) for smooth transitions[2][8]
  • Maintain visual consistency across similar components[2][10]
  • Combine with cursor changes (e.g., pointer cursor)[2][9]
  • Ensure effects work alongside - not instead of - keyboard navigation[6]

Examples of compliant implementations include:

  • A gray (#DEDEDE) button background on hover with black text maintains 15:1 border contrast[3]
  • Underline animations on links where text already meets 4.5:1 contrast[3]

Non-compliant examples include:

  • Red (#B1262B) underlines on dark backgrounds with <3:1 contrast[3]
  • Gray (#AAA) focus rings with insufficient contrast against white backgrounds[3]

Design systems like Material Design recommend using hover states to enhance usability while maintaining accessibility fundamentals like contrast ratios[10].

Citations:


Verify hover state contrast and accessibility

The hover background color in Time Spinner now uses var(--tv-color-bg-hover-3) instead of the previous hover-1 value. Please ensure that:

  • The new hover color meets the WCAG 2.1 guideline of at least a 3:1 contrast ratio against its adjacent background elements.
  • The hover state remains clearly visible and provides sufficient visual feedback in both light and dark themes.
packages/theme/src/dropdown/vars.less (1)

23-23: LGTM! Color variable updated for dark theme support.

The change from --tv-color-icon-white to --tv-color-icon-inverse-tint aligns with the theme's new color system while maintaining the same fallback color.

packages/theme/src/time-panel/vars.less (1)

23-23: LGTM! Background color variable standardized.

The change from --tv-color-bg-secondary to --tv-color-bg-2 aligns with the new background color system while maintaining the same fallback color.

packages/theme/src/select-dropdown/vars.less (1)

39-39: LGTM! Background color variable standardized.

The change from --tv-color-bg-secondary to --tv-color-bg-2 aligns with the new background color system while maintaining the same fallback color.

packages/theme/src/month-table/vars.less (1)

46-46: LGTM! Color variables updated for dark theme support.

Both changes align with the new color system:

  1. Text color: --tv-color-text-white--tv-color-text-inverse-active
  2. Hover background: --tv-color-bg-hover-1--tv-color-bg-hover-3

The fallback colors are maintained for backward compatibility.

Also applies to: 53-53

packages/theme/src/upload-list/vars.less (1)

41-41: LGTM! Theme improvements enhance component flexibility.

The changes improve dark theme support by using semantic color variables and add new customization options for better component flexibility.

Also applies to: 45-53

packages/theme/src/switch/vars.less (1)

17-17: LGTM! Semantic color variables improve theme consistency.

The switch component now uses more semantic color variables (inverse, unactive) which better support both light and dark themes.

Also applies to: 27-27, 37-37

packages/theme/src/guide/vars.less (1)

11-11: LGTM! Updated to use semantic background color.

The guide header now uses the semantic background color variable bg-3, aligning with the new theme system.

packages/theme/src/notify/vars.less (1)

20-20: LGTM! Updated to use semantic background color.

The notification background now uses the semantic background color variable bg-2, aligning with the new theme system.

packages/theme/src/date-panel/vars.less (1)

26-26: LGTM! Updated to use semantic background color while maintaining fallback.

The date panel background now uses the semantic background color variable bg-2 while properly maintaining the fallback color value.

packages/theme/src/tree-menu/vars.less (1)

31-31: LGTM! Theme variable updates improve dark mode support.

The changes to text and background color variables are well-structured:

  • Using --tv-color-text-control provides better semantic meaning for interactive elements
  • Moving to --tv-color-bg-3 aligns with the standardized background color system

Also applies to: 71-71

packages/theme/src/button-group/vars.less (1)

51-51: LGTM! Improved semantic naming for text colors.

The changes from text-white to text-inverse variants are excellent:

  • More semantic naming better represents the intent
  • Supports both light and dark themes effectively

Also applies to: 59-59

packages/theme/src/tooltip/vars.less (1)

22-22: LGTM! Comprehensive theme variable improvements.

The changes enhance theme consistency and flexibility:

  • Standardized background color levels with bg-2
  • Better dark theme support with text-inverse variants
  • Improved contrast with text-inverse-tint for light theme

Also applies to: 59-59, 64-64, 66-66

packages/theme/src/slider/vars.less (1)

43-43: LGTM! Consistent theme variable update.

The change to bg-inverse for the slider handle improves theme consistency and contrast.

packages/theme/src/date-table/vars.less (1)

40-40: LGTM! Enhanced visual hierarchy in theme variables.

The changes improve both theme support and visual hierarchy:

  • Better dark theme support with text-inverse
  • Stronger emphasis for hover states using bg-hover-3

Also applies to: 46-46, 54-54

packages/theme/src/nav-menu/vars.less (2)

17-17: LGTM! Background color update aligns with dark theme support.

The change from --tv-color-bg-secondary to --tv-color-bg-3 standardizes the background color variable usage.


38-39: LGTM! Good addition of fallback value.

The new variable with fallback value #fff ensures backward compatibility while supporting dark themes.

packages/theme/src/link/vars.less (1)

46-51: LGTM! Link color variables updated for better theme support.

The changes standardize link colors by:

  • Using --tv-color-text-link-1 for primary text
  • Using --tv-color-text-link-1-hover for hover state
packages/theme/src/modal/vars.less (1)

23-23: LGTM! Good use of layered background colors.

The changes implement a layered approach to modal backgrounds:

  • Main modal box uses --tv-color-bg-3
  • Message box uses --tv-color-bg-2

This creates visual hierarchy through subtle background differences.

Also applies to: 93-93

packages/theme/src/cascader/vars.less (1)

41-41: LGTM! Good color contrast handling.

The changes improve theme support through:

  • Using --tv-color-bg-2 for dropdown background
  • Using --tv-color-icon-inverse for tag icons, ensuring good contrast

Also applies to: 47-47

packages/theme/src/calendar-view/vars.less (1)

24-24: LGTM! Good use of inverse background for active state.

The change from --tv-color-icon-white to --tv-color-bg-inverse ensures proper contrast in both light and dark themes.

packages/theme/src/tabs/vars.less (4)

19-20: LGTM! Standardizing color variables.

The change from action-specific color variable to a more generic text color variable helps standardize the theme system while maintaining the same visual appearance.


41-42: LGTM! Simplifying color variables.

The change to use the base text color variable simplifies the theme system while maintaining the same visual appearance.


151-152: LGTM! Replacing hardcoded color with theme variable.

The change improves theme customization by using a theme variable while maintaining backward compatibility with the same fallback color.


159-160: LGTM! Using semantic background color variable.

The change to use a more semantic background color variable improves theme consistency while maintaining the same visual appearance.

packages/theme/src/tag/vars.less (1)

86-87: LGTM! Using semantic inverse color variable for dark theme.

The change to use a semantic inverse color variable improves dark theme support and maintains consistency with other components.

packages/theme/src/grid/vars.less (1)

45-46: LGTM! Replacing hardcoded color with theme variable.

The change improves dark theme support by using a theme variable instead of a hardcoded color.

packages/theme/src/nav-menu/index.less (1)

128-129: LGTM! Using theme variable for popmenu background.

The change improves theme customization by using a theme variable instead of a hardcoded color.

packages/theme/src/button/vars.less (3)

95-101: LGTM! Improved dark theme support with inverse color variables.

The changes to button text and icon colors using inverse color variables (inverse-tint and inverse) provide better support for dark themes while maintaining visual consistency across different button states.

Also applies to: 110-110, 119-119, 128-128, 137-137


223-223: LGTM! Enhanced hover state consistency.

Using --tv-color-border-secondary-hover for the active button border color improves visual consistency in hover states.


357-357: LGTM! Improved control element consistency.

Using --tv-color-text-control for icon button text color aligns with the styling of other control elements in the UI.

packages/theme/src/base/aurora-theme.less (1)

451-451: LGTM! Enhanced visual depth for interactive elements.

The updated shadow value for card hover/selection states improves the visual hierarchy and interaction feedback.

packages/theme/src/base/old-theme.less (1)

6-6: LGTM! Theme-specific color values preserved.

The distinct color values between themes maintain their unique visual identities while sharing the same variable structure.

Also applies to: 23-34

packages/theme/src/base/vars.less (1)

147-149: LGTM! Well-structured inverse text color variables.

The new inverse text color variables provide a consistent way to handle text colors on inverse backgrounds, improving theme flexibility.

packages/theme/src/base/dark-theme.less (1)

298-318:

❓ Verification inconclusive

Verify color contrast ratios for accessibility.

Please verify that the text colors meet WCAG 2.1 contrast requirements, particularly:

  • Primary text color (#E6E6E6) on dark backgrounds
  • Secondary text color (#B3B3B3) on dark backgrounds
  • Link colors (#5291FF) on dark backgrounds

Run this script to check contrast ratios:


🏁 Script executed:

#!/bin/bash
# Description: Check color contrast ratios using WCAG guidelines

# Install and use color-contrast-checker
npm install color-contrast-checker
node -e '
const ColorContrastChecker = require("color-contrast-checker");
const ccc = new ColorContrastChecker();

const colors = {
  background: "#1A1A1A",
  primaryText: "#E6E6E6",
  secondaryText: "#B3B3B3",
  linkColor: "#5291FF"
};

console.log("Primary text contrast ratio:", 
  ccc.getContrastRatio(colors.primaryText, colors.background));
console.log("Secondary text contrast ratio:", 
  ccc.getContrastRatio(colors.secondaryText, colors.background));
console.log("Link color contrast ratio:", 
  ccc.getContrastRatio(colors.linkColor, colors.background));
'

Length of output: 1591


Manual Verification Required for Color Contrast Ratios

The automated contrast check did not produce output due to dependency issues—specifically, the required module was not found. Please verify manually that the following color combinations meet WCAG 2.1 contrast requirements on the dark background (#1A1A1A):

  • Primary text color: #E6E6E6
  • Secondary text color: #B3B3B3
  • Link color: #5291FF

If an automated approach is desired, consider updating the contrast checking tool or using a known, pre-installed alternative. Until then, manual testing using a trusted contrast checker is necessary.

packages/theme/src/steps/vars.less (2)

31-31: LGTM! Semantic improvements for inverse colors.

The changes to use inverse and inverse-tint variables improve semantic meaning and better support dark theme compatibility.

Also applies to: 144-144, 196-196


232-232:

✅ Verification successful

Verify the background color variable change.

The change from semantic naming (--tv-color-bg-control) to a numbered system (--tv-color-bg-4) appears to deviate from the "inverse" pattern used in other changes. Please verify that this aligns with the intended theming strategy.

Run this script to check for consistency in background color variable usage:


🏁 Script executed:

#!/bin/bash
# Description: Check for usage patterns of background color variables
# Look for both semantic and numbered variants to understand the convention

echo "Checking semantic bg variables..."
rg --type less "tv-color-bg-control"

echo "Checking numbered bg variables..."
rg --type less "tv-color-bg-[0-9]"

Length of output: 4607


Background Color Variable Usage Verified

After reviewing the codebase, the use of the numbered background variable (--tv-color-bg-4) for the timeline dot in Steps is consistent with the theming strategy. We verified that:

  • In several theme definition files (e.g., packages/theme/src/base/vars.less, old-theme.less, and aurora-theme.less), the numeric variable --tv-color-bg-4 is used for timeline components and shares the same fallback value (#c2c2c2) as its semantic counterpart.
  • This change aligns with the intentional design where specific UI elements (like timeline dots) are tied to numbered background color variables rather than semantic ones.

No further adjustments are required here.

@@ -14,4 +14,5 @@
@import './vars.less';
// @import './old-theme.less';
// @import './aurora-theme.less';
// @import './dark-theme.less';
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Uncomment dark theme import to enable dark theme support.

The dark theme import is commented out, which prevents the dark theme styles from being applied. This seems inconsistent with the PR's objective of adding dark theme support.

Apply this diff to enable dark theme support:

-// @import './dark-theme.less';
+@import './dark-theme.less';
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// @import './dark-theme.less';
@import './dark-theme.less';

@zzcr zzcr merged commit 992799c into opentiny:dev Feb 14, 2025
14 of 22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request (功能增强)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants