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(dropdown): [action-menu,dropdown,autocomplete,input] component dark theme adaptation #3014

Merged
merged 5 commits into from
Mar 4, 2025

Conversation

MomoPoppy
Copy link
Collaborator

@MomoPoppy MomoPoppy commented Mar 4, 2025

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

  • Style

    • Updated icons, dropdowns, and action menus with dynamic theming to replace fixed colors.
    • Enhanced hover effects and refined spacing between icons and text for improved visual consistency.
    • Modified layout behavior of dialog select component for better alignment of elements.
    • Adjusted background colors for dropdowns and action menus to utilize CSS variables for better theming.
    • Introduced overflow handling and hover background color updates for the storage list component.
    • Added fill color for loading icons in autocomplete components.
  • New Features

    • Introduced a sizing option for dropdown buttons, offering more flexible layout control.
  • Chores

    • Streamlined dropdown configuration by replacing an older attribute with a more intuitive property.
    • Expanded theme customization with additional color and border options.
    • Removed outdated demo content related to a legacy button option.
    • Added vertical spacing in autocomplete components for improved layout.

Copy link

coderabbitai bot commented Mar 4, 2025

Walkthrough

This pull request updates various Vue component files and theme styles to replace hardcoded color values with CSS variables. It modifies inline styles by adding spacing, and changes certain component attributes (replacing split-button with border) to streamline dropdown behavior. New CSS custom properties are introduced in theme files to expand color options, and a new prop (size) is added to a Vue dropdown component to enhance button rendering.

Changes

File(s) Change Summary
examples/.../action-menu/{icon-composition-api.vue, icon.vue}
examples/.../dropdown/{show-icon-composition-api.vue, show-icon.vue}
Replaced hardcoded color #191919 with CSS variable var(--tv-color-icon-control) for text and SVG fill properties.
examples/.../action-menu/{popper-class-composition-api.vue, popper-class.vue} Updated background color from antiquewhite to var(--tv-color-bg-3) in the custom action-menu styles.
examples/.../action-menu/{slot-item-composition-api.vue, slot-item.vue} Added margin-left: 4px to <span> elements to adjust spacing in the visual layout.
examples/.../dropdown/{inherit-width-composition-api.vue, inherit-width.vue, size-composition-api.vue, size.vue} Replaced the split-button attribute with border in <tiny-dropdown> components across different usage scenarios.
examples/.../dropdown/webdoc/dropdown.js Removed the demo entry for the "split-button" feature, simplifying the dropdown demo list.
packages/theme/src/{action-menu/index.less, base/{dark-theme.less, vars.less}, dropdown/{index.less, vars.less}} Added new CSS custom properties for backgrounds and borders, refined SVG fill colors, and adjusted hover state rules to support dynamic theming.
packages/vue/src/dropdown/src/pc.vue Added a new size prop to the <tiny-button> component and adjusted class binding to enhance button rendering.

Sequence Diagram(s)

sequenceDiagram
    participant U as User
    participant DD as Dropdown Component
    participant TB as Tiny Button Component

    U->>DD: Interact with dropdown
    DD->>TB: Render button with `size` prop
    TB-->>DD: Return rendered button
    DD-->>U: Display updated dropdown
Loading

Possibly related PRs

Suggested labels

feature

Poem

I’m a little rabbit, hopping with glee,
Watching colors change dynamically on screen 🥕,
No more stale hues, now variables lead the way,
With borders and spacing making a brighter day,
From icons to buttons, our code sings in harmony,
A bunny’s cheer for cleaner themes — hip, hip, hooray!

Warning

There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

examples/sites/demos/pc/app/action-menu/popper-class.spec.ts

Oops! Something went wrong! :(

ESLint: 8.57.1

ESLint couldn't find the plugin "eslint-plugin-vue".

(The package "eslint-plugin-vue" was not found when loaded as a Node module from the directory "".)

It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:

npm install eslint-plugin-vue@latest --save-dev

The plugin "eslint-plugin-vue" was referenced from the config file in ".eslintrc.js » @antfu/eslint-config » @antfu/eslint-config-vue".

If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team.

examples/sites/demos/pc/app/dropdown/split-button.spec.ts

Oops! Something went wrong! :(

ESLint: 8.57.1

ESLint couldn't find the plugin "eslint-plugin-vue".

(The package "eslint-plugin-vue" was not found when loaded as a Node module from the directory "".)

It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:

npm install eslint-plugin-vue@latest --save-dev

The plugin "eslint-plugin-vue" was referenced from the config file in ".eslintrc.js » @antfu/eslint-config » @antfu/eslint-config-vue".

If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team.


📜 Recent review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between a5326e8 and b563ad1.

📒 Files selected for processing (2)
  • examples/sites/demos/pc/app/action-menu/popper-class.spec.ts (1 hunks)
  • examples/sites/demos/pc/app/dropdown/split-button.spec.ts (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • examples/sites/demos/pc/app/dropdown/split-button.spec.ts
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: PR E2E Test (pnpm test:e2e3)
🔇 Additional comments (1)
examples/sites/demos/pc/app/action-menu/popper-class.spec.ts (1)

16-16:

❓ Verification inconclusive

Updated background color assertion to match new theming system.

The test has been updated to expect a white background ('rgb(255, 255, 255)') instead of the previous color, aligning with the component's updated styling as part of the dark theme adaptation.

Given that this PR is implementing dark theme adaptation, we should verify that this test will still pass in both light and dark themes:


🏁 Script executed:

#!/bin/bash
# Verify if the component's CSS handles theme-specific background colors correctly

# Check if there are CSS variables defined for the dropdown menu background
rg -A 2 -B 2 "tiny-dropdown-menu" --type css

# Check if the theme system has variables for dropdown backgrounds
rg -A 2 -B 2 "dropdown.*background" --type css

# Check how the dropdown menu background is defined in the component
rg "background(-color)?" --type vue --glob "*dropdown*"

Length of output: 179


Action Required: Verify Theme-Specific Backgrounds in Dropdown Menu

The test now explicitly asserts a white background ('rgb(255, 255, 255)'), which aligns with the updated theming system. However, because our automated CSS queries returned no relevant output and the search for Vue files produced an error, it’s unclear whether the component correctly adapts to both light and dark themes. Please manually verify that:

  • The component applies theme-specific CSS variables (if any) for dropdown backgrounds in both themes.
  • The styling changes in dark mode do not conflict with the expected white background in light mode.
✨ Finishing Touches
  • 📝 Generate Docstrings

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.
  • @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 Mar 4, 2025
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: 2

🔭 Outside diff range comments (1)
packages/vue/src/dropdown/src/pc.vue (1)

33-44: 💡 Verification agent

🧩 Analysis chain

Verify this new API addition is documented

The PR introduces a new size prop that should be reflected in component documentation. Please ensure the API documentation is updated accordingly.


🏁 Script executed:

#!/bin/bash
# Check if the size prop is documented in the relevant markdown documentation files

# Look for dropdown documentation files
echo "Searching for dropdown documentation files..."
fd --type f "dropdown" -e md

# Check if size prop is mentioned in documentation
echo "Checking if size prop is mentioned in documentation..."
rg -i "size.*prop" --type md

Length of output: 535


Documentation Update Required for New size Prop

It appears that the new size prop introduced in packages/vue/src/dropdown/src/pc.vue is currently not referenced in the dropdown component’s API documentation. The search found several dropdown documentation files (e.g.,
examples/sites/demos/mobile-first/app/dropdown/webdoc/dropdown.cn.md
examples/sites/demos/mobile-first/app/dropdown/webdoc/dropdown.en.md
examples/sites/demos/pc/app/dropdown/webdoc/dropdown.cn.md
examples/sites/demos/pc/app/dropdown/webdoc/dropdown.en.md)
but none of them seem to mention the new size prop.

Please ensure that the API documentation is updated to include a description of the size prop (its type, default value, and usage) so that users have accurate and complete information.

🧹 Nitpick comments (6)
examples/sites/demos/pc/app/dropdown/inherit-width.vue (1)

18-18: Attribute order differs from composition API version

While functionally identical to the change in the composition API version, the attribute order is different here (type="success" border vs border type="success"). Consider maintaining consistent attribute ordering across similar components for better code maintainability.

-    <tiny-dropdown type="success" border :inherit-width="true">
+    <tiny-dropdown border type="success" :inherit-width="true">
packages/theme/src/base/vars.less (2)

376-380: Good addition of new CSS theme variables with descriptive comments.

The new variables provide essential theming capabilities for various UI components:

  1. --tv-color-bg-active-control-1 - For disabled radio button selection states
  2. --tv-color-bg-5 and --tv-color-bg-5-active - For carousel indicators
  3. --tv-color-bg-inverse-tint-1 - For image toolbars
  4. --tv-color-border-inverse-tint - For tag borders

Each variable includes a helpful comment describing its purpose, which enhances code maintainability.

I recommend additionally documenting which components or specific UI elements are affected by these variables in a central theming guide, if one exists.


376-376: Verify spacing consistency in variable declaration.

There's no space after the colon in the variable declaration --tv-color-bg-active-control-1:var(--tv-base-color-common-6), unlike other variable declarations in the file. While it doesn't affect functionality, it's good to maintain consistent formatting.

- --tv-color-bg-active-control-1:var(--tv-base-color-common-6); // #dbdbdb radio 选中禁用色
+ --tv-color-bg-active-control-1: var(--tv-base-color-common-6); // #dbdbdb radio 选中禁用色
packages/theme/src/base/dark-theme.less (3)

387-387: Fix spacing inconsistency in variable declaration.

There's no space after the colon in the variable declaration for --tv-color-bg-active-control-1, similar to the issue in vars.less. Maintaining consistent formatting will improve code readability.

- --tv-color-bg-active-control-1:var(--tv-base-color-brand-4); // #264DAB radio 选中禁用色
+ --tv-color-bg-active-control-1: var(--tv-base-color-brand-4); // #264DAB radio 选中禁用色

388-390: Fix spacing inconsistencies in multiple variable declarations.

The spacing after colons is missing in all new variable declarations. For better readability and consistency with the rest of the file, add spaces after the colons.

- --tv-color-bg-5:var(--tv-base-color-common-6); // #808080 轮播指示器背景色
- --tv-color-bg-5-active:var(--tv-base-color-common-2); // #E6E6E6  轮播指示器激活背景色
- --tv-color-bg-inverse-tint-1:var(--tv-base-color-common-10); // #262626 图片工具栏背景色
+ --tv-color-bg-5: var(--tv-base-color-common-6); // #808080 轮播指示器背景色
+ --tv-color-bg-5-active: var(--tv-base-color-common-2); // #E6E6E6  轮播指示器激活背景色
+ --tv-color-bg-inverse-tint-1: var(--tv-base-color-common-10); // #262626 图片工具栏背景色

413-413: Fix spacing inconsistency in border variable declaration.

The spacing after the colon is missing in the variable declaration. For better readability and consistency with the rest of the file, add a space after the colon.

- --tv-color-border-inverse-tint:var(--tv-base-color-common-13); // #000 tag边框
+ --tv-color-border-inverse-tint: var(--tv-base-color-common-13); // #000 tag边框
📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between e04d727 and 977097b.

⛔ Files ignored due to path filters (1)
  • packages/theme/src/svgs/ellipsis.svg is excluded by !**/*.svg
📒 Files selected for processing (19)
  • examples/sites/demos/pc/app/action-menu/icon-composition-api.vue (1 hunks)
  • examples/sites/demos/pc/app/action-menu/icon.vue (1 hunks)
  • examples/sites/demos/pc/app/action-menu/popper-class-composition-api.vue (1 hunks)
  • examples/sites/demos/pc/app/action-menu/popper-class.vue (1 hunks)
  • examples/sites/demos/pc/app/action-menu/slot-item-composition-api.vue (1 hunks)
  • examples/sites/demos/pc/app/action-menu/slot-item.vue (1 hunks)
  • examples/sites/demos/pc/app/dropdown/inherit-width-composition-api.vue (2 hunks)
  • examples/sites/demos/pc/app/dropdown/inherit-width.vue (2 hunks)
  • examples/sites/demos/pc/app/dropdown/show-icon-composition-api.vue (1 hunks)
  • examples/sites/demos/pc/app/dropdown/show-icon.vue (1 hunks)
  • examples/sites/demos/pc/app/dropdown/size-composition-api.vue (4 hunks)
  • examples/sites/demos/pc/app/dropdown/size.vue (4 hunks)
  • examples/sites/demos/pc/app/dropdown/webdoc/dropdown.js (0 hunks)
  • packages/theme/src/action-menu/index.less (1 hunks)
  • packages/theme/src/base/dark-theme.less (2 hunks)
  • packages/theme/src/base/vars.less (2 hunks)
  • packages/theme/src/dropdown/index.less (2 hunks)
  • packages/theme/src/dropdown/vars.less (1 hunks)
  • packages/vue/src/dropdown/src/pc.vue (2 hunks)
💤 Files with no reviewable changes (1)
  • examples/sites/demos/pc/app/dropdown/webdoc/dropdown.js
✅ Files skipped from review due to trivial changes (3)
  • examples/sites/demos/pc/app/action-menu/slot-item.vue
  • examples/sites/demos/pc/app/action-menu/slot-item-composition-api.vue
  • examples/sites/demos/pc/app/action-menu/popper-class-composition-api.vue
🔇 Additional comments (29)
packages/theme/src/action-menu/index.less (1)

37-40: Good CSS enhancement for dropdown alignment.

Adding flex display with centered alignment for dropdowns within action menu items improves the visual consistency of these nested components, especially important for dark theme compatibility.

examples/sites/demos/pc/app/action-menu/popper-class.vue (1)

42-42: Excellent use of CSS variables for theming.

Replacing the hardcoded background color with var(--tv-color-bg-3) improves theme adaptability, making the component correctly respond to both light and dark themes.

examples/sites/demos/pc/app/dropdown/size-composition-api.vue (4)

4-4: Attribute update from split-button to border enhances component flexibility.

Replacing the split-button attribute with border standardizes the dropdown component API and aligns with the dark theme adaptation goal. This change maintains the button's border appearance while potentially simplifying the implementation.


17-17: Size prop addition provides better component customization.

Adding the size="medium" prop explicitly defines the component's size dimension, improving the clarity and predictability of the rendered output across different themes.


30-30: Consistent attribute replacement maintains UI coherence.

The border attribute replacement is consistently applied to the small size dropdown, ensuring visual continuity across different size variations.


43-43: Complete implementation across all size variants.

The mini-sized dropdown also receives the attribute update, ensuring a complete and consistent implementation of the theming changes across all component variants.

examples/sites/demos/pc/app/dropdown/size.vue (2)

4-4: Synchronized attribute updates in standard Vue implementation.

The change from split-button to border is correctly applied to the standard Vue implementation, maintaining consistency with the Composition API version.


30-30: Comprehensive attribute update across all dropdown variants.

The consistent replacement of split-button with border across all size variations (small and mini) demonstrates thorough implementation of the API change.

Also applies to: 43-43

examples/sites/demos/pc/app/dropdown/show-icon.vue (2)

71-78: Good use of CSS variables for theming support

Replacing hardcoded color values with CSS variables (var(--tv-color-icon-control)) is a great approach for dark theme adaptation. This ensures the icon colors will properly adjust based on the active theme.


83-85: Consistent icon styling with CSS variables

Adding the same CSS variable to .tiny-dropdown__prefix-inner ensures consistent styling for all icon elements in the dropdown component across themes.

examples/sites/demos/pc/app/dropdown/inherit-width-composition-api.vue (1)

18-18: Consistent attribute change from split-button to border

Similar to the previous instance, the attribute has been changed from split-button to border while maintaining the :inherit-width="true" property.

examples/sites/demos/pc/app/dropdown/inherit-width.vue (1)

4-4: Attribute change from split-button to border

Replacing split-button with border is consistent with the changes made in the composition API version.

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

25-25: Improved CSS variable naming consistency

Updating the variable reference from --tv-icon-hover to --tv-color-icon-hover improves naming consistency with other color-related variables. This change supports better organization of theme variables while maintaining the same fallback value.

examples/sites/demos/pc/app/dropdown/show-icon-composition-api.vue (2)

61-71: Well-structured CSS refactoring using nesting!

The nested structure using & for parent reference improves code organization and maintainability. The replacement of hardcoded color values with CSS variables (var(--tv-color-icon-control)) enables proper dark theme support.


73-75: Good addition of consistent styling for prefix icons

Adding specific styling for the .tiny-dropdown__prefix-inner element ensures visual consistency between prefix and suffix icons in the dropdown component. This is essential for a cohesive dark theme adaptation.

packages/vue/src/dropdown/src/pc.vue (2)

187-187: Great enhancement adding size support to bordered dropdown buttons

Previously, the size property was defined but not passed to the button when using the bordered variation. This addition ensures consistent sizing behavior across all dropdown presentation modes.


201-201: Code readability improvement

The class binding expression has been simplified by removing unnecessary line breaks while maintaining the same functionality. This makes the code more concise and easier to read.

examples/sites/demos/pc/app/action-menu/icon-composition-api.vue (3)

79-82: Excellent color variable implementation for text and icons!

Replacing the hardcoded color values with the CSS variable var(--tv-color-icon-control) for both the text color and icon fill ensures consistent styling and proper dark theme adaptation.


85-89: Proper hover state handling

Maintaining the same CSS variable for hover states ensures visual consistency throughout all interactive states of the component in both light and dark themes.


91-95: Consistent dropdown styling within action menu

The consistent application of the same CSS variable to nested dropdown components ensures visual harmony throughout the component hierarchy, which is crucial for a cohesive theme adaptation.

packages/theme/src/dropdown/index.less (3)

49-53: Improved selector specificity for non-button triggers

The selector change from a specific button class to :not(.tiny-button) broadens the scope to apply styles to all non-button triggers, making the implementation more flexible while maintaining the intended styling.

The fill color change to var(--tv-Dropdown-icon-color-link) aligns with the theming system and supports dark mode adaptation.


123-127: Enhanced specificity for hover states on non-disabled buttons

Adding :not(.is-disabled) to the button hover state selector ensures that hover effects are only applied to interactive elements, which is an accessibility improvement. This prevents visual confusion by not showing hover effects on elements that cannot be interacted with.


15-19:

✅ Verification successful

Verify CSS variable definitions for dark theme

The changes in this file refer to CSS variables that should have corresponding definitions for dark theme. Please ensure that these variables (--tv-Dropdown-icon-color-link, etc.) have appropriate values defined in the dark theme variant.


🏁 Script executed:

#!/bin/bash
# Check for CSS variable definitions in theme files

# Look for dark theme variable definitions
echo "Searching for dark theme variable definitions..."
rg -A 3 "tv-Dropdown-icon-color-link" --type less

# Check other related theme variables
echo "Checking other related theme variables..."
rg -A 3 "tv-color-icon-control" --type less

# Look for injection function that might define these variables
echo "Looking for variable injection functions..."
rg "inject-Dropdown-vars" --type less

Length of output: 9639


Dark Theme Variable Definitions Verified

The CSS variable for the dropdown icon color is correctly defined. The variable --tv-Dropdown-icon-color-link is set up in packages/theme/src/dropdown/vars.less using a fallback to --tv-color-icon-link, which is overridden appropriately in the dark theme variant at packages/theme/src/base/dark-theme.less. The injection function .inject-Dropdown-vars() is also present and correctly applied in the dropdown component. No further action is required regarding dark theme definitions.

examples/sites/demos/pc/app/action-menu/icon.vue (5)

84-84: Good use of CSS variables for theming support.

Replacing the hardcoded color #191919 with the CSS variable var(--tv-color-icon-control) improves theme compatibility, especially for dark theme adaptation. This change allows the color to be properly adjusted based on the active theme.


86-87: Consistent application of theme variables for SVG icons.

The SVG fill color is now using the same CSS variable as the parent element, ensuring consistent theming behavior. This is a good practice for maintaining visual consistency across component states.


92-93: Proper handling of hover state with theme variables.

Updating the hover state to use the CSS variable ensures the component maintains proper contrast and visibility in both light and dark themes.


97-98: Correct application of theme variables to dropdown element.

The use of the same variable var(--tv-color-icon-control) across related elements maintains consistency in the UI.


99-100: Good theming for nested SVG elements.

Ensuring that even the deeply nested SVG icons use the theme variable is important for full theme compatibility. This change completes the theming support for the entire component hierarchy.

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

387-390: Good parallel implementation of dark theme variables.

The dark theme variables appropriately use different color values than their light theme counterparts:

  1. --tv-color-bg-active-control-1 uses var(--tv-base-color-brand-4) (#264DAB) - More visible on dark backgrounds
  2. --tv-color-bg-5 uses var(--tv-base-color-common-6) (#808080) - Appropriate neutral gray for dark theme
  3. --tv-color-bg-5-active uses var(--tv-base-color-common-2) (#E6E6E6) - Lighter color for active state
  4. --tv-color-bg-inverse-tint-1 uses var(--tv-base-color-common-10) (#262626) - Darker background suitable for dark theme

This demonstrates good attention to accessibility and visual hierarchy in the dark theme.

@@ -14,7 +14,7 @@
</template>
</tiny-dropdown>
<p class="demo-dropdown">其他尺寸:</p>
<tiny-dropdown size="medium" split-button type="primary">
<tiny-dropdown size="medium" border type="primary">
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Verify that all dropdown sizes render correctly with the new attribute.

The medium-sized dropdown now uses the border attribute instead of split-button. Check that the rendered UI maintains the expected visual appearance in both light and dark themes.


🏁 Script executed:

#!/bin/bash
# Check for other instances of split-button that might need updating
rg "split-button" --type vue packages/ examples/ -l

Length of output: 79


Action: Verify dropdown attribute update and UI consistency

Please manually verify that:

  • The updated medium dropdown now uses the border attribute (instead of split-button) and renders correctly.
  • All dropdown size variations display as expected across both light and dark themes.
  • No outdated usage of split-button remains in any Vue component (use the above script to help confirm).

🏁 Script executed:

#!/bin/bash
# Search for any lingering "split-button" attribute in Vue files by using a glob pattern
rg "split-button" --glob "*.vue" packages/ examples/ -l

Length of output: 529


Action Required: Verify Dropdown Attribute Consistency in All Examples

It looks like the update in examples/sites/demos/pc/app/dropdown/size.vue (where the medium-sized dropdown now uses the border attribute) is correctly applied. However, our search detected several other Vue files that still reference "split-button":

  • examples/sites/demos/pc/app/dropdown/split-button.vue
  • examples/sites/demos/pc/app/dropdown/events-composition-api.vue
  • examples/sites/demos/pc/app/dropdown/disabled-composition-api.vue
  • examples/sites/demos/pc/app/dropdown/disabled.vue
  • examples/sites/demos/pc/app/dropdown/events.vue
  • examples/sites/demos/pc/app/dropdown/split-button-composition-api.vue

Please verify whether these remaining instances are intentional (e.g., for legacy demos or separate variants) or if they also require updating to use the new attribute mechanism. Additionally, double-check that all dropdown sizes render consistently in both light and dark themes.

@@ -1,7 +1,7 @@
<template>
<div class="demo-drop">
<p class="demo-dropdown">场景 1:默认</p>
<tiny-dropdown split-button type="success">
<tiny-dropdown border type="success">
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Attribute change from split-button to border

Changing from split-button to border appears to be an API modification that may affect the component's appearance or behavior. Ensure this change is documented in the component's API documentation.

Consider adding a comment in the code explaining the purpose of the border attribute for better developer understanding.


🌐 Web query:

Is there a difference between split-button and border attributes in Vue dropdown components?

💡 Result:

Split-button and border attributes serve distinct purposes in Vue dropdown components based on implementation across different libraries:

Split-button Attribute

  • Structural division: Creates a two-part button where the main section triggers a default action and the secondary section opens the dropdown menu [1][3][4]
  • Implementation:
    <dx-drop-down-button :split-button="true" @button-click="handleClick" />
    • Requires separate handlers for button click (main action) and menu interaction [1]
    • Supported in libraries like DevExtreme, Element Plus, and BootstrapVue [1][3][4]

Border Attributes

  • Styling control: Typically managed through CSS classes/props rather than dedicated border attributes in most Vue dropdown implementations
  • Alternative approaches:
    1. Use elementAttr for custom styling (DevExtreme):
    <DxDropDownBox :element-attr="{ class: 'custom-border' }" />
    1. Apply CSS directly:
    .dropdown-container {
      border: 2px solid #ccc;
      border-radius: 4px;
    }

Key differences:

Feature Split-button Border Styling
Purpose Functional division Visual appearance
Implementation Component-specific prop CSS/Generic HTML attributes
Library Support Explicit in component APIs [1][3][4] Indirect through styling systems

No standard border-specific prop exists in major Vue dropdown implementations - borders are typically controlled through CSS classes or generic attribute passthroughs [5][8]. The split-button is a deliberate component architecture choice, while border treatment is a presentational concern handled through styling mechanisms.

Citations:


Documentation Update Required Due to Attribute Change

  • The change from using the split-button prop to border in <tiny-dropdown border type="success"> notably shifts the component’s behavior. Whereas split-button is intended for creating a dual-action (functional split) button, the border attribute appears to control only styling.
  • Based on recent research, these two attributes serve distinct purposes. Please ensure that this API change is clearly documented in the component's API documentation.
  • Additionally, consider adding an inline comment in the code explaining the rationale behind using the border attribute to aid future maintainers.

File: examples/sites/demos/pc/app/dropdown/inherit-width-composition-api.vue | Line: 4

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: 0

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

376-379: Ensure consistency in declaration formatting and design intent for new dark theme variables.
Four new variables have been introduced:

  • --tv-color-bg-active-control-1: Intended for the disabled radio button when selected, referencing var(--tv-base-color-common-6) (expected to be #dbdbdb).
  • --tv-color-bg-5: For carousel indicator background using var(--tv-base-color-common-5) (#e6e6e6).
  • --tv-color-bg-5-active: For active carousel indicator background using var(--tv-base-color-common-9) (#595959).
  • --tv-color-bg-inverse-tint-1: For image toolbar backgrounds using var(--tv-base-color-common-1) (#fff).

A minor formatting item: these new declarations omit a space after the colon. For consistency with the rest of the file, consider adding a space. For example:

-  --tv-color-bg-active-control-1:var(--tv-base-color-common-6);
-  --tv-color-bg-5:var(--tv-base-color-common-5);
-  --tv-color-bg-5-active:var(--tv-base-color-common-9);
-  --tv-color-bg-inverse-tint-1:var(--tv-base-color-common-1);
+  --tv-color-bg-active-control-1: var(--tv-base-color-common-6);
+  --tv-color-bg-5: var(--tv-base-color-common-5);
+  --tv-color-bg-5-active: var(--tv-base-color-common-9);
+  --tv-color-bg-inverse-tint-1: var(--tv-base-color-common-1);

Please also verify that the chosen color references comply with the dark theme styling strategy.

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 977097b and 15386bd.

📒 Files selected for processing (3)
  • packages/theme/src/base/dark-theme.less (3 hunks)
  • packages/theme/src/base/vars.less (3 hunks)
  • packages/theme/src/dialog-select/index.less (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/theme/src/base/dark-theme.less
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: PR E2E Test (pnpm test:e2e3)
🔇 Additional comments (2)
packages/theme/src/dialog-select/index.less (1)

57-60: Enhanced Flex Alignment for Input Prefix

Changing the display property from the (presumed) previous inline-block to inline-flex along with adding align-items: center significantly improves the vertical alignment of icons and other inline elements within the .tiny-input__prefix container. This update is a modern CSS practice that supports flexible layouts and should integrate well with the dark theme adaptation efforts.

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

367-367: Validate the color usage for top-level backgrounds.
The updated variable --tv-color-bg-3 now uses var(--tv-base-color-common-1) and its comment has been extended to specify its usage for top navigation, dropdown, modal/dialog boxes, and secondary container backgrounds. Please confirm that switching this background to white (#fff) is fully aligned with both the dark theme adaptation and overall design guidelines.

@MomoPoppy MomoPoppy changed the title feat(dropdown): [action-menu,dropdown] component dark theme adaptation feat(dropdown): [action-menu,dropdown,dialog-select,autocomplete] component dark theme adaptation Mar 4, 2025
@MomoPoppy MomoPoppy changed the title feat(dropdown): [action-menu,dropdown,dialog-select,autocomplete] component dark theme adaptation feat(dropdown): [action-menu,dropdown,dialog-select,autocomplete,input] component dark theme adaptation Mar 4, 2025
@MomoPoppy MomoPoppy changed the title feat(dropdown): [action-menu,dropdown,dialog-select,autocomplete,input] component dark theme adaptation feat(dropdown): [action-menu,dropdown,autocomplete,input] component dark theme adaptation Mar 4, 2025
@zzcr zzcr merged commit 694d6f1 into opentiny:dev Mar 4, 2025
14 of 15 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