refactor(base-select): [base-select] remove tree/grid code from base-select #2522
Conversation
WalkthroughThe changes in this pull request involve modifications to the Changes
Possibly related PRs
Suggested reviewers
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 (
|
kagol
changed the title
|
[e2e-test-warn] The title of the Pull request should look like "fix(vue-renderless): [action-menu, alert] fix xxx bug". Please make sure you've read our contributing guide |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
examples/sites/demos/pc/app/base-select/size.spec.ts(1 hunks)packages/renderless/src/base-select/index.ts(0 hunks)packages/renderless/src/base-select/vue.ts(0 hunks)
💤 Files with no reviewable changes (2)
- packages/renderless/src/base-select/index.ts
- packages/renderless/src/base-select/vue.ts
| @@ -54,5 +54,5 @@ test('mini 尺寸', async ({ page }) => { | |||
|
|
|||
| await expect(input).toHaveClass(/tiny-input-mini/) | |||
| await expect(tag.nth(0)).toHaveClass(/tiny-tag--mini tiny-tag--light/) | |||
| expect(height).toBeCloseTo(27, 1) | |||
| expect(height).toBeCloseTo(28, 1) | |||
There was a problem hiding this comment.
💡 Codebase verification
Based on the shell script results, I can now make a definitive assessment. The results show that across different components:
-
Mini size consistently has 24px height:
- Input:
await expect(mini).toHaveCSS('height', '24px') - Time Select:
await expect(miniBox).toHaveCSS('height', '24px') - Autocomplete:
await expect(miniSize).toHaveCSS('height', '24px') - Cascader:
await expect(page.locator('.tiny-cascader.tiny-cascader--mini input')).toHaveCSS('height', '24px')
- Input:
-
Small size consistently has 28px height:
- Input:
await expect(small).toHaveCSS('height', '28px') - Time Select:
await expect(smallBox).toHaveCSS('height', '28px') - Autocomplete:
await expect(smallSize).toHaveCSS('height', '28px') - Cascader:
await expect(page.locator('.tiny-cascader.tiny-cascader--small input')).toHaveCSS('height', '28px')
- Input:
Height assertion for mini size should be 24px, not 28px
The mini size height assertion of 28px in base-select is inconsistent with the established pattern across other components where mini size is consistently 24px. This appears to be an error in the test case rather than an intentional design change.
examples/sites/demos/pc/app/base-select/size.spec.ts: Change the mini size height assertion from 28px to 24px to maintain consistency with other components
🔗 Analysis chain
Verify if mini and small sizes should have the same height
The height assertion for mini size (28px) is now identical to the small size variant. This could indicate:
- A bug fix to align with correct specifications
- An intentional design change to make mini same height as small
- A potential issue if mini and small sizes shouldn't be identical
If this is an intentional change, please ensure it's documented in the component's specifications.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for size-related specifications or documentation
# Check if there's any documentation about component sizes
fd -e md -e vue . | xargs rg -l "(?i)(size|mini|small).*(height|dimension)"
# Look for size-related constants or theme variables
fd -e ts -e js . | xargs rg "(mini|small).*height"
# Search for other test files that might contain size assertions
fd spec.ts$ . | xargs rg "toBeCloseTo.*28"
Length of output: 3756
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
Refactor
Tests