-
Notifications
You must be signed in to change notification settings - Fork 284
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
feat(dropdown): [action-menu,dropdown,autocomplete,input] component dark theme adaptation #3014
Conversation
WalkthroughThis 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 Changes
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
Possibly related PRs
Suggested labels
Poem
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
examples/sites/demos/pc/app/action-menu/popper-class.spec.tsOops! 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:
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.tsOops! 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:
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 detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (1)
✨ Finishing Touches
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? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
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 mdLength of output: 535
Documentation Update Required for New
size
PropIt appears that the new
size
prop introduced inpackages/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 newsize
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 versionWhile functionally identical to the change in the composition API version, the attribute order is different here (
type="success" border
vsborder 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:
--tv-color-bg-active-control-1
- For disabled radio button selection states--tv-color-bg-5
and--tv-color-bg-5-active
- For carousel indicators--tv-color-bg-inverse-tint-1
- For image toolbars--tv-color-border-inverse-tint
- For tag bordersEach 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
⛔ 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 fromsplit-button
toborder
enhances component flexibility.Replacing the
split-button
attribute withborder
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
toborder
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
withborder
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 supportReplacing 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 variablesAdding 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 fromsplit-button
toborder
Similar to the previous instance, the attribute has been changed from
split-button
toborder
while maintaining the:inherit-width="true"
property.examples/sites/demos/pc/app/dropdown/inherit-width.vue (1)
4-4
: Attribute change fromsplit-button
toborder
Replacing
split-button
withborder
is consistent with the changes made in the composition API version.packages/theme/src/dropdown/vars.less (1)
25-25
: Improved CSS variable naming consistencyUpdating 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 iconsAdding 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 buttonsPreviously, 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 improvementThe 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 handlingMaintaining 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 menuThe 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 triggersThe 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 buttonsAdding
: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 lessLength 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 inpackages/theme/src/dropdown/vars.less
using a fallback to--tv-color-icon-link
, which is overridden appropriately in the dark theme variant atpackages/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 variablevar(--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:
--tv-color-bg-active-control-1
usesvar(--tv-base-color-brand-4)
(#264DAB) - More visible on dark backgrounds--tv-color-bg-5
usesvar(--tv-base-color-common-6)
(#808080) - Appropriate neutral gray for dark theme--tv-color-bg-5-active
usesvar(--tv-base-color-common-2)
(#E6E6E6) - Lighter color for active state--tv-color-bg-inverse-tint-1
usesvar(--tv-base-color-common-10)
(#262626) - Darker background suitable for dark themeThis 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"> |
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.
💡 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 ofsplit-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"> |
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.
💡 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:
- Use
elementAttr
for custom styling (DevExtreme):
<DxDropDownBox :element-attr="{ class: 'custom-border' }" />
- Apply CSS directly:
.dropdown-container { border: 2px solid #ccc; border-radius: 4px; }
- Use
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:
- 1: https://js.devexpress.com/Vue/Documentation/Guide/UI_Components/DropDownButton/Getting_Started_with_DropDownButton/
- 2: https://ej2.syncfusion.com/vue/documentation/drop-down-list/accessibility
- 3: https://element-plus.org/en-US/component/dropdown
- 4: https://bootstrap-vue.org/docs/components/dropdown
- 5: https://js.devexpress.com/Vue/Documentation/ApiReference/UI_Components/dxDropDownBox/Configuration/
- 6: https://ui.vuestic.dev/ui-elements/button-dropdown/
- 7: https://ej2.syncfusion.com/vue/documentation/api/split-button/index
- 8: https://v3.primevue.org/dropdown/
- 9: https://www.syncfusion.com/vue-components/vue-button
- 10: https://developers.quantum.thalesdigital.io/vue/next/components/split-button
Documentation Update Required Due to Attribute Change
- The change from using the
split-button
prop toborder
in<tiny-dropdown border type="success">
notably shifts the component’s behavior. Whereassplit-button
is intended for creating a dual-action (functional split) button, theborder
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
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.
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, referencingvar(--tv-base-color-common-6)
(expected to be#dbdbdb
).--tv-color-bg-5
: For carousel indicator background usingvar(--tv-base-color-common-5)
(#e6e6e6
).--tv-color-bg-5-active
: For active carousel indicator background usingvar(--tv-base-color-common-9)
(#595959
).--tv-color-bg-inverse-tint-1
: For image toolbar backgrounds usingvar(--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
📒 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 PrefixChanging the display property from the (presumed) previous
inline-block
toinline-flex
along with addingalign-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 usesvar(--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.
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
Style
New Features
Chores