feat(switch): add width property to support custom switch width #3879
Conversation
WalkthroughA new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
packages/renderless/src/switch/vue.ts (1)
50-67: Extract duplicate width transformation logic to reduce code duplication.The width transformation logic (
typeof props.width === 'number' ? \${props.width}px` : props.width) is duplicated in bothswitchStyleanddisplayOnlyStyle`. Consider extracting this to a helper computed property for better maintainability.Apply this refactor:
}), + // 计算 width 值 + computedWidth: computed(() => { + if (!props.width) return null + return typeof props.width === 'number' ? `${props.width}px` : props.width + }), // 添加 switchStyle 计算属性 switchStyle: computed(() => { - if (props.width) { - return { - width: typeof props.width === 'number' ? `${props.width}px` : props.width - } - } - return {} + return state.computedWidth ? { width: state.computedWidth } : {} }), // 添加 displayOnlyStyle 计算属性 displayOnlyStyle: computed(() => { - if (props.width) { - return { - width: typeof props.width === 'number' ? `${props.width}px` : props.width, - display: 'inline-block' - } - } - return {} + return state.computedWidth + ? { width: state.computedWidth, display: 'inline-block' } + : {} })examples/sites/demos/pc/app/switch/width-composition-api.vue (1)
10-10: Consider using lowercase "px" for consistency with CSS conventions.While CSS units are case-insensitive and "150PX" will work, the standard convention is lowercase ("150px"). Consider updating for consistency with typical CSS practices.
- <tiny-switch width="150PX"></tiny-switch> + <tiny-switch width="150px"></tiny-switch>examples/sites/demos/pc/app/switch/width.spec.ts (1)
1-17: LGTM! Basic width functionality is properly tested.The test correctly validates that both numeric and string width values render as expected. The pageerror check is a good practice.
Consider adding edge case tests for more comprehensive coverage (can be deferred to a future PR):
- Different CSS units (em, rem, %, vw)
- Very small/large numeric values
- Invalid string values (to verify graceful handling)
examples/sites/demos/pc/app/switch/width.vue (1)
10-10: Consider using lowercase "px" for consistency with CSS conventions.Similar to the composition API demo, consider using lowercase "px" instead of uppercase "PX" to align with standard CSS notation practices.
- <tiny-switch width="150PX"></tiny-switch> + <tiny-switch width="150px"></tiny-switch>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
examples/sites/demos/apis/switch.js(1 hunks)examples/sites/demos/pc/app/switch/webdoc/switch.js(1 hunks)examples/sites/demos/pc/app/switch/width-composition-api.vue(1 hunks)examples/sites/demos/pc/app/switch/width.spec.ts(1 hunks)examples/sites/demos/pc/app/switch/width.vue(1 hunks)packages/renderless/src/switch/vue.ts(1 hunks)packages/vue/src/switch/src/index.ts(1 hunks)packages/vue/src/switch/src/pc.vue(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: PR E2E Test (pnpm test:e2e3)
🔇 Additional comments (9)
examples/sites/demos/pc/app/switch/width-composition-api.vue (1)
1-29: LGTM! Clear demonstration of width prop usage.The demo effectively shows both numeric and string width types. The composition API pattern is correctly implemented.
examples/sites/demos/pc/app/switch/width.vue (1)
1-35: LGTM! Options API demo is correctly implemented.The demo clearly demonstrates width prop usage with both numeric and string types using the Options API pattern.
packages/vue/src/switch/src/pc.vue (3)
17-17: LGTM! Style binding correctly applies width customization.The style binding to
state.switchStyleproperly integrates the new width prop with the existing switch styling.
40-40: LGTM! Display-only mode style binding is correctly implemented.The style binding to
state.displayOnlyStyleappropriately handles width customization for display-only mode.
66-67: LGTM! Width prop declaration follows component conventions.The width prop is correctly added to the props array, maintaining consistency with the existing prop declaration pattern.
examples/sites/demos/pc/app/switch/webdoc/switch.js (1)
31-42: LGTM! Width demo documentation is properly structured.The demo entry is well-documented with bilingual descriptions and follows the established pattern for demo documentation. The placement between mini-mode and loading demos is logical.
packages/vue/src/switch/src/index.ts (1)
72-73: LGTM! Width prop correctly added to public API.The width prop is properly declared with the correct type signature
[Number, String], allowing both numeric pixel values and string values with units. The syntax change to the loading prop's closing brace is correct.examples/sites/demos/apis/switch.js (1)
114-128: Width prop API documentation is complete and well-structured.The API documentation properly describes the width prop with correct type information (
number | string), bilingual descriptions, appropriate demo references, and version metadata. However, verify that3.28.0is the correct version for this feature release—if this PR targets a different version, update themeta.stablevalue accordingly.packages/renderless/src/switch/vue.ts (1)
59-67: Verify necessity ofdisplay: 'inline-block'for display-only mode.The
displayOnlyStyleaddsdisplay: 'inline-block'when width is specified. Please confirm this is necessary and won't conflict with existing display styles for the display-only span. If the span already has appropriate display styling, this explicit override may be unnecessary.
1 participant
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
widthprop, accepting both numeric values and string values for flexible width configuration.✏️ Tip: You can customize this high-level summary in your review settings.