-
Notifications
You must be signed in to change notification settings - Fork 278
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(switch): [switch] Adapting to the SMB theme #2112
Conversation
WalkthroughThe changes introduced in this pull request primarily focus on enhancing the visual representation and functionality of switch components within a Vue application. Updates include modifications to CSS properties for active, inactive, and loading states, the addition of new Vue components to demonstrate these features, and the creation of test cases to validate the loading functionality. A new boolean property for loading status has also been added to the API, improving the overall usability of the component. Changes
Possibly related PRs
Poem
Tip OpenAI O1 model for chat
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Additional comments not posted (1)
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: 0
Outside diff range and nitpick comments (2)
examples/sites/demos/pc/app/switch/loading.spec.ts (1)
3-13
: Test case implementation looks good! Consider translating the name to English for consistency.The test case is well-structured and properly tests the loading state of the switch components. It follows a clear sequence of actions, uses appropriate locators, and makes relevant assertions. Good job on handling page errors as well!
One minor suggestion: If the codebase primarily uses English for test case names, consider translating
加载状态
toLoading State
for consistency.examples/sites/demos/pc/app/switch/loading-composition-api.vue (1)
7-18
: Approved with a suggestion!The script section correctly uses the Composition API, imports the necessary components, defines reactive variables, and implements the
change
method.To enhance the readability and maintainability of the code, consider using more descriptive names for the reactive variables and the
change
method. For example:-const value = ref(true) -const value2 = ref(false) +const switchValue1 = ref(true) +const switchValue2 = ref(false) const loading = ref(true) -function change() { +function toggleLoading() { loading.value = !loading.value }
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (10)
- examples/sites/demos/pc/app/switch/basic-usage.spec.ts (1 hunks)
- examples/sites/demos/pc/app/switch/dynamic-disable.spec.ts (1 hunks)
- examples/sites/demos/pc/app/switch/loading-composition-api.vue (1 hunks)
- examples/sites/demos/pc/app/switch/loading.spec.ts (1 hunks)
- examples/sites/demos/pc/app/switch/loading.vue (1 hunks)
- examples/sites/demos/pc/app/switch/mini-mode.spec.ts (1 hunks)
- examples/sites/demos/pc/app/switch/webdoc/switch.js (1 hunks)
- packages/theme/src/switch/index.less (1 hunks)
- packages/theme/src/switch/vars.less (1 hunks)
- packages/vue/src/switch/src/pc.vue (3 hunks)
Additional comments not posted (19)
examples/sites/demos/pc/app/switch/mini-mode.spec.ts (1)
10-10
: LGTM!The change to the expected CSS width of the switch button from '34px' to '32px' is a minor refinement that aligns the test assertion with the intended visual representation of the component. The test assertion is logically and syntactically correct.
examples/sites/demos/pc/app/switch/dynamic-disable.spec.ts (2)
10-10
: LGTM!The change in the expected background color for the first switch button aligns with the PR objective of adapting the switch component to the SMB theme. The code segment looks good.
11-11
: LGTM!The change in the expected background color for the second switch button aligns with the PR objective of adapting the switch component to the SMB theme. The code segment looks good.
examples/sites/demos/pc/app/switch/basic-usage.spec.ts (2)
10-11
: LGTM!The updated background color and border-top color for the active state of the switch button align with the PR objective of adapting to the SMB theme. The test case correctly asserts the expected CSS properties.
13-14
: LGTM!The updated background color and border-top color for the inactive state of the switch button align with the PR objective of adapting to the SMB theme. The test case correctly asserts the expected CSS properties.
examples/sites/demos/pc/app/switch/loading-composition-api.vue (3)
1-5
: LGTM!The template section is well-structured and correctly uses the
v-model
directive,loading
prop, and@click
event. The usage of theclass
attribute to apply a scoped style is also appropriate.
20-24
: LGTM!The scoped style section correctly applies a right margin to the second switch component using the
.demo-switch
class.
1-24
: Great job on the overall component structure and conventions!The component follows the recommended Vue SFC structure, consistently uses the Composition API, imports and uses external components correctly, defines and uses reactive variables and methods appropriately, and has scoped styles to prevent style leakage. Keep up the good work!
examples/sites/demos/pc/app/switch/loading.vue (3)
1-5
: LGTM!The template section is well-structured and correctly utilizes the
v-model
directive,:loading
prop, and@click
event binding. The usage of thedemo-switch
class for styling is also appropriate.
7-28
: LGTM!The script section is properly structured with correct component imports, registration, and data initialization. The
change
method correctly toggles theloading
state.
30-34
: LGTM!The scoped style for the
demo-switch
class is correctly defined and adds appropriate spacing between the switch components.packages/vue/src/switch/src/pc.vue (5)
22-25
: LGTM!The conditional rendering logic for the text labels has been correctly updated to hide the labels when the component is in the loading state. This change aligns with the goal of enhancing the loading state management.
28-34
: LGTM!The conditional rendering of the loading icon and the dynamic class assignment based on the current value and size of the switch are implemented correctly. This change enhances the visual feedback during the loading state.
35-36
: LGTM!The conditional rendering of the active and inactive icon slots has been correctly updated to hide the icons when the component is in the loading state. This change aligns with the goal of enhancing the loading state management.
50-50
: LGTM!The import statement for the
IconLoadingShadow
component is correct and necessary for using theicon-loading
component in the template.
64-67
: LGTM!The addition of the
loading
prop to the list of exported props and the registration of theIconLoading
component are correct and necessary for the component to accept theloading
prop and use theicon-loading
component in the template. These changes align with the list of alterations provided in the AI-generated summary.packages/theme/src/switch/vars.less (1)
61-66
: LGTM!The addition of these CSS variables for styling the loading state of the switch component looks good. The naming convention is consistent, and the values are leveraging other common variables from the theme. These changes align well with the PR objective of adapting the switch component to the SMB theme.
packages/theme/src/switch/index.less (1)
57-67
: LGTM!The addition of the
&__on-loading
,&__off-loading
, and&__loading-mini
class selectors enhances the visual representation of the switch component during loading states. The use of CSS variables for dynamic styling based on the component's state is a good practice. These changes align with the PR objective of adapting the switch component to the SMB theme and improve the user experience by providing better visual feedback during loading scenarios.The code changes are well-structured and follow the existing coding style of the file.
examples/sites/demos/pc/app/switch/webdoc/switch.js (1)
29-40
: LGTM!The new demo entry for the loading status feature is correctly implemented and follows the existing structure of the demo entries. The addition of this demo enhances the functionality of the switch component by providing users with an example of how to implement a loading state.
The localized names and descriptions are provided in both Chinese and English, which is consistent with the other demo entries. The associated code file
loading.vue
is assumed to contain the implementation of the loading status feature.
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
New Features
Bug Fixes
Style
Documentation