test(tree-select): [tree-select] add e2e test #2540
Conversation
WalkthroughThis pull request includes updates to various Vue components and their associated test cases for a tree-select feature. Key changes involve modifying initial state values in several components, enhancing test cases for better specificity, and introducing new tests to cover additional functionalities such as collapsible tags and size variations. Additionally, improvements were made to the base-select component's handling of user interactions and state management. Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (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 (
|
kagol
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Outside diff range and nitpick comments (34)
examples/sites/demos/pc/app/tree-select/disabled.spec.ts (3)
3-5: Consider using English for test descriptionsThe test description is in Chinese ('禁用'). For better maintainability and consistency, consider using English descriptions.
-test('禁用', async ({ page }) => { +test('should be disabled', async ({ page }) => {
7-9: Enhance selector specificity and reliabilityThe current selectors could be more robust:
- Using
nth(0)might be fragile if more elements are added- Consider using more specific selectors or data-testid attributes
- const select = wrap.locator('.tiny-tree-select').nth(0) + const select = wrap.locator('[data-testid="disabled-tree-select"]')
1-13: Consider adding more test scenariosThe current test file only covers the basic disabled state. Consider adding tests for:
- Dynamically toggling disabled state
- Disabled state with pre-selected values
- Partially disabled tree nodes
- Keyboard navigation in disabled state
Would you like me to provide example test implementations for these scenarios?
examples/sites/demos/pc/app/tree-select/native-properties.spec.ts (1)
3-5: Consider enhancing error handling and test namingWhile the basic setup is good, consider these improvements:
- The error handler could be more specific about the type of errors it's catching
- Consider using English test descriptions for consistency across the codebase
-test('原生属性', async ({ page }) => { +test('native properties - should handle input attributes correctly', async ({ page }) => { - page.on('pageerror', (exception) => expect(exception).toBeNull()) + page.on('pageerror', (error) => { + console.error('Page error:', error); + expect(error).toBeNull(); + })examples/sites/demos/pc/app/tree-select/map-field.spec.ts (1)
1-4: Consider enhancing error handling and test naming.While the basic setup is good, consider these improvements:
- Add more specific error handling to catch and validate specific exceptions rather than just expecting null
- Consider using English test names for better international collaboration, or add English comments explaining the test purpose
-test('映射字段', async ({ page }) => { +// Test: Field mapping functionality for tree-select component +test('tree-select-field-mapping', async ({ page }) => { // Add specific error types you want to catch - page.on('pageerror', (exception) => expect(exception).toBeNull()) + page.on('pageerror', (exception) => { + console.error('Page error:', exception); + expect(exception.message).not.toContain('TypeError'); + expect(exception.message).not.toContain('ReferenceError'); + })examples/sites/demos/pc/app/tree-select/basic-usage.spec.ts (4)
3-11: Consider using English test names for better international collaboration.While the test setup is good, consider using English test names to make the tests more accessible to a broader audience. Additionally, consider using data-testid attributes for more reliable element selection.
-test('下拉树单选', async ({ page }) => { +test('tree-select single selection', async ({ page }) => {Also, consider adding data-testid attributes to your component:
<tiny-tree-select data-testid="basic-tree-select">Then update the locators:
-const select = wrap.locator('.tiny-tree-select').nth(0) +const select = wrap.locator('[data-testid="basic-tree-select"]')
14-16: Enhance test coverage for the suffix icon.While the basic assertions are good, consider adding:
- Interaction tests (clicking the suffix icon should toggle the dropdown)
- State changes (icon rotation on dropdown open/close)
- Negative test cases
// Add these test cases after the existing assertions await suffixSvg.click() await expect(dropdown).toBeVisible() await expect(suffixSvg).toHaveClass(/is-reverse/) // Assuming the icon rotates await suffixSvg.click() await expect(dropdown).toBeHidden() await expect(suffixSvg).not.toHaveClass(/is-reverse/)
Line range hint
19-24: Add comprehensive test scenarios for tree-select behavior.The current test covers basic selection, but consider adding:
- Loading state handling
- Keyboard navigation (Arrow keys, Enter, Escape)
- Empty state handling
- Search/filter functionality
// Add keyboard navigation tests await input.press('ArrowDown') await expect(treeNode.first()).toHaveClass(/is-current/) await input.press('Enter') await expect(input).toHaveValue('一级 1') // Add search functionality test await input.click() await input.type('2-1') await expect(treeNode).toHaveCount(1) await expect(treeNode).toHaveText(/二级 2-1/)
Line range hint
1-24: Ensure test isolation and alignment with PR objectives.While the test improvements are good, consider:
- Adding
test.describeblocks to group related test cases- Using
test.beforeEachfor common setup- Adding more test cases to fully cover the tree-select functionality mentioned in the PR objectives
test.describe('TreeSelect Component', () => { test.beforeEach(async ({ page }) => { page.on('pageerror', (exception) => expect(exception).toBeNull()) await page.goto('tree-select#basic-usage') }) test('single selection behavior', async ({ page }) => { // Current test code }) test('keyboard navigation', async ({ page }) => { // New keyboard test }) test('search functionality', async ({ page }) => { // New search test }) })examples/sites/demos/pc/app/tree-select/multiple.spec.ts (1)
1-5: Consider enhancing test setup for better maintainabilityA few suggestions to improve the test setup:
- Consider using English for test names to maintain consistency across international development teams
- The error handler could be more specific about the types of errors it expects to catch
-test('下拉树多选', async ({ page }) => { - page.on('pageerror', (exception) => expect(exception).toBeNull()) +test('tree-select multiple selection functionality', async ({ page }) => { + page.on('pageerror', (error) => { + console.error('Page error:', error); + expect(error).toBeNull(); + })examples/sites/demos/pc/app/tree-select/copy.spec.ts (2)
3-16: Consider making the test more resilientWhile the test is well-structured following the AAA pattern, there are a few suggestions for improvement:
test('多选一键复制所有标签', async ({ page }) => { await page.goto('tree-select#copy') const wrap = page.locator('#copy') const select = wrap.locator('.tiny-tree-select').nth(0) const copyValueInput = wrap.locator('.copy-value .tiny-input__inner') const copyIcon = select.locator('.tiny-base-select__copy .tiny-svg') await select.hover() await expect(copyIcon).toBeVisible() await copyIcon.click() + try { await copyValueInput.press('Control+V') - await expect(copyValueInput).toHaveValue('三级 1-1-1,二级 2-2') + await expect(copyValueInput).toHaveValue(expect.stringMatching(/[\S\s]+,[\S\s]+/)) + } catch (error) { + throw new Error(`Failed to verify copied content: ${error.message}`) + } })Consider:
- Using a more flexible assertion that doesn't rely on specific text content
- Adding error handling for clipboard operations
- Adding a comment explaining the expected format of the copied content
1-30: Consider architectural improvements for test maintainabilityTo improve the test suite's maintainability:
- Consider extracting common selectors into a shared configuration:
const selectors = { wrapper: '#copy', treeSelect: '.tiny-tree-select', copyIcon: '.tiny-base-select__copy .tiny-svg', copyValueInput: '.copy-value .tiny-input__inner' } as const;
- Consider creating a test helper for common operations:
async function performCopyOperation(page, selectIndex: number) { const wrap = page.locator(selectors.wrapper); const select = wrap.locator(selectors.treeSelect).nth(selectIndex); const copyValueInput = wrap.locator(selectors.copyValueInput); const copyIcon = select.locator(selectors.copyIcon); await select.waitFor({ state: 'visible' }); await select.hover(); await copyIcon.click(); await copyValueInput.press('Control+V'); return copyValueInput; }This would make the tests more DRY and easier to maintain.
examples/sites/demos/pc/app/tree-select/copy-composition-api.vue (1)
16-16: Add a comment explaining the initial selection.The initial value
[9, 6]selects specific nodes from different levels of the tree. Consider adding a comment to explain why these values were chosen for the demo.-const value = ref([9, 6]) +// Initialize with "三级 1-1-1" and "二级 2-2" to demonstrate multi-level selection +const value = ref([9, 6])examples/sites/demos/pc/app/tree-select/size-composition-api.vue (1)
18-18: LGTM! Consider adding a comment to explain the test scenario.The initialization with specific values
[9, 6]is good for testing as it covers both multi-level and multiple selection scenarios across different size variants.Consider adding a comment to explain the test scenario:
+// Initialize with multi-level selections to test rendering across different sizes +// 9: "三级 1-1-1" (third level) +// 6: "二级 2-2" (second level) const value = ref([9, 6])examples/sites/demos/pc/app/tree-select/reference-style-composition-api.vue (1)
25-25: LGTM! Consider adding a comment to explain the pre-selected value.The initialization of
value2with[9]provides a good demonstration of the pre-selected state and ensures a consistent initial state for e2e testing. Consider adding a comment to explain why this specific value was chosen.-const value2 = ref([9]) +// Pre-select "三级 1-1-1" to demonstrate multiple selection with tag-type +const value2 = ref([9])examples/sites/demos/pc/app/tree-select/panel-style.spec.ts (3)
3-13: Consider enhancing test robustness and maintainabilityWhile the basic test structure is good, here are some suggested improvements:
- Consider using English for test titles for better international collaboration
- The error handling could be more specific rather than catching all page errors
- Add cleanup after the test to ensure a clean state
- Add more assertions to verify the mounting behavior comprehensively
Here's a suggested improvement:
-test('不挂载在 body 元素上', async ({ page }) => { +test('panel should not mount on body element', async ({ page }) => { page.on('pageerror', (exception) => expect(exception).toBeNull()) await page.goto('tree-select#panel-style') const wrap = page.locator('#panel-style') const select = wrap.locator('.tiny-tree-select').nth(0) const dropdown = select.locator('.tiny-base-select__tags-group > .tiny-select-dropdown') await select.click() await expect(dropdown).toHaveCount(1) + // Additional assertions + await expect(dropdown).toBeVisible() + await expect(page.locator('body > .tiny-select-dropdown')).toHaveCount(0) + + // Cleanup + await select.click() // Close the dropdown })
28-38: Enhance panel placement test coverageThe test could be improved with additional assertions and setup:
- Use English for the test title
- Add viewport setup to ensure consistent behavior
- Add visual verification
- Include cleanup
Here's a suggested improvement:
-test('从上方弹出面板', async ({ page }) => { +test('should display dropdown panel from top', async ({ page }) => { + // Set viewport size to ensure consistent behavior + await page.setViewportSize({ width: 1280, height: 720 }) + page.on('pageerror', (exception) => expect(exception).toBeNull()) await page.goto('tree-select#panel-style') const wrap = page.locator('#panel-style') const select = wrap.locator('.tiny-tree-select').nth(2) const dropdown = page.locator('body > .tiny-select-dropdown') await select.click() await expect(dropdown).toHaveAttribute('x-placement', 'top') + + // Additional visual verification + await expect(dropdown).toBeVisible() + const dropdownBox = await dropdown.boundingBox() + const selectBox = await select.boundingBox() + expect(dropdownBox.y + dropdownBox.height).toBeLessThanOrEqual(selectBox.y) + + // Cleanup + await select.click() // Close the dropdown })
1-38: Consider global test improvementsWhile individual tests are functional, the test suite could benefit from some global improvements:
- Consider extracting common setup into a test fixture
- Add beforeEach/afterEach hooks for common operations
- Consider implementing a shared error handling mechanism
Here's an example of how to implement these improvements:
import { test as base, expect } from '@playwright/test' // Custom test fixture type TreeSelectTest = { openDropdown: (selector: string) => Promise<void> closeDropdown: (selector: string) => Promise<void> } const test = base.extend<TreeSelectTest>({ openDropdown: async ({ page }, use) => { await use(async (selector) => { const select = page.locator(selector) await select.click() await page.waitForSelector('.tiny-select-dropdown') }) }, closeDropdown: async ({ page }, use) => { await use(async (selector) => { const select = page.locator(selector) await select.click() await page.waitForSelector('.tiny-select-dropdown', { state: 'hidden' }) }) } }) test.beforeEach(async ({ page }) => { page.on('pageerror', (exception) => expect(exception).toBeNull()) await page.setViewportSize({ width: 1280, height: 720 }) await page.goto('tree-select#panel-style') }) test.afterEach(async ({ page }) => { // Cleanup code here })examples/sites/demos/pc/app/tree-select/collapse-tags.spec.ts (3)
1-11: Consider enhancing test robustness and error handling.While the basic setup is good, consider these improvements:
- Add specific error handling instead of just expecting null
- Use more robust selectors (e.g., data-testid) instead of class-based selectors
- Configure timeouts for potential slow operations
import { test, expect } from '@playwright/test' test('折叠标签 collapse-tags', async ({ page }) => { - page.on('pageerror', (exception) => expect(exception).toBeNull()) + page.on('pageerror', (exception) => { + console.error('Page error:', exception); + expect(exception).toBeNull(); + }) await page.goto('tree-select#collapse-tags') const wrap = page.locator('#collapse-tags') - const select = wrap.locator('.tiny-tree-select').nth(0) + const select = wrap.locator('[data-testid="tree-select"]').first() const dropdown = page.locator('body > .tiny-select-dropdown') const tag = select.locator('.tiny-tag') const treeNode = dropdown.locator('.tiny-tree-node') const checkedTreeNodes = dropdown.locator('.tiny-tree-node.is-checked') + + // Configure timeouts for potential slow operations + await page.waitForSelector('#collapse-tags', { timeout: 5000 })
13-17: Enhance default state verification coverage.The initial state verification could be more comprehensive by checking:
- The exact text content of all visible tags
- The state of the dropdown (should be closed)
- The total number of available options
// 验证默认值的折叠标签显示 await expect(tag).toHaveCount(2) await expect(tag.filter({ hasText: '三级 1-1-1' })).toBeVisible() await expect(tag.filter({ hasText: '+ 1' })).toBeVisible() + // Verify dropdown is closed initially + await expect(dropdown).toBeHidden() + // Verify all tag texts + const tagTexts = await tag.allTextContents() + expect(tagTexts).toEqual(['三级 1-1-1', '+ 1'])
1-34: Consider adding tests for additional scenarios.The test file would benefit from additional test cases:
- Error handling (e.g., network issues during selection)
- Edge cases (e.g., maximum selection limit)
- Keyboard navigation
- Screen reader accessibility
Would you like me to provide example test cases for these scenarios?
examples/sites/demos/pc/app/tree-select/copy.vue (2)
4-4: Consider using separate v-model bindings for each tree-selectBoth tree-select components are bound to the same
valueproperty, which means they'll always show the same selection. This might not be the best practice for demonstrating independent copy scenarios.Consider using separate bindings:
-<tiny-tree-select v-model="value" :tree-op="treeOp" multiple copyable></tiny-tree-select> +<tiny-tree-select v-model="value1" :tree-op="treeOp" multiple copyable></tiny-tree-select> -<tiny-tree-select v-model="value" :tree-op="treeOp" multiple copyable text-split="/"></tiny-tree-select> +<tiny-tree-select v-model="value2" :tree-op="treeOp" multiple copyable text-split="/"></tiny-tree-select>Also applies to: 6-6
22-22: Enhance test coverage with more diverse initial selectionsThe current initial selection
[9, 6]is good but could be enhanced to better demonstrate copy functionality.Consider selecting nodes that:
- Have special characters in labels
- Include parent-child relationships
- Have longer labels to test text wrapping
Example:
- value: [9, 6], + value: [9, 6, 4], // Adding "二级 1-1" to test parent-child label copyingexamples/sites/demos/pc/app/tree-select/size.vue (1)
23-23: Consider adding comments to document the selected valuesTo improve maintainability, consider adding a comment explaining which tree nodes these values correspond to:
+ // Pre-selected values: [9: "三级 1-1-1", 6: "二级 2-2"] value: [9, 6],examples/sites/demos/pc/app/tree-select/reference-style.spec.ts (3)
3-15: Consider improving test maintainability and readabilityA few suggestions to enhance the test:
- Use English for test titles to maintain consistency with standard practices
- Extract the SVG path into a constant to improve maintainability
- Consider using test-id attributes for more reliable element selection
+const CUSTOM_DROPDOWN_ICON_PATH = 'M2.3 7.78v.08c.02.65.561 1.17 1.221 1.17.68 0 1.229-.54 1.229-1.21 0-.672-.549-1.21-1.229-1.21-.66 0-1.2.519-1.22 1.17Zm4.241.04c0 .67.54 1.21 1.22 1.21.68 0 1.23-.54 1.23-1.21 0-.672-.55-1.21-1.23-1.21-.68 0-1.22.538-1.22 1.21Zm4.24.04v-.08c.02-.651.559-1.17 1.219-1.17.682 0 1.23.538 1.23 1.21 0 .67-.548 1.21-1.23 1.21-.66 0-1.2-.52-1.219-1.17Z' -test('自定义下拉图标', async ({ page }) => { +test('custom dropdown icon should have correct SVG path', async ({ page }) => { page.on('pageerror', (exception) => expect(exception).toBeNull()) await page.goto('tree-select#reference-style') const wrap = page.locator('#reference-style') - const select = wrap.locator('.tiny-tree-select').nth(0) + const select = wrap.locator('.tiny-tree-select').first() const suffixIconPath = select.locator('.tiny-input__suffix .tiny-base-select__caret path') await expect(suffixIconPath).toHaveAttribute( 'd', - 'M2.3 7.78v.08c.02.65.561 1.17 1.221 1.17.68 0 1.229-.54 1.229-1.21 0-.672-.549-1.21-1.229-1.21-.66 0-1.2.519-1.22 1.17Zm4.241.04c0 .67.54 1.21 1.22 1.21.68 0 1.23-.54 1.23-1.21 0-.672-.55-1.21-1.23-1.21-.68 0-1.22.538-1.22 1.21Zm4.24.04v-.08c.02-.651.559-1.17 1.219-1.17.682 0 1.23.538 1.23 1.21 0 .67-.548 1.21-1.23 1.21-.66 0-1.2-.52-1.219-1.17Z' + CUSTOM_DROPDOWN_ICON_PATH ) })
17-27: Enhance tag type test coverage and readabilityThe test could be improved by:
- Using English for test title and comments
- Adding assertions for other important tag properties
- Using more specific selectors
-test('标签类型', async ({ page }) => { +test('tag should have warning type style', async ({ page }) => { page.on('pageerror', (exception) => expect(exception).toBeNull()) await page.goto('tree-select#reference-style') const wrap = page.locator('#reference-style') const select = wrap.locator('.tiny-tree-select').nth(1) const tag = select.locator('.tiny-tag') - // 验证是否有对应类型的类名 + // Verify tag styling await expect(tag.nth(0)).toHaveClass(/tiny-tag--warning/) + // Additional assertions for comprehensive testing + await expect(tag.nth(0)).toBeVisible() + await expect(tag.nth(0)).toHaveCSS('background-color', 'rgb(255, 251, 230)') })
1-2: Consider adding test utilities and shared setupTo improve maintainability and reduce duplication, consider:
- Creating a test fixture for common setup
- Extracting shared selectors into constants
- Adding utility functions for common operations
import { test as base, expect } from '@playwright/test' +const TEST_SELECTORS = { + wrapper: '#reference-style', + treeSelect: '.tiny-tree-select', + dropdown: 'body > .tiny-select-dropdown', + input: '.tiny-input__inner' +} + +const test = base.extend({ + treeSelect: async ({ page }, use) => { + await page.goto('tree-select#reference-style') + const component = { + getSelect: (index: number) => page.locator(TEST_SELECTORS.treeSelect).nth(index), + getDropdown: () => page.locator(TEST_SELECTORS.dropdown), + waitForDropdown: async () => await page.locator(TEST_SELECTORS.dropdown).waitFor({ state: 'visible' }) + } + await use(component) + } +})examples/sites/demos/pc/app/tree-select/size.spec.ts (3)
3-16: Consider improvements for test maintainability and clarity.A few suggestions to enhance the test:
- Use English for test titles for consistency
- Add more context to error handling
- Consider using data-testid attributes instead of CSS classes for more stable selectors
- Document or reference the expected height values
-test('medium 尺寸', async ({ page }) => { +test('medium size variant', async ({ page }) => { - page.on('pageerror', (exception) => expect(exception).toBeNull()) + page.on('pageerror', (exception) => { + console.error('Page error occurred:', exception); + expect(exception).toBeNull(); + }) await page.goto('tree-select#size') const wrap = page.locator('#size') - const select = wrap.locator('.tiny-tree-select').nth(0) + const select = wrap.locator('[data-testid="tree-select-medium"]') const input = select.locator('.tiny-input') const tag = select.locator('.tiny-tag') await expect(input).toHaveClass(/tiny-input-medium/) + // Medium size input height should be 40px as per design system await expect(input.locator('.tiny-input__inner')).toHaveCSS('height', '40px') await expect(tag.nth(0)).toHaveClass(/tiny-tag--medium tiny-tag--light/) })
18-29: Align test implementation with other size variants.For consistency with small/mini size tests:
- Use English test title
- Add boundingBox check for more precise height verification
-test('默认尺寸', async ({ page }) => { +test('default size variant', async ({ page }) => { page.on('pageerror', (exception) => expect(exception).toBeNull()) await page.goto('tree-select#size') const wrap = page.locator('#size') const select = wrap.locator('.tiny-tree-select').nth(1) const input = select.locator('.tiny-input') const tag = select.locator('.tiny-tag') + const { height } = await input.locator('.tiny-input__inner').boundingBox() - await expect(input.locator('.tiny-input__inner')).toHaveCSS('height', '32px') + expect(height).toBeCloseTo(32, 1) await expect(tag.nth(0)).toHaveClass(/tiny-tag--light/) })
31-59: Reduce duplication and improve clarity in small/mini size tests.Consider these improvements:
- Extract common height values and precision to constants
- Use English test titles consistently
- Consider extracting common test setup logic
+const SIZES = { + SMALL: 28, + MINI: 28, + HEIGHT_PRECISION: 1 +}; -test('small 尺寸', async ({ page }) => { +test('small size variant', async ({ page }) => { // ... existing code ... - expect(height).toBeCloseTo(28, 1) + expect(height).toBeCloseTo(SIZES.SMALL, SIZES.HEIGHT_PRECISION) }) test('mini size variant', async ({ page }) => { // ... existing code ... - expect(height).toBeCloseTo(28, 1) + expect(height).toBeCloseTo(SIZES.MINI, SIZES.HEIGHT_PRECISION) })packages/renderless/src/base-select/index.ts (4)
Line range hint
437-443: Handle potentialNaNvalues in padding calculationWhen calculating
state.inputPaddingRight,parseFloatis used onwidth,marginLeft, andmarginRight. If any of these values are not numeric, it could result inNaN, causing layout issues. Consider adding validation or default values to ensure robust computation.Apply this diff to enhance robustness:
const widthValue = parseFloat(width) || 0 const marginLeftValue = parseFloat(marginLeft) || 0 const marginRightValue = parseFloat(marginRight) || 0 vm.$refs.tags.style.paddingRight = `${Math.ceil(widthValue + marginLeftValue + marginRightValue)}px`
Line range hint
609-610: Potential race condition withstate.isClickChooseThe asynchronous call to
api.initQuery({ init: true }).then(() => api.setSelected())relies on!state.isClickChoose. Sincestate.isClickChooseis set tofalseimmediately after, there may be a race condition wherestate.isClickChoosechanges before the promise resolves. Consider moving the assignment ofstate.isClickChoose = falseinside thethenblock to ensure proper sequencing.Apply this diff to fix the potential race condition:
if (!state.isClickChoose) { api.initQuery({ init: true }).then(() => { api.setSelected() + state.isClickChoose = false }) - state.isClickChoose = false }
Line range hint
1306-1310: Safely compare array lengths inclearNoMatchValueWhen comparing
props.modelValue.lengthandnewModelValue.length, ensure that bothprops.modelValueandnewModelValueare arrays to avoid undefined property access errors. UseArray.isArrayto validate the variables before accessing theirlengthproperties.Apply this diff to ensure safe array handling:
if ( + (props.multiple && Array.isArray(props.modelValue) && Array.isArray(newModelValue) && props.modelValue.length !== newModelValue.length) || - (props.multiple && props.modelValue.length !== newModelValue.length) || (!props.multiple && props.modelValue !== newModelValue) ) { emit('update:modelValue', newModelValue) }
1873-1875: Avoid hardcoding icon width for better responsivenessThe
COPY_ICON_WIDTHis set to a fixed value of16. For better responsiveness and to accommodate different icon sizes, consider calculating the width dynamically or using CSS to handle spacing, ensuring consistency across various screen sizes and devices.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (20)
examples/sites/demos/pc/app/tree-select/basic-usage-composition-api.vue(1 hunks)examples/sites/demos/pc/app/tree-select/basic-usage.spec.ts(1 hunks)examples/sites/demos/pc/app/tree-select/collapse-tags.spec.ts(1 hunks)examples/sites/demos/pc/app/tree-select/copy-composition-api.vue(1 hunks)examples/sites/demos/pc/app/tree-select/copy.spec.ts(1 hunks)examples/sites/demos/pc/app/tree-select/copy.vue(2 hunks)examples/sites/demos/pc/app/tree-select/disabled-composition-api.vue(1 hunks)examples/sites/demos/pc/app/tree-select/disabled.spec.ts(1 hunks)examples/sites/demos/pc/app/tree-select/disabled.vue(1 hunks)examples/sites/demos/pc/app/tree-select/map-field.spec.ts(1 hunks)examples/sites/demos/pc/app/tree-select/multiple.spec.ts(1 hunks)examples/sites/demos/pc/app/tree-select/native-properties.spec.ts(1 hunks)examples/sites/demos/pc/app/tree-select/panel-style.spec.ts(1 hunks)examples/sites/demos/pc/app/tree-select/reference-style-composition-api.vue(1 hunks)examples/sites/demos/pc/app/tree-select/reference-style.spec.ts(1 hunks)examples/sites/demos/pc/app/tree-select/reference-style.vue(1 hunks)examples/sites/demos/pc/app/tree-select/size-composition-api.vue(1 hunks)examples/sites/demos/pc/app/tree-select/size.spec.ts(1 hunks)examples/sites/demos/pc/app/tree-select/size.vue(1 hunks)packages/renderless/src/base-select/index.ts(1 hunks)
🔇 Additional comments (24)
examples/sites/demos/pc/app/tree-select/disabled.spec.ts (1)
1-2: LGTM!
The imports are minimal and appropriate for the test requirements.
examples/sites/demos/pc/app/tree-select/native-properties.spec.ts (3)
1-2: LGTM: Clean and minimal imports
The imports are correctly specified and include only the necessary testing utilities.
7-9: LGTM: Well-structured element selection
The locator chains are clear, maintainable, and follow Playwright's best practices.
11-15: Enhance attribute testing coverage
While the basic attribute testing is good, consider these improvements:
- Verify the actual value of the autocomplete attribute
- Add tests for other common native attributes (e.g., disabled, readonly)
Here's a suggested enhancement:
await expect(input).toHaveAttribute('name', 'inputName')
await expect(input).toHaveAttribute('placeholder', '请选择一个选项')
-const isHasAutocomplete = await input.evaluate((input) => input.hasAttribute('autocomplete'))
-await expect(isHasAutocomplete).toBe(true)
+await expect(input).toHaveAttribute('autocomplete', 'on')
+
+// Add tests for other native attributes
+await expect(input).not.toHaveAttribute('readonly')
+await expect(input).not.toHaveAttribute('disabled')examples/sites/demos/pc/app/tree-select/map-field.spec.ts (1)
1-18: Verify test consistency across the codebase.
Let's ensure this test follows the established patterns and doesn't duplicate existing tests.
✅ Verification successful
Test follows established patterns and is correctly implemented
The test follows consistent patterns seen across the codebase:
- Uses similar selector patterns (
toHaveClass(/is-current/)) as found in other tree-related components - Matches the testing approach used in
basic-usage.spec.tsfor the same component - No duplicate test cases found for the "映射字段" (map field) functionality
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for similar test patterns and potential duplicates
# Look for similar tree-select test files
echo "Similar test files:"
fd "tree-select.*spec"
# Check for similar test patterns
echo "\nSimilar test patterns:"
rg -A 2 "test\(.*tree-select.*mapping"
# Look for consistent testing patterns
echo "\nTest patterns in other components:"
rg "toHaveClass\(/is-current/" --type ts
Length of output: 2613
examples/sites/demos/pc/app/tree-select/basic-usage-composition-api.vue (2)
Line range hint 1-54: Implementation follows Vue Composition API best practices.
The component implementation is well-structured:
- Proper use of
<script setup>syntax - Correct import of dependencies
- Clean template with v-model binding
- Well-organized tree data structure
9-9: Verify the intention behind changing the default selected value.
The default value has been changed from 4 (二级 1-1) to 10 (三级 1-1-2), moving the selection from a level 2 node to a level 3 node. Since this file serves as a demo for basic usage and is referenced in e2e tests, please confirm if this change is intentional and aligns with the test requirements.
Let's check if this change is consistent with other demo files and test expectations:
✅ Verification successful
Based on the test file content, I can now generate the final response:
The default value change to 10 is intentional and verified by tests.
The e2e test for basic usage explicitly expects and verifies that the initial value displays "三级 1-1-2" (value 10) in the input field. The test includes assertions:
await expect(input).toHaveValue('三级 1-1-2')for the initial state- Verifies the node with value 10 has the
is-currentclass
The change from value 4 to 10 is properly covered by tests and appears to be an intentional update to demonstrate selection of a deeper tree node in the basic usage example.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistency in default values across demo files and tests
# Search for other tree-select demo files with default values
echo "Searching for default values in other demo files:"
rg -B 2 -A 2 "const value = ref\(" "examples/sites/demos/pc/app/tree-select/"
# Search for related test assertions
echo "Searching for test assertions related to default value:"
rg -B 2 -A 2 "value.*10" "test/unit/tree-select/"
Length of output: 4498
Script:
#!/bin/bash
# Let's check the e2e tests and webspecs for tree-select to understand if this change affects any tests
# Search for tree-select e2e test files
echo "Searching for tree-select e2e test files:"
fd "tree-select.*spec" "examples/sites"
# If found, check their content for value assertions
echo -e "\nSearching for value assertions in e2e tests:"
rg -B 2 -A 2 "value.*[49]|value.*10" "examples/sites" --type spec
# Check webspecs as well
echo -e "\nSearching for tree-select webspecs:"
fd "tree-select.*spec" "test"
# Check demo files for any comments about the default value
echo -e "\nChecking demo files for comments about default value:"
rg -B 2 -A 2 "默认|default.*value" "examples/sites/demos/pc/app/tree-select/"
Length of output: 5309
Script:
#!/bin/bash
# Let's check the basic usage test file and webdoc to understand the intended behavior
# Check basic usage test file
echo "Checking basic usage test file:"
fd "basic-usage.*spec" "examples/sites/demos/pc/app/tree-select/" --exec cat {}
# Check the webdoc for basic usage description
echo -e "\nChecking webdoc for basic usage description:"
rg -B 2 -A 2 "basic-usage" "examples/sites/demos/pc/app/tree-select/webdoc/tree-select.js"
# Check the tree data structure to understand the node values
echo -e "\nChecking tree data structure in basic usage:"
rg -A 20 "const treeOp = ref\({" "examples/sites/demos/pc/app/tree-select/basic-usage-composition-api.vue"
Length of output: 1835
examples/sites/demos/pc/app/tree-select/multiple.spec.ts (1)
1-29: Consider organizing tests into describe blocks
To improve test organization and maintainability, consider grouping related test cases into describe blocks. This will make it easier to add more test cases in the future and provide better test output.
import { test, expect } from '@playwright/test'
test.describe('TreeSelect Multiple Selection', () => {
test.beforeEach(async ({ page }) => {
page.on('pageerror', (error) => {
console.error('Page error:', error);
expect(error).toBeNull();
})
await page.goto('tree-select#multiple')
})
test('should handle basic multiple selection', async ({ page }) => {
// Current test implementation
})
test('should handle deselection', async ({ page }) => {
// New test case for deselection
})
test('should handle keyboard navigation', async ({ page }) => {
// New test case for keyboard navigation
})
})examples/sites/demos/pc/app/tree-select/copy.spec.ts (1)
1-2: LGTM: Imports are clean and appropriate
The imports are minimal and contain exactly what's needed for the test cases.
examples/sites/demos/pc/app/tree-select/disabled-composition-api.vue (1)
18-18: Verify selected node values and enhance code documentation.
The selected values [9, 6] correspond to nodes "三级 1-1-1" and "二级 2-2" respectively. While these nodes exist in the tree structure, consider adding comments to explain the selected values for better maintainability.
-const value3 = ref([9, 6])
+// Selected nodes: "三级 1-1-1" (9) and "二级 2-2" (6)
+const value3 = ref([9, 6])Let's verify that these nodes exist in the tree structure:
✅ Verification successful
Selected node values are valid and consistent with the tree structure
The verification confirms that both selected values [9, 6] exist in the tree structure of the component:
- Value
9corresponds to node "三级 1-1-1" (level 3) - Value
6corresponds to node "二级 2-2" (level 2)
The original suggestion to add comments for better maintainability still stands:
-const value3 = ref([9, 6])
+// Selected nodes: "三级 1-1-1" (9) and "二级 2-2" (6)
+const value3 = ref([9, 6])🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the selected node values (9 and 6) exist in the tree structure
# Expected: Both values should be found in the tree data structure
# Search for value: 9 (三级 1-1-1)
echo "Searching for node with value 9:"
rg "value: 9," -A 1
# Search for value: 6 (二级 2-2)
echo "Searching for node with value 6:"
rg "value: 6," -A 1
Length of output: 7723
examples/sites/demos/pc/app/tree-select/copy-composition-api.vue (1)
4-4: LGTM! Verify copy functionality with e2e tests.
The template changes look good:
- Consistent v-model binding using single
valueref - Clear demonstration of two copy scenarios (all tags and custom separator)
- Added styling class for better visual consistency
Let's verify if e2e tests cover these copy scenarios:
Also applies to: 6-6, 8-8
✅ Verification successful
Copy functionality is well covered by e2e tests
The template changes are properly verified with comprehensive e2e tests that cover both scenarios:
- Test for copying all tags with default separator:
'三级 1-1-1,二级 2-2' - Test for copying with custom text separator:
'三级 1-1-1/二级 2-2'
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if e2e tests exist for copy functionality
# Look for copy-related test files
fd "copy.spec" --type f
# Search for copy-related test cases
rg -A 5 "describe.*copy" --type ts
Length of output: 113
Script:
#!/bin/bash
# Let's examine the content of the copy.spec.ts file
cat examples/sites/demos/pc/app/tree-select/copy.spec.ts
# Also search for any copy-related test cases with different patterns
rg "test.*copy" -A 5 --type ts
Length of output: 4306
examples/sites/demos/pc/app/tree-select/panel-style.spec.ts (1)
1-2: LGTM! Clean and minimal imports.
The test file correctly imports only the necessary functions from Playwright.
examples/sites/demos/pc/app/tree-select/disabled.vue (1)
23-23: LGTM! Verify test coverage for the updated selection.
The change to [9, 6] improves the example by:
- Reducing the number of selected items for better clarity
- Demonstrating selection across different tree branches
- Making the disabled state more visually distinguishable
Let's verify the test coverage for these values:
✅ Verification successful
Let me generate additional shell scripts to verify the test coverage for the tree-select disabled state:
This will help verify:
- The test coverage for disabled tree-select functionality
- The complete context of the disabled.vue demo file
- Any other test files that may be testing disabled functionality
Let me generate additional shell scripts to verify the test coverage for the tree-select disabled state:
This will help verify:
- The complete context of the disabled.vue demo file
- The e2e test files for tree-select
- Any test cases specifically covering disabled functionality and value3 changes
Based on the shell script results, I can now provide a final response:
The change to value3 is appropriate and test coverage is verified.
The change from [1, 4, 9, 10] to [9, 6] in the disabled tree-select demo is well-tested because:
-
The demo file shows three distinct test scenarios for disabled state:
- Empty selection (value1)
- Single selection (value2)
- Multiple selection (value3)
-
The e2e test file
disabled.spec.tsexists and specifically tests the disabled functionality by navigating totree-select#disabled -
The new values
[9, 6]provide good coverage by testing:- Selection across different tree branches (9 from first branch, 6 from second branch)
- Different tree depths (level 3 and level 2 nodes)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the e2e tests adequately cover the disabled state with the new values
# Look for related test files
fd -e spec.ts -e cy.ts "tree-select.*disabled"
# Check if the test files reference these specific values
rg -A 5 "[9, 6]"
# Look for other disabled tree-select test scenarios
rg -B 2 -A 5 "disabled.*tree-select|tree-select.*disabled"
Length of output: 151289
Script:
#!/bin/bash
# Look for tree-select disabled test files and their content
fd -e spec.ts -e cy.ts "tree-select" | xargs rg -B 2 -A 5 "disabled|value3"
# Check the disabled.vue demo file content
cat examples/sites/demos/pc/app/tree-select/disabled.vue
# Look for other tree-select test files that may cover disabled functionality
fd -e spec.ts -e cy.ts "tree-select" | xargs cat
Length of output: 107978
Script:
#!/bin/bash
# Get the content of the disabled.vue demo file
cat examples/sites/demos/pc/app/tree-select/disabled.vue
# Look for tree-select e2e test files
fd -e spec.ts -e cy.ts "tree-select" -x cat {}
# Look for tree-select disabled test cases
rg -B 2 -A 5 "disabled.*tree-select|tree-select.*disabled|value3" examples/sites/demos/pc/app/tree-select/
Length of output: 6005
examples/sites/demos/pc/app/tree-select/size.vue (1)
23-23: LGTM! Initial state setup for e2e testing
The change from empty array to [9, 6] provides a consistent initial state across different examples, which is beneficial for e2e testing scenarios. These values correspond to "三级 1-1-1" and "二级 2-2" in the tree structure.
examples/sites/demos/pc/app/tree-select/reference-style.vue (1)
30-30: LGTM! Initializing with a default selection improves the demo.
The change to initialize value2 with [9] enhances the demo by showing the warning tag type with a pre-selected value ("三级 1-1-1"), making it more illustrative for users and providing a better starting point for e2e tests.
Let's verify if the e2e tests cover this initial state:
✅ Verification successful
LGTM! The test case verifies the warning tag type with the pre-selected value.
The test case 标签类型 specifically checks for the warning tag class on the selected value, confirming that the initialization of value2: [9] is properly tested:
const tag = select.locator('.tiny-tag')
await expect(tag.nth(0)).toHaveClass(/tiny-tag--warning/)This validates that the change to initialize value2 with [9] is covered by the e2e tests, ensuring both the selection state and the warning tag type are verified.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if e2e tests verify the initial selected state
# Look for e2e test files related to tree-select
fd -e spec.ts -e cy.ts "tree-select" test/
# Search for test cases checking initial selection
rg -l "value2.*\[9\]" test/
Length of output: 211
Script:
#!/bin/bash
# Let's first find the correct test directory structure
fd -t d "test|e2e|cypress|__tests__"
# Then search for tree-select related test files in any test directory
fd -e spec.ts -e cy.ts -e test.ts "tree-select"
# Also check if there are any test files directly in the examples directory
fd -e spec.ts -e cy.ts -e test.ts . examples/
Length of output: 60025
Script:
#!/bin/bash
# Let's check the test file for tree-select reference-style demo
cat examples/sites/demos/pc/app/tree-select/reference-style.spec.ts
# Also check for any other test files that might reference value2 initialization
rg -l "value2.*\[9\]" examples/sites/demos/pc/app/tree-select/
Length of output: 2415
Script:
#!/bin/bash
# Let's check both files that contain the value2 initialization
cat examples/sites/demos/pc/app/tree-select/reference-style.spec.ts examples/sites/demos/pc/app/tree-select/reference-style-composition-api.vue examples/sites/demos/pc/app/tree-select/reference-style.vue
Length of output: 5504
examples/sites/demos/pc/app/tree-select/size.spec.ts (2)
1-2: LGTM!
The imports are clean and include only the necessary testing utilities.
1-59: Consider adding tests for dynamic size changes and edge cases.
The current tests cover static size variants well, but consider adding tests for:
- Dynamic size changes through props
- Edge cases like empty state rendering
- Responsive behavior with different viewport sizes
Let's check if the component supports these scenarios:
packages/renderless/src/base-select/index.ts (7)
Line range hint 217-219: Verify label assignment logic with clearNoMatchValue
The computation of label considers multiple conditions, including !props.clearNoMatchValue. Ensure that this logic correctly handles all possible values of value, especially when value is an object, null, or undefined, to avoid unintended labels.
Line range hint 234-237: Potential unintended clearing of values in setSelected
In the setSelected function, when !option.state.currentLabel is true, api.clearNoMatchValue('') is called. Verify that this does not unintentionally clear valid selections, and that clearNoMatchValue appropriately handles empty strings without affecting other stateful data.
Line range hint 265-266: Ensure accurate model value update in getResultOfSetSelected
The invocation of api.clearNoMatchValue(newModelValue) should be checked to confirm that newModelValue accurately reflects the desired state and does not inadvertently remove valid selections or introduce inconsistencies.
Line range hint 279-281: Conditional assignment of state.selected
The assignment state.selected = result occurs only when !state.selected?.[0]?.isTree && !state.selected?.[0]?.isGrid is true. Ensure that this condition encompasses all necessary cases and that state.selected is correctly updated without overlooking scenarios where state.selected may be undefined or empty.
Line range hint 401-405: Correct handling of created options
Setting state.createdSelected and state.createdLabel when option.created is true ensures that newly created options are handled properly. This logic appears sound and integrates well with the selection mechanism.
Line range hint 906-908: Ensure early return does not skip necessary updates
The early return in watchVisible when api.onCopying() is true might skip essential state updates or side effects. Confirm that all necessary code paths are executed even when copying is in progress to prevent inconsistent states.
Line range hint 1275-1278: Confirm correct tagType assignment
In computedGetTagType, the condition checks designConfig?.props?.tagType && !props.tagType to assign designConfig.props.tagType. Ensure that this logic correctly prioritizes props.tagType over designConfig.props.tagType when props.tagType is provided, even if it is falsy (e.g., an empty string).
| const hasDisabled = await input.evaluate((input) => input.hasAttribute('disabled')) | ||
| await expect(hasDisabled).toBe(true) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance test coverage for disabled state
While the basic disabled attribute check is good, consider adding more comprehensive validations:
- Verify visual disabled state (CSS classes)
- Attempt interactions to ensure they're blocked
- Validate ARIA attributes
const hasDisabled = await input.evaluate((input) => input.hasAttribute('disabled'))
await expect(hasDisabled).toBe(true)
+
+ // Verify visual state
+ await expect(select).toHaveClass(/is-disabled/)
+
+ // Verify ARIA attributes
+ await expect(input).toHaveAttribute('aria-disabled', 'true')
+
+ // Verify that clicking doesn't open the dropdown
+ await input.click()
+ await expect(page.locator('.tiny-tree-select__dropdown')).not.toBeVisible()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const hasDisabled = await input.evaluate((input) => input.hasAttribute('disabled')) | |
| await expect(hasDisabled).toBe(true) | |
| const hasDisabled = await input.evaluate((input) => input.hasAttribute('disabled')) | |
| await expect(hasDisabled).toBe(true) | |
| // Verify visual state | |
| await expect(select).toHaveClass(/is-disabled/) | |
| // Verify ARIA attributes | |
| await expect(input).toHaveAttribute('aria-disabled', 'true') | |
| // Verify that clicking doesn't open the dropdown | |
| await input.click() | |
| await expect(page.locator('.tiny-tree-select__dropdown')).not.toBeVisible() |
| await page.goto('tree-select#map-field') | ||
|
|
||
| const wrap = page.locator('#map-field') | ||
| const select = wrap.locator('.tiny-tree-select').nth(0) | ||
| const input = select.locator('.tiny-input__inner') | ||
| const dropdown = page.locator('body > .tiny-select-dropdown') | ||
| const treeNode = dropdown.locator('.tiny-tree-node') |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve test reliability with better element selection and navigation.
The current element selection strategy could be more robust:
- Add navigation completion check
- Use more specific selectors instead of nth(0)
- Add timeout handling for element selection
- await page.goto('tree-select#map-field')
+ await Promise.all([
+ page.goto('tree-select#map-field'),
+ page.waitForNavigation({ waitUntil: 'networkidle' })
+ ])
const wrap = page.locator('#map-field')
- const select = wrap.locator('.tiny-tree-select').nth(0)
+ const select = wrap.locator('.tiny-tree-select[data-test-id="map-field-select"]')
const input = select.locator('.tiny-input__inner')
- const dropdown = page.locator('body > .tiny-select-dropdown')
+ const dropdown = page.locator('.tiny-select-dropdown[role="listbox"]')
const treeNode = dropdown.locator('.tiny-tree-node')
+ // Add timeout handling
+ await expect(select).toBeVisible({ timeout: 5000 })Committable suggestion skipped: line range outside the PR's diff.
| await input.click() | ||
| await treeNode.filter({ hasText: /^二级 2-1$/ }).click() | ||
| await expect(input).toHaveValue('二级 2-1') | ||
| await input.click() | ||
| await expect(treeNode.filter({ hasText: /^二级 2-1$/ })).toHaveClass(/is-current/) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance test coverage with additional scenarios and assertions.
The current test only covers the happy path. Consider adding:
- Verification of dropdown visibility states
- Negative test cases
- Data mapping verification
- Cleanup after test
await input.click()
+ await expect(dropdown).toBeVisible()
+
await treeNode.filter({ hasText: /^二级 2-1$/ }).click()
await expect(input).toHaveValue('二级 2-1')
+ await expect(dropdown).toBeHidden()
+
await input.click()
await expect(treeNode.filter({ hasText: /^二级 2-1$/ })).toHaveClass(/is-current/)
+
+ // Verify actual field mapping
+ const selectedValue = await select.evaluate((el) => el.__vue__.modelValue)
+ expect(selectedValue).toBe('2-1') // Assuming this is the mapped value
+
+ // Test invalid selection
+ await input.click()
+ await treeNode.filter({ hasText: 'Non-existent Node' }).click()
+ await expect(input).toHaveValue('二级 2-1') // Should retain previous value
+
+ // Cleanup
+ await page.evaluate(() => {
+ localStorage.clear();
+ sessionStorage.clear();
+ })Committable suggestion skipped: line range outside the PR's diff.
| await suffixSvg.click() | ||
| await expect(checkedTreeNodes).toHaveCount(2) | ||
| await expect(treeNode).toHaveCount(7) | ||
| await page | ||
| .locator('div') | ||
| .filter({ hasText: /^一级 2$/ }) | ||
| .locator('.tiny-checkbox') | ||
| .click() | ||
|
|
||
| await expect(checkedTreeNodes).toHaveCount(4) | ||
| await expect(tag).toHaveCount(4) | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add test coverage for additional scenarios
The current test covers basic selection functionality but should include:
- Deselection of items
- Maximum selection limit (if applicable)
- Keyboard navigation
- Search functionality (if available)
- Clear all selections
Here's an example of additional test cases:
// Test deselection
await page.locator('.tiny-tag').nth(0).locator('.tiny-tag__close').click()
await expect(tag).toHaveCount(3)
await expect(checkedTreeNodes).toHaveCount(3)
// Test clear all
await select.locator('.tiny-input__suffix .tiny-input__clear').click()
await expect(tag).toHaveCount(0)
await expect(checkedTreeNodes).toHaveCount(0)
// Test keyboard navigation
await select.click()
await page.keyboard.press('ArrowDown')
await page.keyboard.press('Space')
await expect(tag).toHaveCount(1)| const wrap = page.locator('#multiple') | ||
| const select = wrap.locator('.tiny-tree-select').nth(0) | ||
|
|
||
| const dropdown = page.locator('body > .tiny-select-dropdown') | ||
| const suffixSvg = select.locator('.tiny-input__suffix .tiny-base-select__caret') | ||
| const treeNode = dropdown.locator('.tiny-tree-node') | ||
| const checkedTreeNodes = dropdown.locator('.tiny-tree-node.is-checked') | ||
| const tag = select.locator('.tiny-tag') |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve selector reliability with data-testid attributes
The current selectors rely heavily on CSS classes and nth-child selectors, which can be brittle. Consider using data-testid attributes for more reliable test selectors.
- const wrap = page.locator('#multiple')
- const select = wrap.locator('.tiny-tree-select').nth(0)
- const dropdown = page.locator('body > .tiny-select-dropdown')
+ const wrap = page.locator('[data-testid="multiple-tree-select-demo"]')
+ const select = wrap.locator('[data-testid="tree-select"]')
+ const dropdown = page.locator('[data-testid="tree-select-dropdown"]')This would require adding corresponding data-testid attributes to the component template.
Committable suggestion skipped: line range outside the PR's diff.
| // 再选中2个 | ||
| await page.getByRole('treeitem', { name: '三级 1-1-1' }).locator('path').nth(1).click() | ||
| await page.getByRole('treeitem', { name: '三级 1-1-2' }).locator('path').nth(1).click() | ||
| await expect(tag.filter({ hasText: '+ 4' })).toBeVisible() | ||
| await expect(tag).toHaveCount(2) | ||
| }) |
There was a problem hiding this comment.
Improve multiple selection test robustness and coverage.
The multiple selection test needs improvements:
- Replace fragile path selectors
- Add verification of all selected items
- Verify the final state comprehensively
// 再选中2个
- await page.getByRole('treeitem', { name: '三级 1-1-1' }).locator('path').nth(1).click()
- await page.getByRole('treeitem', { name: '三级 1-1-2' }).locator('path').nth(1).click()
+ const nodes = [
+ page.getByRole('treeitem', { name: '三级 1-1-1' }),
+ page.getByRole('treeitem', { name: '三级 1-1-2' })
+ ]
+ for (const node of nodes) {
+ await node.getByRole('checkbox').click()
+ // Wait for selection to take effect
+ await expect(node).toHaveClass(/is-checked/)
+ }
await expect(tag.filter({ hasText: '+ 4' })).toBeVisible()
await expect(tag).toHaveCount(2)
+ // Verify all selected nodes
+ await expect(checkedTreeNodes).toHaveCount(4)
+ // Verify the collapsed tag shows correct count
+ const selectedCount = await checkedTreeNodes.count()
+ await expect(tag.filter({ hasText: `+ ${selectedCount - 1}` })).toBeVisible()Committable suggestion skipped: line range outside the PR's diff.
| // 取消选中一个 | ||
| await page.getByRole('treeitem', { name: '三级 1-1-1' }).locator('path').nth(1).click() | ||
| await expect(tag.filter({ hasText: '+ 1' })).toBeHidden() | ||
| await expect(tag).toHaveCount(1) |
There was a problem hiding this comment.
Replace fragile path selector and enhance deselection verification.
The current implementation has potential stability issues:
- Using SVG path selectors is fragile and may break with visual changes
- Missing verification of the deselected node's state
// 取消选中一个
- await page.getByRole('treeitem', { name: '三级 1-1-1' }).locator('path').nth(1).click()
+ const nodeToDeselect = page.getByRole('treeitem', { name: '三级 1-1-1' })
+ await nodeToDeselect.getByRole('checkbox').click()
await expect(tag.filter({ hasText: '+ 1' })).toBeHidden()
await expect(tag).toHaveCount(1)
+ // Verify the node is actually deselected
+ await expect(nodeToDeselect).not.toHaveClass(/is-checked/)
+ // Verify the remaining selected tag text
+ await expect(tag.first()).toHaveText('三级 1-1-2')📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // 取消选中一个 | |
| await page.getByRole('treeitem', { name: '三级 1-1-1' }).locator('path').nth(1).click() | |
| await expect(tag.filter({ hasText: '+ 1' })).toBeHidden() | |
| await expect(tag).toHaveCount(1) | |
| // 取消选中一个 | |
| const nodeToDeselect = page.getByRole('treeitem', { name: '三级 1-1-1' }) | |
| await nodeToDeselect.getByRole('checkbox').click() | |
| await expect(tag.filter({ hasText: '+ 1' })).toBeHidden() | |
| await expect(tag).toHaveCount(1) | |
| // Verify the node is actually deselected | |
| await expect(nodeToDeselect).not.toHaveClass(/is-checked/) | |
| // Verify the remaining selected tag text | |
| await expect(tag.first()).toHaveText('三级 1-1-2') |
| // 点击下拉后选中效果 | ||
| await tag.first().click() | ||
| await expect(checkedTreeNodes).toHaveCount(2) | ||
| await expect(treeNode.nth(2)).toHaveClass(/is-checked/) | ||
| await expect(treeNode.nth(6)).toHaveClass(/is-checked/) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add stability improvements for click interactions.
The click interaction test could be more stable by:
- Waiting for animations to complete
- Verifying the dropdown visibility state
- Adding explicit waits for state changes
// 点击下拉后选中效果
await tag.first().click()
+ // Wait for dropdown animation
+ await expect(dropdown).toBeVisible()
+ // Wait for tree nodes to be ready
+ await page.waitForSelector('.tiny-tree-node.is-checked')
await expect(checkedTreeNodes).toHaveCount(2)
await expect(treeNode.nth(2)).toHaveClass(/is-checked/)
await expect(treeNode.nth(6)).toHaveClass(/is-checked/)
+ // Verify the selected nodes' text content
+ await expect(treeNode.nth(2)).toHaveText('三级 1-1-1')
+ await expect(treeNode.nth(6)).toHaveText('三级 1-1-2')Committable suggestion skipped: line range outside the PR's diff.
| @@ -1,11 +1,11 @@ | |||
| <template> | |||
| <div> | |||
| <p>场景1：一键复制所有标签</p> | |||
| <tiny-tree-select v-model="value2" :tree-op="treeOp" multiple copyable></tiny-tree-select> | |||
| <tiny-tree-select v-model="value" :tree-op="treeOp" multiple copyable></tiny-tree-select> | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add data-test attributes for e2e testing
Since this component is intended for e2e testing, consider adding data-test attributes to make the tests more robust and maintainable.
Add test attributes to key elements:
-<tiny-tree-select v-model="value" :tree-op="treeOp" multiple copyable></tiny-tree-select>
+<tiny-tree-select data-test="basic-copy-select" v-model="value" :tree-op="treeOp" multiple copyable></tiny-tree-select>
-<tiny-tree-select v-model="value" :tree-op="treeOp" multiple copyable text-split="/"></tiny-tree-select>
+<tiny-tree-select data-test="custom-separator-select" v-model="value" :tree-op="treeOp" multiple copyable text-split="/"></tiny-tree-select>
-<tiny-input v-model="inputVal" class="copy-value" type="text"></tiny-input>
+<tiny-input data-test="paste-target" v-model="inputVal" class="copy-value" type="text"></tiny-input>Also applies to: 6-6, 8-8
| test('下划线类型', async ({ page }) => { | ||
| page.on('pageerror', (exception) => expect(exception).toBeNull()) | ||
| await page.goto('tree-select#reference-style') | ||
| const wrap = page.locator('#reference-style') | ||
| const select = wrap.locator('.tiny-tree-select').nth(2) | ||
| const input = select.locator('.tiny-input__inner') | ||
| const dropdown = page.locator('body > .tiny-select-dropdown') | ||
| const treeNode = dropdown.locator('.tiny-tree-node') | ||
|
|
||
| await expect(select.locator('.tiny-input')).toHaveClass(/tiny-input-underline/) | ||
| await expect(input).toHaveCSS('border-top-width', '0px') | ||
| await expect(input).toHaveCSS('border-left-width', '0px') | ||
| await expect(input).toHaveCSS('border-right-width', '0px') | ||
| await expect(input).toHaveCSS('border-bottom-color', 'rgb(194, 194, 194)') | ||
|
|
||
| await select.click() | ||
| await treeNode.filter({ hasText: /^二级 2-1$/ }).click() | ||
| await expect(dropdown).toBeHidden() | ||
| await expect(input).toHaveValue('二级 2-1') | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve test reliability and organization
The test needs several improvements:
- Add proper waiting mechanisms for dropdown visibility
- Group related assertions
- Use constants for test data
- Add error handling for interaction failures
+const TEST_NODE_TEXT = '二级 2-1';
-test('下划线类型', async ({ page }) => {
+test('underline input should have correct styling and interaction', async ({ page }) => {
page.on('pageerror', (exception) => expect(exception).toBeNull())
await page.goto('tree-select#reference-style')
const wrap = page.locator('#reference-style')
const select = wrap.locator('.tiny-tree-select').nth(2)
const input = select.locator('.tiny-input__inner')
const dropdown = page.locator('body > .tiny-select-dropdown')
const treeNode = dropdown.locator('.tiny-tree-node')
+ // Verify underline styling
await expect(select.locator('.tiny-input')).toHaveClass(/tiny-input-underline/)
await expect(input).toHaveCSS('border-top-width', '0px')
await expect(input).toHaveCSS('border-left-width', '0px')
await expect(input).toHaveCSS('border-right-width', '0px')
await expect(input).toHaveCSS('border-bottom-color', 'rgb(194, 194, 194)')
+ // Test interaction
await select.click()
+ await expect(dropdown).toBeVisible()
- await treeNode.filter({ hasText: /^二级 2-1$/ }).click()
+ await treeNode.filter({ hasText: new RegExp(`^${TEST_NODE_TEXT}$`) }).click()
+
+ // Verify final state
await expect(dropdown).toBeHidden()
- await expect(input).toHaveValue('二级 2-1')
+ await expect(input).toHaveValue(TEST_NODE_TEXT)
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| test('下划线类型', async ({ page }) => { | |
| page.on('pageerror', (exception) => expect(exception).toBeNull()) | |
| await page.goto('tree-select#reference-style') | |
| const wrap = page.locator('#reference-style') | |
| const select = wrap.locator('.tiny-tree-select').nth(2) | |
| const input = select.locator('.tiny-input__inner') | |
| const dropdown = page.locator('body > .tiny-select-dropdown') | |
| const treeNode = dropdown.locator('.tiny-tree-node') | |
| await expect(select.locator('.tiny-input')).toHaveClass(/tiny-input-underline/) | |
| await expect(input).toHaveCSS('border-top-width', '0px') | |
| await expect(input).toHaveCSS('border-left-width', '0px') | |
| await expect(input).toHaveCSS('border-right-width', '0px') | |
| await expect(input).toHaveCSS('border-bottom-color', 'rgb(194, 194, 194)') | |
| await select.click() | |
| await treeNode.filter({ hasText: /^二级 2-1$/ }).click() | |
| await expect(dropdown).toBeHidden() | |
| await expect(input).toHaveValue('二级 2-1') | |
| }) | |
| const TEST_NODE_TEXT = '二级 2-1'; | |
| test('underline input should have correct styling and interaction', async ({ page }) => { | |
| page.on('pageerror', (exception) => expect(exception).toBeNull()) | |
| await page.goto('tree-select#reference-style') | |
| const wrap = page.locator('#reference-style') | |
| const select = wrap.locator('.tiny-tree-select').nth(2) | |
| const input = select.locator('.tiny-input__inner') | |
| const dropdown = page.locator('body > .tiny-select-dropdown') | |
| const treeNode = dropdown.locator('.tiny-tree-node') | |
| // Verify underline styling | |
| await expect(select.locator('.tiny-input')).toHaveClass(/tiny-input-underline/) | |
| await expect(input).toHaveCSS('border-top-width', '0px') | |
| await expect(input).toHaveCSS('border-left-width', '0px') | |
| await expect(input).toHaveCSS('border-right-width', '0px') | |
| await expect(input).toHaveCSS('border-bottom-color', 'rgb(194, 194, 194)') | |
| // Test interaction | |
| await select.click() | |
| await expect(dropdown).toBeVisible() | |
| await treeNode.filter({ hasText: new RegExp(`^${TEST_NODE_TEXT}$`) }).click() | |
| // Verify final state | |
| await expect(dropdown).toBeHidden() | |
| await expect(input).toHaveValue(TEST_NODE_TEXT) | |
| }) |
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Outside diff range and nitpick comments (42)
examples/sites/demos/pc/app/tree-select/disabled.spec.ts (2)
3-5: Consider using English test descriptions for better international collaborationWhile the current test name '禁用' is valid, using English descriptions would improve accessibility for international contributors.
-test('禁用', async ({ page }) => { +test('should verify tree-select disabled state', async ({ page }) => {Also, consider adding more specific test descriptions using test.describe() for better organization.
11-12: Enhance test coverage with additional assertionsThe current test only verifies the presence of the disabled attribute. Consider adding:
- Visual state verification
- Interaction attempt verification
- Proper error messages
const hasDisabled = await input.evaluate((input) => input.hasAttribute('disabled')) await expect(hasDisabled).toBe(true) + + // Verify visual state + await expect(select).toHaveClass(/is-disabled/) + + // Verify that clicking doesn't open the dropdown + await input.click() + await expect(page.locator('.tiny-tree-select__dropdown')).not.toBeVisible() + + // Verify that typing is prevented + await input.type('test') + await expect(input).toHaveValue('')examples/sites/demos/pc/app/tree-select/native-properties.spec.ts (3)
3-5: Consider improving test case robustness and internationalization
- Consider using English test names for better international collaboration
- The error handling could be more specific about expected errors
- URL navigation could be more robust
Consider these improvements:
-test('原生属性', async ({ page }) => { +test('native properties - should have correct input attributes', async ({ page }) => { - page.on('pageerror', (exception) => expect(exception).toBeNull()) + page.on('pageerror', (exception) => { + console.error('Page error:', exception); + expect(exception).toBeNull(); + }) - await page.goto('tree-select#native-properties') + await page.goto('tree-select#native-properties', { waitUntil: 'networkidle' })
11-15: Enhance test coverage for native propertiesWhile the current assertions are good, consider adding more comprehensive tests for native properties.
Consider adding these additional test cases:
await expect(input).toHaveAttribute('name', 'inputName') await expect(input).toHaveAttribute('placeholder', '请选择一个选项') - const isHasAutocomplete = await input.evaluate((input) => input.hasAttribute('autocomplete')) - await expect(isHasAutocomplete).toBe(true) + // Test more native properties + await expect(input).toHaveAttribute('autocomplete', 'off') + + // Test disabled state + await expect(input).not.toBeDisabled() + + // Test readonly state + await expect(input).not.toHaveAttribute('readonly', '') + + // Test input type + await expect(input).toHaveAttribute('type', 'text') + + // Test aria attributes for accessibility + await expect(input).toHaveAttribute('role', 'combobox') + await expect(input).toHaveAttribute('aria-expanded', 'false')
1-16: Document test coverage and consider integration scenariosWhile this test covers native properties well, consider adding:
- JSDoc comments describing the test's purpose and coverage
- Integration scenarios with other tree-select features
Add documentation and consider these scenarios:
/** * E2E Test: Tree Select Native Properties * * Coverage: * - Input element native attributes * - Accessibility attributes * - Default states * * Related tests: * - basic-usage.spec.ts * - keyboard-operations.spec.ts */examples/sites/demos/pc/app/tree-select/map-field.spec.ts (2)
1-5: Consider using English test descriptions for better international collaboration.While the current test name '映射字段' describes the functionality, using English would improve readability for a wider audience.
-test('映射字段', async ({ page }) => { +test('Tree Select: Field Mapping Functionality', async ({ page }) => {
7-11: Consider using data-testid attributes for more reliable element selection.While the current selectors work, they rely on CSS classes which might change. Using data-testid attributes would make the tests more resilient to UI changes.
- const select = wrap.locator('.tiny-tree-select').nth(0) + const select = wrap.locator('[data-testid="tree-select"]')Also, using
.nth(0)might be fragile if more tree-select components are added. Consider using a more specific selector if possible.examples/sites/demos/pc/app/tree-select/basic-usage-composition-api.vue (1)
Line range hint
1-55: Consider adding more test scenarios.While the current example demonstrates basic single selection, consider adding examples and corresponding e2e tests for:
- Multiple selection mode
- Disabled nodes
- Dynamic loading of nodes
- Custom node rendering
- Search/filter functionality
Would you like me to help create additional example components covering these scenarios?
examples/sites/demos/pc/app/tree-select/basic-usage.spec.ts (4)
3-3: Consider using English for test namesWhile the new test name "下拉树单选" (dropdown tree single select) is more specific, consider using English to maintain consistency with the codebase. For example: "tree-select single selection".
14-16: Consider adding dropdown visibility checkThe assertions for suffix icon and initial value are good. Consider adding an assertion to verify that the dropdown is initially closed:
await expect(suffixSvg).toHaveCount(1) await expect(suffixSvg).toBeVisible() await expect(input).toHaveValue('三级 1-1-2') + await expect(dropdown).toBeHidden()
Line range hint
19-24: Add dropdown visibility checks during interactionsThe selection flow is well tested, but consider adding dropdown visibility assertions to ensure proper opening/closing behavior:
await input.click() + await expect(dropdown).toBeVisible() await expect(treeNode.filter({ hasText: /^三级 1-1-2$/ })).toHaveClass(/is-current/) await treeNode.filter({ hasText: /^二级 2-1$/ }).click() + await expect(dropdown).toBeHidden() await expect(input).toHaveValue('二级 2-1') await input.click() + await expect(dropdown).toBeVisible() await expect(treeNode.filter({ hasText: /^二级 2-1$/ })).toHaveClass(/is-current/)
Line range hint
1-24: Consider adding keyboard interaction testsThe current test covers mouse interactions well. Consider adding another test case for keyboard interactions:
- Arrow key navigation
- Enter key for selection
- Escape key for closing dropdown
Would you like me to help create a test case for keyboard interactions?
examples/sites/demos/pc/app/tree-select/multiple.spec.ts (2)
1-5: Consider enhancing the test setup.A few suggestions to improve the test setup:
- Consider using English for test descriptions to maintain consistency with typical testing conventions
- The error handler could be more specific about the types of errors it expects to catch
-test('下拉树多选', async ({ page }) => { +test('tree-select multiple selection functionality', async ({ page }) => { // Consider adding specific error type handling - page.on('pageerror', (exception) => expect(exception).toBeNull()) + page.on('pageerror', (error) => { + console.error('Page error:', error); + expect(error).toBeNull(); + })
1-29: Consider test isolation and reusability.To improve the test architecture, consider:
- Creating a test fixture for common setup and teardown
- Extracting reusable selectors and test utilities
- Adding test isolation by resetting component state between tests
This will make the tests more maintainable and reliable as the test suite grows.
Example implementation:
// fixtures/tree-select.ts export const treeSelectFixture = { async createTreeSelect(page) { const selectors = { wrapper: '[data-testid="tree-select"]', dropdown: '[data-testid="dropdown"]', // ... other selectors }; return { selectors, async open() { // ... common open logic }, async close() { // ... common close logic }, async selectNode(nodeId: string) { // ... common selection logic } }; } };examples/sites/demos/pc/app/tree-select/copy.spec.ts (1)
1-30: Consider improving test organization and reusabilityThe test file could benefit from better organization:
- Extract common selectors and actions into a Page Object Model
- Centralize test data in a separate file
- Use test fixtures for common setup/teardown
Here's a suggested structure:
// tree-select.po.ts export class TreeSelectPage { constructor(private page: Page) {} async goto() { await this.page.goto('tree-select#copy') } getSelect(index: number) { const wrap = this.page.locator('#copy') return { element: wrap.locator('.tiny-tree-select').nth(index), copyIcon: wrap.locator('.tiny-base-select__copy .tiny-svg').nth(index), copyValueInput: wrap.locator('.copy-value .tiny-input__inner') } } async copyAndPaste(selectIndex: number) { const { element, copyIcon, copyValueInput } = this.getSelect(selectIndex) await element.hover() await copyIcon.click() await copyValueInput.press(process.platform === 'darwin' ? 'Meta+V' : 'Control+V') return copyValueInput } }Would you like me to help create this structure in a new PR?
examples/sites/demos/pc/app/tree-select/disabled-composition-api.vue (1)
Line range hint
1-67: Document the test scenario purposeThe component demonstrates three disabled states but lacks documentation explaining the specific test scenarios, particularly for the multi-select case with disconnected nodes. Consider adding comments to clarify the purpose of each test case.
Add JSDoc comments to explain the scenarios:
<script setup> import { ref } from 'vue' import { TinyTreeSelect } from '@opentiny/vue' +/** + * Test Scenarios: + * 1. Empty selection - Validates disabled state with no initial selection + * 2. Single selection - Tests disabled state with a parent node selected + * 3. Multiple selection - Verifies disabled state with disconnected nodes + */ const value1 = ref('') const value2 = ref(1) const value3 = ref([9, 6])examples/sites/demos/pc/app/tree-select/copy-composition-api.vue (2)
8-8: Consider adding placeholder text to the input.The input field's purpose would be clearer with a placeholder text indicating it's for testing the copied values.
- <tiny-input v-model="inputVal" class="copy-value" type="text"></tiny-input> + <tiny-input v-model="inputVal" class="copy-value" type="text" placeholder="Paste copied values here"></tiny-input>
16-16: Initial values match tree data structure.The initial values [9, 6] correspond to valid nodes in the tree:
- 9: "三级 1-1-1" (third level)
- 6: "二级 2-2" (second level)
Consider adding a comment to document why these specific values were chosen as defaults.
-const value = ref([9, 6]) +// Pre-select a mix of different level nodes to demonstrate multi-level selection +const value = ref([9, 6]) // Selected: "三级 1-1-1" and "二级 2-2"examples/sites/demos/pc/app/tree-select/reference-style-composition-api.vue (2)
Line range hint
1-15: Consider internationalizing the scenario descriptions.The template uses Chinese text for scenario descriptions. Consider using i18n for better maintainability and accessibility in an international context.
Example implementation:
<template> <div> - <p>场景1：自定义图标 + 自定义样式</p> + <p>{{ t('treeSelect.scenario1') }}</p> <tiny-tree-select v-model="value" :tree-op="treeOp" :dropdown-icon="tinyIconPopup" :drop-style="{ width: '200px', 'min-width': '200px' }" > </tiny-tree-select> - <p>场景2：标签类型</p> + <p>{{ t('treeSelect.scenario2') }}</p> <tiny-tree-select v-model="value2" :tree-op="treeOp" multiple tag-type="warning"></tiny-tree-select>
Line range hint
71-79: Consider using CSS variables for better maintainability.The scoped styles could benefit from using CSS variables for better maintainability and consistency across components.
Example implementation:
<style scoped> .tiny-tree-select { - width: 280px; + width: var(--tiny-tree-select-width, 280px); } p { - font-size: 14px; - line-height: 2.5; + font-size: var(--tiny-text-size-sm, 14px); + line-height: var(--tiny-line-height-lg, 2.5); } </style>examples/sites/demos/pc/app/tree-select/panel-style.spec.ts (3)
3-13: Consider improving test robustness and maintainabilityA few suggestions to enhance the test:
- Use English for test descriptions to maintain consistency with the codebase
- Add additional assertions to verify the mounting behavior more thoroughly
- Consider adding element existence checks before interactions
Consider this refactor:
-test('不挂载在 body 元素上', async ({ page }) => { +test('dropdown should not mount on body element', async ({ page }) => { page.on('pageerror', (exception) => expect(exception).toBeNull()) await page.goto('tree-select#panel-style') const wrap = page.locator('#panel-style') const select = wrap.locator('.tiny-tree-select').nth(0) const dropdown = select.locator('.tiny-base-select__tags-group > .tiny-select-dropdown') + await expect(select).toBeVisible() await select.click() await expect(dropdown).toHaveCount(1) + // Verify it's not mounted on body + const bodyDropdown = page.locator('body > .tiny-select-dropdown') + await expect(bodyDropdown).toHaveCount(0) })
28-38: Improve placement test coverageThe current test only checks the placement attribute but doesn't verify the actual positioning:
- Consider checking initial placement before interaction
- Verify actual visual positioning
- Handle viewport-dependent behavior
Consider this enhancement:
-test('从上方弹出面板', async ({ page }) => { +test('should display dropdown panel from top', async ({ page }) => { page.on('pageerror', (exception) => expect(exception).toBeNull()) await page.goto('tree-select#panel-style') const wrap = page.locator('#panel-style') const select = wrap.locator('.tiny-tree-select').nth(2) const dropdown = page.locator('body > .tiny-select-dropdown') + // Ensure element is in viewport + await select.scrollIntoViewIfNeeded() await select.click() + await expect(dropdown).toBeVisible() await expect(dropdown).toHaveAttribute('x-placement', 'top') + + // Verify actual positioning + const dropdownBox = await dropdown.boundingBox() + const selectBox = await select.boundingBox() + expect(dropdownBox.bottom).toBeLessThanOrEqual(selectBox.top) })
1-38: Consider test suite optimization opportunitiesThe test file could benefit from some structural improvements:
- Extract common setup code
- Share selectors
- Add proper cleanup
Consider implementing a test fixture:
import { test as base, expect } from '@playwright/test' interface TreeSelectFixture { openDropdown: (index: number) => Promise<void> getDropdown: () => Locator } const test = base.extend<TreeSelectFixture>({ openDropdown: async ({ page }, use) => { await use(async (index: number) => { const select = page.locator('#panel-style .tiny-tree-select').nth(index) await select.scrollIntoViewIfNeeded() await select.click() }) }, getDropdown: async ({ page }, use) => { await use(() => page.locator('body > .tiny-select-dropdown')) } }) test.beforeEach(async ({ page }) => { page.on('pageerror', (exception) => expect(exception).toBeNull()) await page.goto('tree-select#panel-style') })examples/sites/demos/pc/app/tree-select/disabled.vue (1)
23-23: Consider documenting the selection rationaleThe selection of nodes from different levels and branches (
[9, 6]) provides a good demonstration of multi-select capability. Consider adding a comment explaining why these specific nodes were chosen for the disabled state demo.- value3: [9, 6], + // Demonstrates disabled state with selections from different tree levels and branches + value3: [9, 6],examples/sites/demos/pc/app/tree-select/collapse-tags.spec.ts (4)
1-11: Consider enhancing error handling and selector robustness.While the setup is generally good, consider these improvements:
- The error handler could log the actual error for debugging
- Consider using more robust selectors (e.g., data-testid) instead of class-based selectors
- page.on('pageerror', (exception) => expect(exception).toBeNull()) + page.on('pageerror', (exception) => { + console.error('Page error:', exception); + expect(exception).toBeNull(); + }) - const select = wrap.locator('.tiny-tree-select').nth(0) + const select = wrap.locator('[data-testid="tree-select"]')
13-17: Enhance default state validation coverage.The initial state checks are good but could be more comprehensive. Consider adding assertions for:
- The exact text content of all visible tags
- The collapsed tag count accuracy
await expect(tag).toHaveCount(2) await expect(tag.filter({ hasText: '三级 1-1-1' })).toBeVisible() await expect(tag.filter({ hasText: '+ 1' })).toBeVisible() + // Verify exact text content + await expect(tag.first()).toHaveText('三级 1-1-1') + // Verify that the collapsed count matches actual selections + const selectedCount = await checkedTreeNodes.count() + await expect(tag.last()).toHaveText(`+ ${selectedCount - 1}`)
18-23: Add dropdown visibility and transition checks.The click interaction tests could be more thorough. Consider adding:
- Dropdown visibility validation after click
- Transition state checks
- Validation that the correct nodes are expanded
await tag.first().click() + // Verify dropdown is visible + await expect(dropdown).toBeVisible() + // Verify parent nodes are expanded + await expect(dropdown.locator('.tiny-tree-node.expanded')).toBeVisible() await expect(checkedTreeNodes).toHaveCount(2)
1-34: Consider organizing tests into logical test groups and adding edge cases.While the current test covers the basic functionality well, consider:
- Breaking this into smaller, focused test cases using
describeblocks- Adding edge case tests for:
- Maximum selection limits
- Empty states
- Keyboard navigation
- Screen reader accessibility
Example structure:
describe('TreeSelect Collapse Tags', () => { test('should display correct initial state', async () => { // Current lines 13-17 }); test('should handle selection interactions correctly', async () => { // Current lines 18-23 }); test('should handle deselection correctly', async () => { // Current lines 24-28 }); test('should handle multiple selections correctly', async () => { // Current lines 29-34 }); test('should handle keyboard navigation', async () => { // New test }); test('should be accessible to screen readers', async () => { // New test }); });examples/sites/demos/pc/app/tree-select/size.vue (2)
Line range hint
1-12: LGTM! Well-structured for e2e testing.The template provides a clear demonstration of all size variants, making it ideal for e2e testing. Each variant uses consistent props, which will help in writing reliable tests.
Would you like me to help generate e2e test cases for:
- Verifying the rendering of all size variants
- Testing multiple selection functionality
- Validating the tree-select dropdown appearance for each size
23-23: Good choice of initial values for testing.Pre-selecting
[9, 6]is excellent for e2e testing as these values:
- Come from different tree levels (三级 1-1-1 and 二级 2-2)
- Test multiple selection across different branches
- Provide a visible initial state to verify rendering
Would you like me to help generate e2e test cases to verify:
- Initial rendering of pre-selected values
- Correct display of selected items in different size variants
- Proper handling of the collapsible tags with pre-selected values
examples/sites/demos/pc/app/tree-select/reference-style.vue (1)
30-30: LGTM! Consider adding explanatory comments.The initialization of
value2with[9]effectively demonstrates the warning tag type scenario by pre-selecting "三级 1-1-1". This makes the demo more immediately useful and testable.Consider adding a comment to explain why this specific value is pre-selected:
tinyIconPopup: iconPopup(), value: '', + // Pre-select "三级 1-1-1" to demonstrate warning tag appearance value2: [9],examples/sites/demos/pc/app/tree-select/reference-style.spec.ts (3)
3-15: Consider making the SVG path test more maintainableThe current test uses a hard-coded SVG path value which could make the test brittle to minor visual changes. Consider:
- Adding an English description using test.describe()
- Using a constant for the SVG path value
- Adding assertions for icon visibility and dimensions
+const CUSTOM_DROPDOWN_ICON_PATH = 'M2.3 7.78v.08c.02.65.561 1.17 1.221 1.17.68 0 1.229-.54 1.229-1.21 0-.672-.549-1.21-1.229-1.21-.66 0-1.2.519-1.22 1.17Zm4.241.04c0 .67.54 1.21 1.22 1.21.68 0 1.23-.54 1.23-1.21 0-.672-.55-1.21-1.23-1.21-.68 0-1.22.538-1.22 1.21Zm4.24.04v-.08c.02-.651.559-1.17 1.219-1.17.682 0 1.23.538 1.23 1.21 0 .67-.548 1.21-1.23 1.21-.66 0-1.2-.52-1.219-1.17Z'; +test.describe('Tree Select - Custom Styling', () => { + test('should render custom dropdown icon correctly', async ({ page }) => { -test('自定义下拉图标', async ({ page }) => { page.on('pageerror', (exception) => expect(exception).toBeNull()) await page.goto('tree-select#reference-style') const wrap = page.locator('#reference-style') const select = wrap.locator('.tiny-tree-select').nth(0) const suffixIconPath = select.locator('.tiny-input__suffix .tiny-base-select__caret path') + await expect(suffixIconPath).toBeVisible() await expect(suffixIconPath).toHaveAttribute( 'd', - 'M2.3 7.78v.08c.02.65.561 1.17 1.221 1.17.68 0 1.229-.54 1.229-1.21 0-.672-.549-1.21-1.229-1.21-.66 0-1.2.519-1.22 1.17Zm4.241.04c0 .67.54 1.21 1.22 1.21.68 0 1.23-.54 1.23-1.21 0-.672-.55-1.21-1.23-1.21-.68 0-1.22.538-1.22 1.21Zm4.24.04v-.08c.02-.651.559-1.17 1.219-1.17.682 0 1.23.538 1.23 1.21 0 .67-.548 1.21-1.23 1.21-.66 0-1.2-.52-1.219-1.17Z' + CUSTOM_DROPDOWN_ICON_PATH ) }) +})
29-48: Improve underline style test coverageThe test should cover all visual states of the underline input:
- Extract the color value to a constant
- Add hover state validation
- Test focus state styling
+const BORDER_COLOR = 'rgb(194, 194, 194)'; +const HOVER_BORDER_COLOR = 'rgb(220, 220, 220)'; // Adjust to match your design -test('下划线类型', async ({ page }) => { +test.describe('Tree Select - Underline Style', () => { + test('should render and behave correctly with underline style', async ({ page }) => { page.on('pageerror', (exception) => expect(exception).toBeNull()) await page.goto('tree-select#reference-style') const wrap = page.locator('#reference-style') const select = wrap.locator('.tiny-tree-select').nth(2) const input = select.locator('.tiny-input__inner') const dropdown = page.locator('body > .tiny-select-dropdown') const treeNode = dropdown.locator('.tiny-tree-node') await expect(select.locator('.tiny-input')).toHaveClass(/tiny-input-underline/) await expect(input).toHaveCSS('border-top-width', '0px') await expect(input).toHaveCSS('border-left-width', '0px') await expect(input).toHaveCSS('border-right-width', '0px') - await expect(input).toHaveCSS('border-bottom-color', 'rgb(194, 194, 194)') + await expect(input).toHaveCSS('border-bottom-color', BORDER_COLOR) + // Test hover state + await input.hover() + await expect(input).toHaveCSS('border-bottom-color', HOVER_BORDER_COLOR) + // Test focus state + await input.focus() + await expect(input).toHaveClass(/is-focus/) await select.click() await treeNode.filter({ hasText: /^二级 2-1$/ }).click() await expect(dropdown).toBeHidden() await expect(input).toHaveValue('二级 2-1') }) +})
1-2: Improve test organization and documentationConsider enhancing the test file structure:
- Add JSDoc comments describing the test suite purpose
- Use consistent language for test names (preferably English)
- Consider adding common setup/teardown if needed
import { test, expect } from '@playwright/test' +/** + * E2E tests for Tree Select component styling variants + * Tests custom dropdown icon, tag types, and underline input style + */ + +const test = base.extend({ + // Add any common setup here if needed +}); + +test.beforeEach(async ({ page }) => { + page.on('pageerror', (exception) => expect(exception).toBeNull()) +});examples/sites/demos/pc/app/tree-select/size.spec.ts (3)
1-2: Consider using beforeEach for common setupThe error handling setup is repeated in each test. Consider using a
beforeEachhook to reduce code duplication.import { test, expect } from '@playwright/test' + +test.beforeEach(async ({ page }) => { + page.on('pageerror', (exception) => expect(exception).toBeNull()) + await page.goto('tree-select#size') +})
3-16: Consider adding more comprehensive assertionsWhile the basic size checks are good, consider adding:
- Verification that exactly one tag exists when expected
- Check for correct tag text content
- Verify the input placeholder if applicable
await expect(input.locator('.tiny-input__inner')).toHaveCSS('height', '40px') await expect(tag.nth(0)).toHaveClass(/tiny-tag--medium tiny-tag--light/) + await expect(tag).toHaveCount(2) // Assuming 2 tags based on [9, 6] value + await expect(tag.nth(0)).toHaveText(/Node 9/)
1-59: Improve test organization and isolationConsider the following improvements to the test suite:
- Use Playwright's test fixtures for common selectors
- Add isolation between tests
- Consider using test.describe for better organization
import { test as base, expect } from '@playwright/test' interface TreeSelectFixtures { getTreeSelect: (index: number) => Promise<{ select: Locator input: Locator tag: Locator }> } const test = base.extend<TreeSelectFixtures>({ getTreeSelect: async ({ page }, use) => { await use(async (index: number) => { const wrap = page.locator('#size') const select = wrap.locator('.tiny-tree-select').nth(index) return { select, input: select.locator('.tiny-input'), tag: select.locator('.tiny-tag') } }) } }) test.describe('TreeSelect Size Variants', () => { test.beforeEach(async ({ page }) => { page.on('pageerror', (exception) => expect(exception).toBeNull()) await page.goto('tree-select#size') }) // Your existing tests using the fixture })packages/renderless/src/base-select/index.ts (5)
1873-1873: Translate Comment to EnglishThe comment on line 1873 is in Chinese. Please translate it to English for better code readability and maintainability.
Apply this diff to translate the comment:
- // 给多选复制全部标签图标留一部分间距 + // Add spacing for the multi-select copy-all-tags icon
1877-1877: Translate Comment to EnglishThe comment on line 1877 is in Chinese. Please translate it to English to improve code readability and team collaboration.
Apply this diff to translate the comment:
-// 以下为tiny 新增功能 +// Below are the new features added by Tiny
Line range hint
1878-1881: Translate Comments to EnglishThe comments from lines 1878 to 1881 are in Chinese. Please translate them into English to ensure consistency and maintainability.
Apply this diff to translate the comments:
-/** - * 兼容不同主题多选禁用的展示类型 - * default 和 smb 主题，displayOnly 时显示为 tagText,否则为 tag - * aurora 主题 displayOnly||disabled 时显示为tagText,否则为 tag - */ +/** + * Compatible display types of multi-select disabled state across different themes + * In default and SMB themes, display as tagText when 'displayOnly'; otherwise, display as tag + * In Aurora theme, display as tagText when 'displayOnly' or 'disabled'; otherwise, display as tag + */
Line range hint
1812-1814: Potential use of undefinedstate.debounceIn the
handleDebouncedQueryChangefunction,state.debouncemay not be defined or initialized before being used in thedebouncefunction. This could lead to unexpected behavior or runtime errors. Ensure thatstate.debounceis properly initialized before use, or consider usingprops.debounceinstead.Apply this diff to fix the issue:
-export const handleDebouncedQueryChange = ({ state, api }) => - debounce(state.debounce, (value) => { +export const handleDebouncedQueryChange = ({ props, api }) => + debounce(props.debounce, (value) => {
Line range hint
2024-2024: Translate Comment to EnglishThe comment on line 2024 is in Chinese. Please translate it into English for better code readability and maintainability.
Apply this diff to translate the comment:
-// tiny 新增： aui有自己的逻辑，移至defineConfig中去了 +// Tiny addition: The AUI theme has its own logic, moved to defineConfig
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (20)
examples/sites/demos/pc/app/tree-select/basic-usage-composition-api.vue(1 hunks)examples/sites/demos/pc/app/tree-select/basic-usage.spec.ts(1 hunks)examples/sites/demos/pc/app/tree-select/collapse-tags.spec.ts(1 hunks)examples/sites/demos/pc/app/tree-select/copy-composition-api.vue(1 hunks)examples/sites/demos/pc/app/tree-select/copy.spec.ts(1 hunks)examples/sites/demos/pc/app/tree-select/copy.vue(2 hunks)examples/sites/demos/pc/app/tree-select/disabled-composition-api.vue(1 hunks)examples/sites/demos/pc/app/tree-select/disabled.spec.ts(1 hunks)examples/sites/demos/pc/app/tree-select/disabled.vue(1 hunks)examples/sites/demos/pc/app/tree-select/map-field.spec.ts(1 hunks)examples/sites/demos/pc/app/tree-select/multiple.spec.ts(1 hunks)examples/sites/demos/pc/app/tree-select/native-properties.spec.ts(1 hunks)examples/sites/demos/pc/app/tree-select/panel-style.spec.ts(1 hunks)examples/sites/demos/pc/app/tree-select/reference-style-composition-api.vue(1 hunks)examples/sites/demos/pc/app/tree-select/reference-style.spec.ts(1 hunks)examples/sites/demos/pc/app/tree-select/reference-style.vue(1 hunks)examples/sites/demos/pc/app/tree-select/size-composition-api.vue(1 hunks)examples/sites/demos/pc/app/tree-select/size.spec.ts(1 hunks)examples/sites/demos/pc/app/tree-select/size.vue(1 hunks)packages/renderless/src/base-select/index.ts(1 hunks)
🔇 Additional comments (18)
examples/sites/demos/pc/app/tree-select/disabled.spec.ts (2)
1-2: LGTM: Imports are clean and appropriate
The necessary Playwright test utilities are imported correctly.
1-13: Verify consistency with other tree-select test files
Let's ensure this test follows the same patterns as other tree-select tests.
✅ Verification successful
Test follows consistent patterns with other tree-select tests
The test file follows the same patterns observed across all tree-select test files:
- Uses standard Playwright test structure with
test()and async callback - Implements error handling via
page.on('pageerror') - Uses consistent page navigation pattern with
page.goto() - Follows similar selector patterns using
locator() - Has focused, single-purpose test cases like other files
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Find and analyze other tree-select test files for consistency
echo "Searching for other tree-select test files..."
fd -e spec.ts -e spec.js . | grep -i "tree-select"
# Check for common patterns in test files
echo "Analyzing test patterns..."
rg -l "test\(" $(fd -e spec.ts -e spec.js . | grep -i "tree-select") | \
xargs rg "test\(.*async.*" -A 2
Length of output: 7255
examples/sites/demos/pc/app/tree-select/native-properties.spec.ts (1)
1-2: LGTM: Clean and minimal imports
The test setup follows Playwright's best practices with appropriate imports.
examples/sites/demos/pc/app/tree-select/map-field.spec.ts (1)
1-18: Verify test coverage overlap with existing specs.
Let's ensure this test doesn't duplicate coverage from other tree-select test files.
✅ Verification successful
No test duplication found - implementation is unique and necessary
The map-field test implementation is distinct from other tree-select tests:
- While basic-usage.spec.ts tests similar node selection patterns, it verifies different functionality (basic single selection)
- multiple.spec.ts focuses on multi-selection behavior
- The map-field test specifically verifies field mapping functionality, which isn't covered by other tests
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for similar tree-select tests in the codebase
echo "Searching for other tree-select test files..."
fd -e spec.ts -e spec.js tree-select
echo "Checking for similar test patterns..."
rg -l "tiny-tree-select.*tiny-input__inner.*tiny-select-dropdown" --type ts
Length of output: 302
Script:
#!/bin/bash
# Let's try a different approach to find tree-select test files
echo "Searching for tree-select test files:"
fd "tree-select.*\.spec\.(ts|js)$"
echo -e "\nSearching for similar test patterns with more flexible matching:"
rg -l "映射字段|map-field" --type ts
echo -e "\nChecking content of any found tree-select test files:"
fd "tree-select.*\.spec\.(ts|js)$" --exec cat {}
Length of output: 765
Script:
#!/bin/bash
# Let's examine the content of the related test files to check for duplication
echo "Checking map-field tests in select components:"
echo -e "\nContent of base-select map-field test:"
cat examples/sites/demos/pc/app/base-select/map-field.spec.ts
echo -e "\nContent of select map-field test:"
cat examples/sites/demos/pc/app/select/map-field.spec.ts
# Let's also search for any other tree-select tests that might be named differently
echo -e "\nSearching for other tree-select test files with different patterns:"
rg -l "tiny-tree-select" --type ts | grep "spec"
Length of output: 4587
Script:
#!/bin/bash
# Let's check the content of other tree-select test files to ensure no duplication
echo "Checking content of basic-usage test for potential overlap:"
cat examples/sites/demos/pc/app/tree-select/basic-usage.spec.ts
echo -e "\nChecking content of multiple test for potential overlap:"
cat examples/sites/demos/pc/app/tree-select/multiple.spec.ts
# Also check if there are any similar node selection patterns
echo -e "\nSearching for similar tree node selection patterns:"
rg "filter\({ hasText: .*}\).*click\(\)" examples/sites/demos/pc/app/tree-select/
Length of output: 2683
examples/sites/demos/pc/app/tree-select/basic-usage-composition-api.vue (1)
9-9: Initial value change improves test coverage.
The change from ref(4) to ref(10) shifts the initial selection from a parent node ("二级 1-1") to a leaf node ("三级 1-1-2"). This is beneficial for e2e testing as it allows verification of deep node selection and proper rendering of leaf nodes.
Let's verify if there are other similar examples that might need consistent updates:
✅ Verification successful
Initial value change is specific to basic usage demo
The value change from 4 to 10 in basic-usage-composition-api.vue is appropriate and doesn't require changes in other examples. Each demo file serves a different purpose:
- Multiple selection demos use
[9, 6]consistently - Basic usage demo now uses
10for leaf node selection - Other variants use empty string
''for their specific demonstration purposes
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find other tree-select examples that might need similar updates
# Look for tree-select examples with initial value declarations
rg -l "tiny-tree-select.*v-model" examples/sites/demos/pc/app/tree-select/
Length of output: 1196
Script:
#!/bin/bash
# Let's check the initial values in these files to ensure consistency
rg -A 2 "const value = ref\(" examples/sites/demos/pc/app/tree-select/
Length of output: 2438
examples/sites/demos/pc/app/tree-select/basic-usage.spec.ts (1)
11-12: LGTM! Well-structured locators
The locators are well-defined and follow the component's class naming convention.
examples/sites/demos/pc/app/tree-select/copy.spec.ts (1)
1-2: LGTM!
The imports are clean and include only the necessary Playwright test utilities.
examples/sites/demos/pc/app/tree-select/disabled-composition-api.vue (1)
18-18: Verify the intended behavior of disconnected node selection
The initial value [9, 6] selects nodes from different branches of the tree (三级 1-1-1 and 二级 2-2) without their parent nodes. While this might be intentional for testing purposes, it's worth verifying if this represents a valid use case.
Let's check if other test files or components allow selecting child nodes without their parents:
✅ Verification successful
The previous command failed because vue file type wasn't recognized. Let's try a different approach by first finding relevant files and then analyzing their content. This will help us understand:
- If disconnected node selection is a common pattern
- How other test files initialize their tree-select values
- The overall behavior pattern across different test cases
The previous output showed us the files but we need to analyze the actual content, particularly focusing on how tree-select values are initialized in other examples and if there's any documentation about node selection behavior.
The previous commands failed due to file type issues. Let's try to get the raw content of the files to analyze the patterns of node selection and any available documentation about the component's behavior.
Let's check the component documentation and the specific demo file to understand if disconnected node selection is documented or if there are any constraints on node selection behavior.
Disconnected node selection is valid in disabled state demo
The code represents a valid test case for the disabled state of tree-select component where:
- The component is explicitly testing three scenarios: empty, single selection, and multiple selection
- The disconnected node selection
[9, 6]is intentional as it verifies that the disabled state correctly preserves and displays multiple selected nodes, regardless of their hierarchical relationship - This is a demonstration file specifically for testing the disabled state, not a general usage example
The verification shows that:
- The file is part of a set of demos, each testing specific aspects of the tree-select component
- The disconnected selection is appropriate in this context as it's testing the disabled state's display capabilities
- The documentation doesn't impose any restrictions on node selection relationships, allowing both connected and disconnected selections in multiple selection mode
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for similar patterns in other tree-select test files
# Look for ref arrays containing node values 9 or 6
rg -A 5 "ref\(\[.*[69].*\]\)" --type vue
Length of output: 70
Script:
#!/bin/bash
# Search for similar patterns in tree-select test files and demos
# First, find all relevant files
fd "tree-select.*\.vue"
# Then search for ref arrays in these files
rg -l "ref\(\[.*\]\)" $(fd "tree-select.*\.vue")
# Finally, let's look at the actual content of these files to analyze the patterns
fd "tree-select.*\.vue" --exec cat {}
Length of output: 13986
Script:
#!/bin/bash
# Let's specifically look at tree-select examples to understand the selection patterns
fd "tree-select.*\.vue" --exec grep -A 20 "const.*ref\(\[.*\]\)" {}
# Also search for any documentation or comments about node selection behavior
rg -i "tree.*select.*node" --type vue
Length of output: 139
Script:
#!/bin/bash
# Let's try a different approach to examine tree-select examples
# First, get the content of tree-select demo files
fd "tree-select.*\.vue" --exec cat {}
# Also look for any documentation files that might contain information about the component
fd "README|tree-select.*\.md"
Length of output: 1429
Script:
#!/bin/bash
# Let's examine the tree-select documentation and the specific demo file
cat examples/sites/demos/pc/app/tree-select/webdoc/tree-select.en.md
cat examples/sites/demos/pc/app/tree-select/webdoc/tree-select.cn.md
cat examples/sites/demos/pc/app/tree-select/disabled-composition-api.vue
Length of output: 1712
examples/sites/demos/pc/app/tree-select/copy-composition-api.vue (2)
4-4: LGTM! Tree-select implementation follows best practices.
The implementation correctly demonstrates two copy scenarios:
- Basic copying of all tags
- Copying with a custom delimiter
Also applies to: 6-6
Line range hint 1-67: Verify test coverage for copy functionality.
Since this is a demo component for e2e testing, ensure that test cases cover both copy scenarios:
- Copying all tags
- Copying with custom delimiter
✅ Verification successful
Let me check the test file content to verify the test coverage.
Test coverage is complete for both copy scenarios
The test file copy.spec.ts includes comprehensive end-to-end tests covering both scenarios:
- Test for copying all tags with default comma separator
- Test for copying with custom separator '/'
Both tests verify the correct functionality by:
- Interacting with the copy button
- Performing paste operation
- Validating the expected output in the input field
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for corresponding test files and coverage of copy scenarios
# Look for test files related to tree-select copy functionality
fd -e spec.ts -e spec.js "copy" examples/sites/demos/pc/app/tree-select/
# Check if test cases exist for both scenarios
rg -A 5 "copyable|text-split" examples/sites/demos/pc/app/tree-select/
Length of output: 2523
Script:
#!/bin/bash
# Check the content of the test file to verify coverage of both copy scenarios
cat examples/sites/demos/pc/app/tree-select/copy.spec.ts
Length of output: 1103
examples/sites/demos/pc/app/tree-select/size-composition-api.vue (1)
18-18: LGTM! Good choice of initial values for testing purposes.
The initialization of value with [9, 6] is well-suited for e2e testing as it:
- Pre-selects nodes from different levels (second and third) and branches
- Makes size differences more visible with actual content
- Helps validate multiple selection rendering across different sizes
examples/sites/demos/pc/app/tree-select/reference-style-composition-api.vue (1)
25-25: Verify the pre-selected value matches the intended node.
The initial value [9] corresponds to the node "三级 1-1-1" in the tree structure. This pre-selection helps demonstrate the component's behavior with existing selections, which is particularly useful for testing scenarios.
Let's verify that this value exists in other demo files to ensure consistency:
✅ Verification successful
Let me verify the tree data structure to ensure the value 9 corresponds to the correct node.
Let me try a different approach to find the tree data structure.
Let me try one final approach to check the content of the file directly.
Pre-selected value correctly matches the tree node structure
The value [9] corresponds to the node with label: '三级 1-1-1' in the tree data structure, and this value is consistently used across multiple tree-select demo files. The pre-selection is valid and properly demonstrates the component's behavior with existing selections.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for the usage of value 9 across tree-select demo files
# Expected: Find consistent usage of this value in related demo files
rg -l "value: 9" "examples/sites/demos/pc/app/tree-select/"
Length of output: 1195
Script:
#!/bin/bash
# Check the tree data structure in the file
rg -A 20 "const data = \[" examples/sites/demos/pc/app/tree-select/reference-style-composition-api.vue
Length of output: 104
Script:
#!/bin/bash
# Search for tree data structure using ast-grep to find array declarations
ast-grep --pattern 'const $_ = [{
$$$
}]' examples/sites/demos/pc/app/tree-select/reference-style-composition-api.vue
Length of output: 121
Script:
#!/bin/bash
# Get the content of the file to examine the data structure
cat examples/sites/demos/pc/app/tree-select/reference-style-composition-api.vue
Length of output: 1525
examples/sites/demos/pc/app/tree-select/disabled.vue (1)
23-23: Verify consistency with related test files
The initial value for multiple selection has been updated to [9, 6], selecting nodes from different branches ("三级 1-1-1" and "二级 2-2"). While these are valid selections, let's ensure this change is consistent across related test files.
✅ Verification successful
Change in value3 is valid and consistent with demo purpose
The change to value3: [9, 6] is appropriate as it:
- Selects valid nodes from the tree structure ("三级 1-1-1" and "二级 2-2")
- Demonstrates multi-level selection in disabled state
- Shows nodes from different branches, which is a good test case
- Is consistent with the demo's purpose of showing disabled states
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistency in value3 initialization across related files
# Expected: Find all tree-select demo files and their value3 initializations
# Search for all tree-select demo files
echo "Searching for related tree-select demo files..."
fd -e vue . examples/sites/demos/pc/app/tree-select/ --exec grep -l "value3:"
# Check value3 initialization in these files
echo "Checking value3 initialization in found files..."
fd -e vue . examples/sites/demos/pc/app/tree-select/ --exec grep -A 1 "value3:"
Length of output: 460
Script:
#!/bin/bash
# Let's verify the tree structure and node values in the disabled.vue file
echo "Checking the tree data structure in disabled.vue..."
rg -A 30 "const data = \[" examples/sites/demos/pc/app/tree-select/disabled.vue
# Also check if there are any test files related to this demo
echo "Checking for related test files..."
fd "tree-select.*test" .
# Look for any documentation or comments about the demo values
echo "Checking for documentation about demo values..."
rg -B 2 -A 2 "value3" examples/sites/demos/pc/app/tree-select/disabled.vue
Length of output: 793
Script:
#!/bin/bash
# Let's get the complete tree data structure to verify the node values
echo "Getting the complete tree data structure..."
rg -A 50 "treeOp: {" examples/sites/demos/pc/app/tree-select/disabled.vue
# Also check if there are similar demos in the tree-select directory
echo "Checking other tree-select demos..."
fd -e vue . examples/sites/demos/pc/app/tree-select/ --exec head -n 1 {}
Length of output: 632
Script:
#!/bin/bash
# Let's try a different approach to get the tree data structure
echo "Getting the tree data structure..."
rg -l "data: \[" examples/sites/demos/pc/app/tree-select/disabled.vue | xargs cat
# Also get the names of all tree-select demo files for context
echo "Listing all tree-select demo files..."
fd . examples/sites/demos/pc/app/tree-select/ -e vue -x basename {}
Length of output: 2280
examples/sites/demos/pc/app/tree-select/copy.vue (3)
8-8: LGTM! Class addition helps with styling and test selection.
The addition of the copy-value class improves component identification for both styling and testing purposes.
4-4: Consider using separate v-model bindings for each tree-select.
Both tree-select components are bound to the same value property, which could make it difficult to distinguish between their selected values and might cause unexpected behavior when copying.
Consider using separate v-model bindings:
- <tiny-tree-select v-model="value" :tree-op="treeOp" multiple copyable></tiny-tree-select>
- <tiny-tree-select v-model="value" :tree-op="treeOp" multiple copyable text-split="/"></tiny-tree-select>
+ <tiny-tree-select v-model="value1" :tree-op="treeOp" multiple copyable></tiny-tree-select>
+ <tiny-tree-select v-model="value2" :tree-op="treeOp" multiple copyable text-split="/"></tiny-tree-select>Also applies to: 6-6
22-22: Verify that the initial selection matches the intended tree nodes.
The initial value [9, 6] selects nodes from different levels and branches:
9: "三级 1-1-1" (third level)6: "二级 2-2" (second level)
Please ensure this mixed-level selection is intentional and properly handled by the component.
✅ Verification successful
Initial selection values are valid and match the tree structure
The initial value [9, 6] correctly corresponds to existing nodes in the tree data:
9: Maps to "三级 1-1-1" under the path "一级 1 > 二级 1-1"6: Maps to "二级 2-2" under "一级 2"
The component is configured with multiple prop to support multi-selection, and the mixed-level selection is a valid use case as demonstrated in this copy functionality example.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for tests validating mixed-level selection behavior
rg -A 5 "tree-select.*mixed.*level" --type ts
# Search for similar initialization patterns
ast-grep --pattern 'value: [$_]'
Length of output: 82
Script:
#!/bin/bash
# Let's first check the tree data structure in the same file
rg -A 20 "treeData.*=" examples/sites/demos/pc/app/tree-select/copy.vue
# Then search for any tree-select test files or documentation
fd "tree-select.*\.(spec|test)\.(ts|js|vue)" -x cat {}
# Also check the component's implementation for handling multiple selections
rg -A 10 "multiple.*selection" --type vue
Length of output: 204
Script:
#!/bin/bash
# Let's check the content of the file to understand the tree structure
cat examples/sites/demos/pc/app/tree-select/copy.vue
# Search for any tree-select related files
fd "tree-select" --type file
Length of output: 1865
examples/sites/demos/pc/app/tree-select/size.vue (2)
Line range hint 71-80: LGTM! Consistent styling for reliable testing.
The fixed width and consistent spacing will help ensure reliable e2e test results across different environments.
Line range hint 1-80: Verify consistency with other component size demos.
Let's ensure this size demo follows the same pattern as other components for consistent testing.
✅ Verification successful
Based on the search results, I can see that the size demo for tree-select follows a consistent pattern used across many other components in the codebase. The implementation shows:
- All size variants (medium, small, mini) are properly implemented
- The pattern matches other form components like select, input, date-picker etc.
- The size prop values are standardized across components (medium, small, mini)
- The demo includes a default size example for comparison
Size demo implementation is consistent with component library standards
The tree-select size demo follows the established pattern used across the component library, properly demonstrating all size variants with appropriate spacing and labeling. No issues found.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for size demo patterns across components
# Expected: Find similar size demo structures in other components
# Search for other size.vue files in demo directories
fd size.vue examples/sites/demos/pc/app -x echo "Found size demo: {}"
# Check for consistent size prop usage
rg -A 1 'size="(medium|small|mini)"' examples/sites/demos/pc/app/
Length of output: 70061
| const wrap = page.locator('#disabled') | ||
| const select = wrap.locator('.tiny-tree-select').nth(0) | ||
| const input = select.locator('.tiny-input__inner') |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance selector robustness and add proper element waiting
The current element selection could be more robust:
- Add explicit waiting for elements
- Consider using more specific selectors or data-testid attributes
- const wrap = page.locator('#disabled')
- const select = wrap.locator('.tiny-tree-select').nth(0)
- const input = select.locator('.tiny-input__inner')
+ const wrap = await page.waitForSelector('#disabled')
+ const select = await wrap.waitForSelector('.tiny-tree-select:first-child')
+ const input = await select.waitForSelector('.tiny-input__inner', { state: 'attached' })📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const wrap = page.locator('#disabled') | |
| const select = wrap.locator('.tiny-tree-select').nth(0) | |
| const input = select.locator('.tiny-input__inner') | |
| const wrap = await page.waitForSelector('#disabled') | |
| const select = await wrap.waitForSelector('.tiny-tree-select:first-child') | |
| const input = await select.waitForSelector('.tiny-input__inner', { state: 'attached' }) |
| const wrap = page.locator('#native-properties') | ||
| const select = wrap.locator('.tiny-tree-select') | ||
| const input = select.locator('.tiny-input__inner') |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance selector reliability and element waiting strategy
The current selectors might be fragile if CSS classes change. Consider using data-testid attributes and explicit waiting.
Suggested improvements:
- const wrap = page.locator('#native-properties')
- const select = wrap.locator('.tiny-tree-select')
- const input = select.locator('.tiny-input__inner')
+ const wrap = page.locator('[data-testid="native-properties-demo"]')
+ const select = wrap.locator('[data-testid="tree-select"]')
+ const input = select.locator('[data-testid="tree-select-input"]')
+
+ // Wait for elements to be visible
+ await Promise.all([
+ wrap.waitFor({ state: 'visible' }),
+ select.waitFor({ state: 'visible' }),
+ input.waitFor({ state: 'visible' })
+ ])You'll need to add corresponding data-testid attributes to your component template.
Committable suggestion skipped: line range outside the PR's diff.
| await input.click() | ||
| await treeNode.filter({ hasText: /^二级 2-1$/ }).click() | ||
| await expect(input).toHaveValue('二级 2-1') | ||
| await input.click() | ||
| await expect(treeNode.filter({ hasText: /^二级 2-1$/ })).toHaveClass(/is-current/) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance test coverage with additional assertions and edge cases.
While the current test verifies basic functionality, consider adding:
- Verification of dropdown visibility after clicks
- Error handling for non-existent items
- Multiple selection scenarios if supported
Add these assertions to make the test more robust:
await input.click()
+ await expect(dropdown).toBeVisible()
await treeNode.filter({ hasText: /^二级 2-1$/ }).click()
await expect(input).toHaveValue('二级 2-1')
+ await expect(dropdown).toBeHidden()
await input.click()
await expect(treeNode.filter({ hasText: /^二级 2-1$/ })).toHaveClass(/is-current/)
+
+ // Add error case
+ await input.fill('non-existent-item')
+ await expect(dropdown.locator('.tiny-tree-node')).toHaveCount(0)Committable suggestion skipped: line range outside the PR's diff.
| const wrap = page.locator('#multiple') | ||
| const select = wrap.locator('.tiny-tree-select').nth(0) | ||
|
|
||
| const dropdown = page.locator('body > .tiny-select-dropdown') | ||
| const suffixSvg = select.locator('.tiny-input__suffix .tiny-base-select__caret') | ||
| const treeNode = dropdown.locator('.tiny-tree-node') | ||
| const checkedTreeNodes = dropdown.locator('.tiny-tree-node.is-checked') | ||
| const tag = select.locator('.tiny-tag') |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using data-testid attributes for more maintainable selectors.
The current selectors rely heavily on CSS classes which could make the tests brittle if the styling changes. Consider using data-testid attributes for test stability.
Example implementation:
- const select = wrap.locator('.tiny-tree-select').nth(0)
+ const select = wrap.locator('[data-testid="tree-select"]')
- const dropdown = page.locator('body > .tiny-select-dropdown')
+ const dropdown = page.locator('[data-testid="tree-select-dropdown"]')
- const suffixSvg = select.locator('.tiny-input__suffix .tiny-base-select__caret')
+ const suffixSvg = select.locator('[data-testid="select-caret"]')Committable suggestion skipped: line range outside the PR's diff.
| const checkedTreeNodes = dropdown.locator('.tiny-tree-node.is-checked') | ||
| const tag = select.locator('.tiny-tag') | ||
|
|
||
| await expect(tag).toHaveCount(2) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance initial state verification.
The test only verifies the count of tags but should also verify:
- Which items are initially selected
- The content/text of the selected tags
- The initial state of the dropdown (should be closed)
+ await expect(dropdown).toBeHidden()
await expect(tag).toHaveCount(2)
+ await expect(tag.nth(0)).toHaveText('一级 1')
+ await expect(tag.nth(1)).toHaveText('一级 2-1')📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await expect(tag).toHaveCount(2) | |
| await expect(dropdown).toBeHidden() | |
| await expect(tag).toHaveCount(2) | |
| await expect(tag.nth(0)).toHaveText('一级 1') | |
| await expect(tag.nth(1)).toHaveText('一级 2-1') |
| // 取消选中一个 | ||
| await page.getByRole('treeitem', { name: '三级 1-1-1' }).locator('path').nth(1).click() | ||
| await expect(tag.filter({ hasText: '+ 1' })).toBeHidden() | ||
| await expect(tag).toHaveCount(1) | ||
|
|
There was a problem hiding this comment.
Improve selector robustness and uncheck validation.
The current implementation has some potential issues:
- Using path selector for clicks is fragile
- Missing validation of the unchecked node's state
- await page.getByRole('treeitem', { name: '三级 1-1-1' }).locator('path').nth(1).click()
+ await page.getByRole('treeitem', { name: '三级 1-1-1' })
+ .locator('[data-testid="checkbox"]')
+ .click()
await expect(tag.filter({ hasText: '+ 1' })).toBeHidden()
await expect(tag).toHaveCount(1)
+ // Verify node is actually unchecked
+ await expect(treeNode.nth(2)).not.toHaveClass(/is-checked/)Committable suggestion skipped: line range outside the PR's diff.
| // 再选中2个 | ||
| await page.getByRole('treeitem', { name: '三级 1-1-1' }).locator('path').nth(1).click() | ||
| await page.getByRole('treeitem', { name: '三级 1-1-2' }).locator('path').nth(1).click() | ||
| await expect(tag.filter({ hasText: '+ 4' })).toBeVisible() | ||
| await expect(tag).toHaveCount(2) | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add comprehensive state validation after multiple selections.
The multiple selection test could be more thorough:
- Verify the state of all selected nodes
- Validate the exact content of the collapsed tag
await page.getByRole('treeitem', { name: '三级 1-1-2' }).locator('path').nth(1).click()
await expect(tag.filter({ hasText: '+ 4' })).toBeVisible()
await expect(tag).toHaveCount(2)
+ // Verify all selected nodes are in checked state
+ await expect(checkedTreeNodes).toHaveCount(5)
+ // Verify the first visible tag shows the correct item
+ await expect(tag.first()).toHaveText('三级 1-1-1')
+ // Verify collapsed tag shows correct count
+ await expect(tag.last()).toHaveText('+ 4')Committable suggestion skipped: line range outside the PR's diff.
| test('标签类型', async ({ page }) => { | ||
| page.on('pageerror', (exception) => expect(exception).toBeNull()) | ||
| await page.goto('tree-select#reference-style') | ||
|
|
||
| const wrap = page.locator('#reference-style') | ||
| const select = wrap.locator('.tiny-tree-select').nth(1) | ||
| const tag = select.locator('.tiny-tag') | ||
|
|
||
| // 验证是否有对应类型的类名 | ||
| await expect(tag.nth(0)).toHaveClass(/tiny-tag--warning/) | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance tag type test coverage
The current test only verifies the warning class name. Consider expanding the test coverage:
- Verify tag content
- Test tag interaction (click/close if applicable)
- Validate other tag types if they exist
-test('标签类型', async ({ page }) => {
+test.describe('Tree Select - Tag Styling', () => {
+ test('should render warning tag with correct styling and behavior', async ({ page }) => {
page.on('pageerror', (exception) => expect(exception).toBeNull())
await page.goto('tree-select#reference-style')
const wrap = page.locator('#reference-style')
const select = wrap.locator('.tiny-tree-select').nth(1)
const tag = select.locator('.tiny-tag')
- // 验证是否有对应类型的类名
await expect(tag.nth(0)).toHaveClass(/tiny-tag--warning/)
+ await expect(tag.nth(0)).toBeVisible()
+ await expect(tag.nth(0)).toHaveText(/\w+/) // Verify tag has content
+
+ // Test tag interaction if applicable
+ const closeButton = tag.nth(0).locator('.tiny-tag__close')
+ if (await closeButton.isVisible()) {
+ await closeButton.click()
+ await expect(tag.nth(0)).not.toBeVisible()
+ }
})
+})Committable suggestion skipped: line range outside the PR's diff.
| test('默认尺寸', async ({ page }) => { | ||
| page.on('pageerror', (exception) => expect(exception).toBeNull()) | ||
| await page.goto('tree-select#size') | ||
|
|
||
| const wrap = page.locator('#size') | ||
| const select = wrap.locator('.tiny-tree-select').nth(1) | ||
| const input = select.locator('.tiny-input') | ||
| const tag = select.locator('.tiny-tag') | ||
|
|
||
| await expect(input.locator('.tiny-input__inner')).toHaveCSS('height', '32px') | ||
| await expect(tag.nth(0)).toHaveClass(/tiny-tag--light/) | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Maintain consistency in assertions across size tests
The default size test is missing some assertions that are present in other size tests:
- No explicit input class check
- No boundingBox verification
const input = select.locator('.tiny-input')
const tag = select.locator('.tiny-tag')
+ const { height } = await input.locator('.tiny-input__inner').boundingBox()
+ await expect(input).toHaveClass(/tiny-input-default/)
await expect(input.locator('.tiny-input__inner')).toHaveCSS('height', '32px')
await expect(tag.nth(0)).toHaveClass(/tiny-tag--light/)
+ expect(height).toBeCloseTo(32, 1)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| test('默认尺寸', async ({ page }) => { | |
| page.on('pageerror', (exception) => expect(exception).toBeNull()) | |
| await page.goto('tree-select#size') | |
| const wrap = page.locator('#size') | |
| const select = wrap.locator('.tiny-tree-select').nth(1) | |
| const input = select.locator('.tiny-input') | |
| const tag = select.locator('.tiny-tag') | |
| await expect(input.locator('.tiny-input__inner')).toHaveCSS('height', '32px') | |
| await expect(tag.nth(0)).toHaveClass(/tiny-tag--light/) | |
| }) | |
| test('默认尺寸', async ({ page }) => { | |
| page.on('pageerror', (exception) => expect(exception).toBeNull()) | |
| await page.goto('tree-select#size') | |
| const wrap = page.locator('#size') | |
| const select = wrap.locator('.tiny-tree-select').nth(1) | |
| const input = select.locator('.tiny-input') | |
| const tag = select.locator('.tiny-tag') | |
| const { height } = await input.locator('.tiny-input__inner').boundingBox() | |
| await expect(input).toHaveClass(/tiny-input-default/) | |
| await expect(input.locator('.tiny-input__inner')).toHaveCSS('height', '32px') | |
| await expect(tag.nth(0)).toHaveClass(/tiny-tag--light/) | |
| expect(height).toBeCloseTo(32, 1) | |
| }) |
| test('small 尺寸', async ({ page }) => { | ||
| page.on('pageerror', (exception) => expect(exception).toBeNull()) | ||
| await page.goto('tree-select#size') | ||
|
|
||
| const wrap = page.locator('#size') | ||
| const select = wrap.locator('.tiny-tree-select').nth(2) | ||
| const input = select.locator('.tiny-input') | ||
| const tag = select.locator('.tiny-tag') | ||
| const { height } = await input.locator('.tiny-input__inner').boundingBox() | ||
|
|
||
| await expect(input).toHaveClass(/tiny-input-small/) | ||
| await expect(tag.nth(0)).toHaveClass(/tiny-tag--small tiny-tag--light/) | ||
| expect(height).toBeCloseTo(28, 1) | ||
| }) | ||
|
|
||
| test('mini 尺寸', async ({ page }) => { | ||
| page.on('pageerror', (exception) => expect(exception).toBeNull()) | ||
| await page.goto('tree-select#size') | ||
|
|
||
| const wrap = page.locator('#size') | ||
| const select = wrap.locator('.tiny-tree-select').nth(3) | ||
| const input = select.locator('.tiny-input') | ||
| const tag = select.locator('.tiny-tag') | ||
| const { height } = await input.locator('.tiny-input__inner').boundingBox() | ||
|
|
||
| await expect(input).toHaveClass(/tiny-input-mini/) | ||
| await expect(tag.nth(0)).toHaveClass(/tiny-tag--mini tiny-tag--light/) | ||
| expect(height).toBeCloseTo(28, 1) | ||
| }) |
There was a problem hiding this comment.
💡 Codebase verification
Height expectation for mini size should be 24px instead of 28px
The test for mini size in tree-select appears to have an incorrect height expectation. Looking at other components in the codebase:
- base-select mini size expects 24px height
- select component has a TODO comment indicating mini size should be 24px
- tree-select is the only component expecting 28px for mini size, which is inconsistent
The height expectations should be:
- small size: 28px
- mini size: 24px
🔗 Analysis chain
Verify height expectations for small and mini sizes
Both small and mini sizes expect the same height (28px). This seems unusual as typically mini size would be smaller than small size.
Additionally, consider adding a constant for the expected heights to make maintenance easier:
+const SIZES = {
+ medium: 40,
+ default: 32,
+ small: 28,
+ mini: 28 // Verify if this should be different
+}🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for size-related CSS definitions
rg -A 2 "tiny-input-(small|mini)" --type css
Length of output: 46
Script:
#!/bin/bash
# Let's try searching in all files for size-related class definitions
rg -A 5 "tiny-input-(small|mini)"
# Also search for any CSS/SCSS files that might contain these definitions
fd -e css -e scss -e less
Length of output: 36805
Script:
#!/bin/bash
# Let's check the input component's size definitions in theme files
rg -A 5 "tiny-input-(small|mini)" packages/theme/src/input/
Length of output: 61
Script:
#!/bin/bash
# Let's check the input component's size definitions in the base theme variables
rg -A 10 "input.*height|height.*input" packages/theme/src/base/vars.less
# Also check for size-related tests in other components
rg -B 2 -A 2 "expect\(height\).toBeCloseTo\((24|28)" --type ts
Length of output: 2496
PR
主要给 tree-select 组件所有 demo 补充 E2E 测试用例。
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
Tests