feat(base-select): [base-select] optimize base select and fix some bugs #2532
Conversation
WalkthroughThe pull request introduces several modifications across multiple files related to the dropdown and selection components. Key changes include updates to test cases for disabled options, reordering of template elements in Vue components, and enhancements to the functionality of the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
kagol
changed the title
|
[e2e-test-warn] The title of the Pull request should look like "fix(vue-renderless): [action-menu, alert] fix xxx bug". Please make sure you've read our contributing guide |
kagol
force-pushed
the
kagol/optimize-base-select
branch
from
0ec45b2 to
c0acafd
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (17)
packages/vue/src/base-select/src/pc.vue (2)
Line range hint
390-397: Fix the incorrect usage of@update:modelValueevent handlerThe
@update:modelValueshould pass the new value to the handler function. Currently,@update:modelValue="handleQueryChange(state.query)"invokes the function immediately with the currentstate.queryvalue, which is not the intended behavior. It should be updated to accept the new value directly.Apply the following diff to correct the event handler:
<tiny-input v-if="searchable" input-box-type="underline" v-model="state.query" :placeholder="t('ui.search.placeholder')" class="tiny-select-dropdown__search" - @update:modelValue="handleQueryChange(state.query)" + @update:modelValue="handleQueryChange" >
567-569: Ensure consistent naming conventions for imported componentsThe imported icon components have inconsistent naming conventions. It's recommended to use PascalCase for component names to maintain consistency and improve readability.
Apply the following changes:
// Import statements import { IconClose, IconHalfselect, IconCheck, IconCheckedSur, IconCopy, IconDeltaDown, IconSearch, IconEllipsis, IconChevronUp, - iconAddCircle, - iconLoadingShadow + IconAddCircle, + IconLoadingShadow } from '@opentiny/vue-icon' // Component registration components: { TinyTag, TinyInput, TinyOption, TinyButton, IconClose: IconClose(), TinyScrollbar, IconCopy: IconCopy(), - IconAddCircle: iconAddCircle(), - IconLoadingShadow: iconLoadingShadow(), + IconAddCircle: IconAddCircle(), + IconLoadingShadow: IconLoadingShadow(), // ... }Also applies to: 627-628
packages/renderless/src/base-select/index.ts (3)
Line range hint
480-480: Translate inline comments to EnglishAt line 480, there's an inline comment in Chinese:
// tiny 新增 修复select组件，创建条目选中再创建选中，还是上一次的数据Please translate the comment to English for consistency and better understanding among all team members.
Apply this diff:
-// tiny 新增 修复select组件，创建条目选中再创建选中，还是上一次的数据 +// Fix: Resolve issue where creating and selecting a new item in the select component still shows the previous data
Line range hint
1871-1873: Handle potential undefined 'designConfig.icons'In the
computedGetIconfunction at lines 1871-1873, accessingdesignConfig?.icons.dropdownIconmay result inundefinedificonsis not defined withindesignConfig. To prevent potential errors, add a fallback or validate thatdesignConfig.iconsexists.Apply this diff:
export const computedGetIcon = ({ designConfig, props }) => () => { + const defaultIcon = 'icon-delta-down' return props.dropdownIcon ? { icon: props.dropdownIcon } : { - icon: designConfig?.icons.dropdownIcon || 'icon-delta-down', + icon: designConfig?.icons?.dropdownIcon || defaultIcon, isDefault: true } }
275-277: Consider adding unit tests for new functionalitiesGiven the changes introduced in the codebase, especially new computed properties and methods, it would be beneficial to add unit tests to ensure expected behavior and prevent regressions in the future.
Also applies to: 319-321, 480-480, 1879-1895, 1871-1875, 528-530, 1871-1873
examples/sites/demos/pc/app/base-select/slot-prefix-composition-api.vue (1)
4-4: Consider adding accessibility attributes to the icon.To improve accessibility for screen readers, consider adding an
aria-labelto the location icon.- <tiny-icon-location></tiny-icon-location> + <tiny-icon-location aria-label="Location selector"></tiny-icon-location>examples/sites/demos/pc/app/base-select/size-composition-api.vue (2)
5-6: Consider maintaining a logical size progressionThe current order of size demonstrations (medium -> default -> small -> mini) breaks the conventional size hierarchy. Consider reordering the demonstrations to follow a more intuitive progression: default -> medium -> small -> mini.
<div> + <p>默认</p> + <tiny-base-select v-model="value1" multiple :options="options"></tiny-base-select> <p>medium</p> <tiny-base-select v-model="value2" size="medium" multiple :options="options"></tiny-base-select> - <p>默认</p> - <tiny-base-select v-model="value1" multiple :options="options"></tiny-base-select> <p>small</p> <tiny-base-select v-model="value3" size="small" multiple :options="options"> </tiny-base-select>
Line range hint
31-33: Consider using responsive width for better adaptabilityThe fixed width of 280px might not be optimal for all viewport sizes. Consider using relative units or responsive breakpoints for better adaptability.
.tiny-base-select { - width: 280px; + width: 100%; + max-width: 280px; }packages/renderless/src/tree-select/index.ts (1)
40-41: LGTM! Consider adding TypeScript interfaces.The addition of
isTree: truealigns with the PR objective to fix selection features and matches the pattern used in grid-select withisGrid. This helps distinguish tree-select nodes in the base-select component.Consider defining a TypeScript interface for the node structure to ensure type safety across different selection components:
interface BaseSelectNode { currentLabel: string; value: any; } interface TreeSelectNode extends BaseSelectNode { isTree: true; } interface GridSelectNode extends BaseSelectNode { isGrid: true; }packages/renderless/src/grid-select/index.ts (1)
66-67: LGTM! Addition ofisGridflag improves component interoperability.The addition of the
isGridflag aligns with the standardization of selection data structure across different selection components (e.g., tree-select'sisTreeflag). This enhancement helps downstream components identify and handle grid-based selections appropriately.Consider documenting this data structure standardization pattern in the component's API documentation to help other developers understand the expected shape of selection data across different selection components (base-select, tree-select, grid-select).
examples/sites/demos/pc/app/base-select/slot-default-composition-api.vue (3)
Line range hint
7-18: Simplify tooltip conditional renderingThe current v-if/v-else logic for tooltips can be simplified to improve readability and maintainability.
Consider this more concise approach:
- <template v-for="item in options1"> - <tiny-tooltip v-if="item.tip" :content="item.tip" placement="right" effect="light" :key="item.value"> - <tiny-option :label="item.label" :value="item.value"> - <span class="left">{{ item.label }}</span> - <tiny-tag v-if="item.tag" type="danger" effect="light" size="small">{{ item.tag }}</tiny-tag> - </tiny-option> - </tiny-tooltip> - <tiny-option v-if="!item.tip" :key="item.value" :label="item.label" :value="item.value"> - <span class="left">{{ item.label }}</span> - <tiny-tag v-if="item.tag" type="danger" effect="light" size="small">{{ item.tag }}</tiny-tag> - </tiny-option> - </template> + <template v-for="item in options1" :key="item.value"> + <tiny-option :label="item.label" :value="item.value" v-slot:default="slotProps" + v-bind="$attrs" + v-on="$listeners"> + <tiny-tooltip v-if="item.tip" :content="item.tip" placement="right" effect="light"> + <div> + <span class="left">{{ item.label }}</span> + <tiny-tag v-if="item.tag" type="danger" effect="light" size="small">{{ item.tag }}</tiny-tag> + </div> + </tiny-tooltip> + <template v-else> + <span class="left">{{ item.label }}</span> + <tiny-tag v-if="item.tag" type="danger" effect="light" size="small">{{ item.tag }}</tiny-tag> + </template> + </tiny-option> + </template>
Line range hint
36-57: Consider adding TypeScript interfacesAdding type definitions would improve code maintainability and catch potential errors early.
Consider adding these interfaces:
interface Option { value: string label: string tag?: string tip?: string } interface DoubleRowOption { value: string label: string desc: string }
Line range hint
89-96: Consider making option height more flexibleThe hardcoded height of 54px for double-row options might cause content overflow with longer descriptions or different font sizes.
Consider using min-height instead:
.double-row { &.tiny-select-dropdown .tiny-select-dropdown__wrap { max-height: 224px; .tiny-option { - height: 54px; + min-height: 54px; + height: auto; + padding: 8px 12px; } } }examples/sites/demos/pc/app/base-select/slot-default.vue (2)
4-4: Consider using CSS for spacing instead of<br />tagsUsing
<br />tags for layout spacing is not recommended. Consider using CSS margins or padding for better maintainability and semantic markup.<style lang="less" scoped> + p { + margin-bottom: 1rem; + } + .tiny-base-select { + margin-bottom: 1.5rem; + } </style>Also applies to: 19-19, 22-22
Line range hint
7-18: Simplify tooltip rendering logicThe current template uses separate
v-if/v-elseblocks for tooltip rendering. This can be simplified using a single template with conditional tooltip wrapping.- <template v-for="item in options1"> - <tiny-tooltip v-if="item.tip" :content="item.tip" placement="right" effect="light" :key="item.value"> - <tiny-option :label="item.label" :value="item.value"> - <span class="left">{{ item.label }}</span> - <tiny-tag v-if="item.tag" type="danger" effect="light" size="small">{{ item.tag }}</tiny-tag> - </tiny-option> - </tiny-tooltip> - <tiny-option v-if="!item.tip" :key="item.value" :label="item.label" :value="item.value"> - <span class="left">{{ item.label }}</span> - <tiny-tag v-if="item.tag" type="danger" effect="light" size="small">{{ item.tag }}</tiny-tag> - </tiny-option> - </template> + <template v-for="item in options1" :key="item.value"> + <component :is="item.tip ? 'tiny-tooltip' : 'template'" + v-bind="item.tip ? { content: item.tip, placement: 'right', effect: 'light' } : {}"> + <tiny-option :label="item.label" :value="item.value"> + <span class="left">{{ item.label }}</span> + <tiny-tag v-if="item.tag" type="danger" effect="light" size="small">{{ item.tag }}</tiny-tag> + </tiny-option> + </component> + </template>packages/vue/src/base-select/src/index.ts (1)
347-350: LGTM! Consider adding JSDoc comments for the new props.The new props
allTextandshowAllTextTagare well-defined and follow Vue's prop definition patterns. However, adding JSDoc comments would improve maintainability.Add documentation for the new props:
+ /** + * Text to display when all items are selected + * @default '' + */ allText: { type: String, default: '' }, + /** + * Whether to show the all-text as a tag when all items are selected + * @default false + */ showAllTextTag: { type: Boolean, default: false }packages/renderless/src/base-select/vue.ts (1)
Line range hint
1-600: Well-structured component architectureThe changes maintain the component's architectural patterns:
- Clear separation of concerns between state, API, and watch handlers
- Consistent use of dependency injection
- Proper lifecycle management
- Logical grouping of related functionality
This makes the code maintainable and easier to understand.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (15)
examples/sites/demos/pc/app/base-select/disabled.spec.ts(0 hunks)examples/sites/demos/pc/app/base-select/size-composition-api.vue(1 hunks)examples/sites/demos/pc/app/base-select/size.spec.ts(3 hunks)examples/sites/demos/pc/app/base-select/size.vue(1 hunks)examples/sites/demos/pc/app/base-select/slot-default-composition-api.vue(2 hunks)examples/sites/demos/pc/app/base-select/slot-default.vue(2 hunks)examples/sites/demos/pc/app/base-select/slot-prefix-composition-api.vue(3 hunks)examples/sites/demos/pc/app/base-select/slot-prefix.vue(1 hunks)packages/renderless/src/base-select/index.ts(3 hunks)packages/renderless/src/base-select/vue.ts(4 hunks)packages/renderless/src/grid-select/index.ts(1 hunks)packages/renderless/src/tree-select/index.ts(1 hunks)packages/theme/src/base-select/index.less(0 hunks)packages/vue/src/base-select/src/index.ts(1 hunks)packages/vue/src/base-select/src/pc.vue(8 hunks)
💤 Files with no reviewable changes (2)
- examples/sites/demos/pc/app/base-select/disabled.spec.ts
- packages/theme/src/base-select/index.less
🧰 Additional context used
🪛 Biome
packages/renderless/src/base-select/vue.ts
[error] 274-275: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (27)
packages/vue/src/base-select/src/pc.vue (3)
73-77: Approved: Class binding for 'is-show-tag' is correctly implemented
The addition of the 'is-show-tag' class based on the !state.isShowTagText condition is appropriate and aligns with the intended logic.
126-137: Approved: Implementation of 'showAllTextTag' functionality
The addition of the tiny-tag component when showAllTextTag is true aligns with the desired behavior to display all selected tags as a single tag. The logic and event handling are correctly implemented.
374-376: Verify the fallback component in dynamic component rendering
In the topCreate section, you're using a dynamic component with a fallback to 'icon-add-circle'. Ensure that 'icon-add-circle' is correctly registered as a component or is globally available to prevent runtime errors.
Run the following script to check if 'icon-add-circle' is registered:
✅ Verification successful
Component registration for icon-add-circle is properly handled
The fallback component icon-add-circle is correctly handled in the codebase. The component is properly imported from @opentiny/vue-icon as iconAddCircle and registered in the components section as IconAddCircle. The dynamic component usage with fallback is safe and will work as expected.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for component registration of 'icon-add-circle'
# Expect: 'icon-add-circle' should be registered or globally available
rg -A 5 "components: {" packages/vue/src/base-select/src/pc.vue | rg "'icon-add-circle'"
Length of output: 186
Script:
#!/bin/bash
# Search for icon-add-circle registration or imports
rg -l "icon-add-circle"
# Search for component registration in pc.vue
rg "components:\s*{[\s\S]+?}" packages/vue/src/base-select/src/pc.vue
# Search for any icon registration patterns
rg "import.*Icon" packages/vue/src/base-select/src/pc.vue
# Check if there's a global icon registration
rg -l "Vue\.use.*Icon"
Length of output: 377
Script:
#!/bin/bash
# Check the content of both files that use icon-add-circle
cat packages/vue/src/select/src/pc.vue packages/vue/src/base-select/src/pc.vue | rg -A 5 -B 5 "icon-add-circle"
# Look for component registration block with simpler pattern
rg "components:" -A 10 packages/vue/src/base-select/src/pc.vue
# Check for any icon-related imports or configurations
rg "icon" packages/vue/src/base-select/src/pc.vue
Length of output: 2687
packages/renderless/src/base-select/index.ts (5)
319-321: Conditional assignment should handle empty 'state.selected' array
At lines 319-321, the code checks state.selected?.[0]?.isTree and state.selected?.[0]?.isGrid before assigning state.selected = result. Ensure that when state.selected is an empty array, this condition behaves as expected and does not cause unintended side effects.
322-322: Reset 'state.selectedLabel' when selections are present
At line 322, the code sets state.selectedLabel to an empty string when there are selected items. This ensures that the label is cleared in multi-select scenarios, which aligns with the desired behavior.
Line range hint 1871-1875: Ensure default icon is defined in 'computedGetIcon'
In the computedGetIcon function, if neither props.dropdownIcon nor designConfig?.icons.dropdownIcon are provided, the component falls back to 'icon-delta-down'. Verify that this default icon exists and is correctly imported to prevent potential rendering issues.
Line range hint 528-530: Review the impact of setting 'state.showWarper' to false
At lines 528-530, state.showWarper is set to false when props.remote && props.dropOnlySearch is true. Ensure this does not unintentionally hide necessary UI elements or affect user experience during remote searches.
275-277: Ensure 'option.state.currentLabel' is properly initialized before use
At lines 275-277, the code checks if option.state.currentLabel is falsy and calls api.clearNoMatchValue(''). Verify that option.state.currentLabel is correctly initialized in all cases where option.state is created to prevent unintended clearing of the model value.
Run the following script to ensure that currentLabel is initialized whenever option.state is assigned:
✅ Verification successful
Let me gather more information about how currentLabel is handled in the codebase.
Let me search for how option.state is initialized to better understand the flow.
Based on the gathered information, I can now provide the final response:
The check for option.state.currentLabel is properly handled
The code is correctly checking for option.state.currentLabel after proper initialization. The codebase shows:
option.stateis initialized as an empty object before usecurrentLabelis properly set during:- Option creation/selection
- Data updates through
setSelected - Component initialization
The check at lines 275-277 is a valid fallback mechanism that calls clearNoMatchValue('') when no label is found, which is the intended behavior for handling invalid or unmatched selections.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all instances where 'option.state' is assigned to an object and check for 'currentLabel' initialization.
# Search for assignments to 'option.state' and check for 'currentLabel' property
rg "option\.state\s*=\s*\{" -A5 | rg -B5 "currentLabel"
Length of output: 692
Script:
#!/bin/bash
# Search for all references to currentLabel in the codebase
rg "currentLabel" -B3 -A3
# Also search for any state initialization patterns in base-select
ast-grep --pattern 'option.state.$_ = $_'
Length of output: 47783
Script:
#!/bin/bash
# Search for initialization of option.state
ast-grep --pattern 'option.state = $_'
# Also search for any setSelected related code
rg "setSelected" -B2 -A2
Length of output: 15146
examples/sites/demos/pc/app/base-select/slot-prefix-composition-api.vue (2)
4-4: LGTM! Icon component change is properly implemented.
The icon component change in the prefix slot follows the correct naming convention and is properly integrated within the template structure.
13-13: LGTM! Icon import and instantiation are properly updated.
The changes maintain consistency between the template usage and script setup:
- Proper import from '@opentiny/vue-icon'
- Correct instantiation using composition API pattern
Also applies to: 24-24
examples/sites/demos/pc/app/base-select/slot-prefix.vue (3)
4-4: LGTM! Icon component change looks good.
The change to tiny-icon-location follows the Tiny Vue naming conventions and aligns with the PR objective to improve prefix icon spacing.
1-7: Test multiple selection with collapse-tags.
Since this component uses multiple selection with collapse-tags, please verify that:
- The prefix icon spacing remains consistent when multiple items are selected
- The collapsed tags don't overlap with the prefix icon
- The 280px width provides enough space for the prefix icon and collapsed tags
12-12: Verify the prefix icon spacing after selection.
The icon import and component registration look good. Since this change addresses the prefix icon spacing issue mentioned in the PR objectives, please verify that the spacing appears correct after selecting values.
Also applies to: 18-18
examples/sites/demos/pc/app/base-select/size-composition-api.vue (1)
Line range hint 24-27: Verify identical initialization values across all sizes
All size variants are initialized with the same values ['选项1', '选项2']. While this is valid, consider if using different initial selections for each size would better demonstrate the component's behavior with varying content lengths.
✅ Verification successful
Based on the file content shown, I can see that this is a size variant demo file that demonstrates different sizes of the base-select component. The identical initialization values across all size variants is actually a common and valid pattern in the codebase, as evidenced by similar patterns in other size demo files like autocomplete, currency, dept, and other components that also use consistent initial values across different sizes.
Using identical values is appropriate for size demos
The identical initialization values ['选项1', '选项2'] are intentionally consistent across all size variants because:
- The purpose of this demo is to showcase visual size differences
- Using the same values helps users focus on the size variations without being distracted by different content
- It follows the established pattern seen in other size demo files in the codebase
- The values match available options in the options array, making them valid selections
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if other size demos in the codebase use varied initialization values
rg -l "size.*select.*value.*选项" --type vue
Length of output: 72
Script:
#!/bin/bash
# Search for similar initialization patterns in select components
rg -t js -t ts -t jsx -t tsx -t vue "value.*=.*ref.*选项" -A 3
Length of output: 90
Script:
#!/bin/bash
# Look for the select component demo files
fd "select.*\.(vue|js|ts|jsx|tsx)$"
Length of output: 8204
Script:
#!/bin/bash
# Check content of size-composition-api.vue for context
fd -e vue "size-composition-api.vue" --exec cat {}
Length of output: 55000
packages/renderless/src/tree-select/index.ts (1)
40-41: Verify consistent implementation across selection components
Let's ensure this implementation is consistent with other selection components, particularly the grid-select component mentioned in the AI summary.
✅ Verification successful
Implementation is consistent across selection components ✓
The verification confirms that both tree-select and grid-select components follow the same pattern:
- tree-select uses
isTree: true - grid-select uses
isGrid: true
This implementation maintains consistency in identifying the selection component type across the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent implementation across selection components
# Check for similar isGrid implementation in grid-select
echo "Checking grid-select implementation..."
ast-grep --pattern 'isGrid: true' packages/renderless/src/grid-select/
# Check for any other similar flags in selection components
echo "Checking for other selection type flags..."
rg -g '*.ts' -g '*.vue' 'is(Tree|Grid|Select|List): true' packages/renderless/src/
Length of output: 479
examples/sites/demos/pc/app/base-select/size.vue (1)
5-6: Verify initialization and multiple selection fixes
The PR objectives mention fixing "incorrect initialization values" and "non-functional selection features", but the code shows:
- All selects use the same initial values
['选项1', '选项2'] - Multiple selection appears to be properly configured
Please verify:
- Are these initial values correct for the default size variant?
- Has the multiple selection functionality been tested with these values?
Let's check the test coverage for these scenarios:
#!/bin/bash
# Description: Check test coverage for initialization and multiple selection
# Look for related test files
rg -l "base-select.*size" -g "*.spec.ts"
# Check initialization and multiple selection test cases
rg "multiple.*options|v-model.*value" -g "size.spec.ts"packages/renderless/src/grid-select/index.ts (1)
66-67: Verify handling of isGrid flag in consumers.
Let's ensure this change is properly integrated with all consumers of grid selection data.
✅ Verification successful
Let me analyze the grid-select implementation and its consumers to verify the isGrid flag handling. I'll generate scripts to check this:
Let me try a different approach to search for the flag usage and related patterns:
Based on the search results, I can now provide a final response:
The isGrid flag is properly integrated and handled consistently.
The code analysis reveals:
- The
isGridflag is used consistently in the selection data flow:- Set in grid-select's
selectChangehandler when processing selections - Checked in base-select's logic to handle grid-specific selection data
- Set in grid-select's
- The flag works in conjunction with
isTreeflag to differentiate selection types - The implementation follows the same pattern as tree-select, maintaining consistency across selection components
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check how the isGrid flag is used across the codebase and verify consistency with tree-select
# Check for consumers of grid selection data that might need to handle isGrid
echo "Checking for potential consumers of grid selection data:"
rg -l "selectChange|updateSelectedData|gridCheckedData" --type ts --type vue
# Verify consistency with tree-select implementation
echo -e "\nVerifying consistency with tree-select implementation:"
ast-grep --pattern 'updateSelectedData($$$isTree$$$)'
# Check if there are any other selection components that might need similar flags
echo -e "\nChecking for other selection components:"
fd -e ts -e vue --full-path '.*select.*'
Length of output: 37181
Script:
#!/bin/bash
# Check how isGrid flag is used in grid-select and related components
echo "Checking grid-select implementation and isGrid flag usage:"
rg -A 5 "isGrid" --type ts --type vue
# Check for updateSelectedData implementations to verify consistency
echo -e "\nChecking updateSelectedData implementations:"
ast-grep --pattern 'updateSelectedData($$$) {
$$$
}'
# Check for selectChange handlers that might process grid selection data
echo -e "\nChecking selectChange handlers:"
ast-grep --pattern 'selectChange($$$) {
$$$
}'
Length of output: 484
Script:
#!/bin/bash
# Search for isGrid in TypeScript files specifically
echo "Searching for isGrid usage in TypeScript files:"
rg "isGrid" --type ts
# Check grid-select implementation
echo -e "\nChecking grid-select implementation:"
cat packages/renderless/src/grid-select/index.ts
# Check dialog-select implementation for grid handling
echo -e "\nChecking dialog-select grid handling:"
cat packages/renderless/src/dialog-select/index.ts
# Check for selection data type definitions
echo -e "\nChecking selection data type definitions:"
rg "interface.*Select" --type ts
Length of output: 21894
examples/sites/demos/pc/app/base-select/size.spec.ts (3)
3-3: LGTM! The medium size test case is correctly implemented.
The test case properly validates the medium size variant with appropriate assertions:
- Checks for the medium size class on the input
- Verifies the expected height of 40px
- Validates the correct tag classes
Also applies to: 12-14
17-17: LGTM! The default size test case is correctly implemented.
The test case properly validates the default size variant:
- Verifies the standard height of 32px
- Correctly checks for the light tag class without size modifier
Also applies to: 26-27
Line range hint 3-27: Verify the size hierarchy matches the design system.
The reordering of size tests from default → medium → small → mini follows a logical progression. Please verify that this order matches:
- The component's documentation
- The actual rendered demo
- The design system specifications
✅ Verification successful
Let me gather more specific information about the size specifications from the component's documentation and implementation.
Let me check the actual demo implementation and any related size configuration files to verify the size hierarchy.
Based on the comprehensive analysis of the codebase, I can now provide a final response regarding the size hierarchy verification:
The size hierarchy in the test file correctly matches the design system specifications.
The evidence shows consistent size ordering across the codebase:
- The demo implementation shows the correct order: medium → default → small → mini
- The component's size prop consistently supports the values: medium, small, mini
- The test cases follow the same hierarchy with corresponding CSS height values:
- medium: 40px
- default: 32px
- small: 28px
- mini: 24px
This ordering is consistently implemented across multiple components (Button, Input, Select, etc.) and follows the design system's standard size hierarchy.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the size order is consistent across documentation and demo files
# Check the demo file structure
fd -e vue "size" -x cat {} | grep -A 5 "<tiny-base-select"
# Check documentation
fd "README" -e md -x cat {} | grep -A 10 "size"
Length of output: 1970
Script:
#!/bin/bash
# Check component's documentation and implementation for size specifications
# Look for size-related content in the base-select component's documentation
fd "base-select" -e md -x cat {}
# Check the component's implementation for size-related constants or configurations
ast-grep --pattern 'size: {
$$$
}'
# Also check for any size-related props or configurations
ast-grep --pattern 'props: {
$$$
size: $_,
$$$
}'
Length of output: 376
Script:
#!/bin/bash
# Check the demo implementation and related configuration files
# Look for the size demo file
fd "size" -e vue -e ts -x cat {} | grep -B 5 -A 5 "size"
# Check for any size-related configuration or constant files
rg -B 2 -A 2 "size.*(?:medium|small|mini)" --type ts
# Look for size-related component definitions
ast-grep --pattern 'const $_: PropType<"medium" | "small" | "mini">'
Length of output: 152443
examples/sites/demos/pc/app/base-select/slot-default-composition-api.vue (1)
4-4: LGTM: Improved visual spacing between scenarios
The added line breaks enhance the demo's readability by creating clear visual separation between different scenarios.
Also applies to: 19-19, 22-22
examples/sites/demos/pc/app/base-select/slot-default.vue (2)
Line range hint 1-98: Overall implementation looks good!
The changes successfully implement the required functionality for both single-line options with tooltips and double-row options with descriptions. The code is well-structured and follows Vue best practices.
Line range hint 91-98: Consider flexible height handling for options
The current implementation uses fixed heights for options, which might cause issues with dynamic content or different text lengths.
Let's check if there are any existing issues with long content:
Consider using min-height instead of fixed height and allow for content-based growth:
.double-row {
&.tiny-select-dropdown .tiny-select-dropdown__wrap {
max-height: 224px;
.tiny-option {
- height: 54px;
+ min-height: 54px;
+ height: auto;
+ padding: 8px 0;
}
}
}packages/vue/src/base-select/src/index.ts (1)
347-350: Verify the integration with existing tag-related props.
The new props work alongside existing tag-related props like collapseTags and tagType. Let's verify their interaction.
✅ Verification successful
Based on the search results, I can now provide a conclusive response about the integration of the showAllTextTag prop with existing tag-related functionality:
The showAllTextTag prop is properly integrated with existing tag-related functionality
The code shows clear integration with existing tag-related props:
- Works alongside
collapseTagswith proper conditional rendering - Integrates with
tagTypethrough thestate.getTagTypebinding - Has proper template logic for both collapsed and expanded states
Key integration points:
- When
showAllTextTagis true, it renders a single tag with the "all text" representation - When false, it falls back to the original tag rendering mode
- Properly handles tag closability and disabled states
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for potential conflicts with existing tag-related functionality
# Search for tag-related computed properties and methods
echo "Searching for tag-related computed properties and methods..."
ast-grep --pattern 'computed: {
$$$
$tag$_: {
$$$
}
$$$
}'
# Search for tag-related template usage
echo "Searching for tag-related template usage..."
rg -A 5 'v-if.*tag|v-show.*tag' --type vue
# Search for tag-related tests
echo "Searching for tag-related tests..."
rg -A 5 'describe.*tag|test.*tag|it.*tag' --type spec
Length of output: 657
Script:
#!/bin/bash
# Let's try a different approach to search for tag-related code
# Search for tag-related properties and methods in the component
echo "Searching for tag-related code in the component..."
rg -A 5 "collapseTags|tagType|showAllTextTag" packages/vue/src/base-select/src/
# Search for tag-related template usage with .vue extension
echo "Searching for tag-related template usage..."
rg -A 5 'v-if.*tag|v-show.*tag' --type-add 'vue:*.vue' --type vue
# Search for tag-related tests with proper extension
echo "Searching for tag-related tests..."
rg -A 5 'describe.*tag|test.*tag|it.*tag' packages/vue/src/base-select/__tests__/
Length of output: 27075
packages/renderless/src/base-select/vue.ts (3)
157-159: LGTM: API registrations are consistent
The new functions are properly registered in the API array, maintaining consistency with their imports.
365-366: Verify usage of new computed properties
Let's ensure these computed properties are properly used in the component template.
Run the following to check their usage:
#!/bin/bash
# Search for usage of these computed properties in template files
rg -t vue 'isTagClosable|computedShowTagText' 98-100: Verify implementations of new API functions
The new API functions align with the PR objectives for fixing tag display and panel behavior.
Let's verify their implementations:
✅ Verification successful
All imported functions are properly implemented and used
The verification shows that all three functions are correctly implemented in packages/renderless/src/base-select/index.ts:
hidePanel: Controls panel visibility statecomputedShowTagText: Computes tag text display based onisDisplayOnlystateisTagClosable: Determines if a tag can be closed based onrequiredproperty
The imports are valid and the functions are being used appropriately in the component's API initialization.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for the implementation of these new functions
ast-grep --pattern 'export const $func = $_' | rg -A 5 'hidePanel|computedShowTagText|isTagClosable'
Length of output: 169
Script:
#!/bin/bash
# Search for the implementation of these functions in TypeScript files
rg -t ts "export const (hidePanel|computedShowTagText|isTagClosable)" -A 5
# Also search for their usage to understand the context
rg -t ts "(hidePanel|computedShowTagText|isTagClosable)" --no-filename
Length of output: 2962
| /** | ||
| * 兼容不同主题多选禁用的展示类型 | ||
| * default 和 smb 主题，displayOnly 时显示为 tagText,否则为 tag | ||
| * aurora 主题 displayOnly||disabled 时显示为tagText,否则为 tag | ||
| */ | ||
| export const computedShowTagText = | ||
| ({ state }) => | ||
| () => | ||
| state.isDisplayOnly | ||
|
|
||
| /** | ||
| * 兼容不同主题多选，tag 在disabled 和 required 时是否显示关闭按钮的区别 | ||
| * default 主题 ，禁用显示关闭按钮，required目前和aurora保持一致不显示，待设计图补充时更新 | ||
| * aurora 主题，禁用时无禁用效果，required 时不显示关闭按钮 | ||
| */ | ||
| export const isTagClosable = () => (item) => !item.required | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use consistent language for code comments
The comments added in lines 1879-1895 are in Chinese. To maintain consistency and improve collaboration among the development team, please translate these comments into English.
Apply this diff to update the comments:
-/**
- * 兼容不同主题多选禁用的展示类型
- * default 和 smb 主题，displayOnly 时显示为 tagText,否则为 tag
- * aurora 主题 displayOnly||disabled 时显示为tagText,否则为 tag
- */
+/**
+ * Handle display types for multi-select disabled states across different themes
+ * In default and smb themes, shows tagText when displayOnly is true; otherwise, shows tag
+ * In aurora theme, shows tagText when displayOnly or disabled is true; otherwise, shows tag
+ */
export const computedShowTagText =
({ state }) =>
() =>
state.isDisplayOnly
-/**
- * 兼容不同主题多选，tag 在disabled 和 required 时是否显示关闭按钮的区别
- * default 主题 ，禁用显示关闭按钮，required目前和aurora保持一致不显示，待设计图补充时更新
- * aurora 主题，禁用时无禁用效果，required 时不显示关闭按钮
- */
+/**
+ * Manage tag closability for multi-select across different themes when disabled or required
+ * In default theme, the close button is shown when disabled; currently, required behaves like aurora theme and does not display the close button (pending design updates)
+ * In aurora theme, disabled state has no effect; the close button is not displayed when required
+ */
export const isTagClosable = () => (item) => !item.required📝 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.
| /** | |
| * 兼容不同主题多选禁用的展示类型 | |
| * default 和 smb 主题，displayOnly 时显示为 tagText,否则为 tag | |
| * aurora 主题 displayOnly||disabled 时显示为tagText,否则为 tag | |
| */ | |
| export const computedShowTagText = | |
| ({ state }) => | |
| () => | |
| state.isDisplayOnly | |
| /** | |
| * 兼容不同主题多选，tag 在disabled 和 required 时是否显示关闭按钮的区别 | |
| * default 主题 ，禁用显示关闭按钮，required目前和aurora保持一致不显示，待设计图补充时更新 | |
| * aurora 主题，禁用时无禁用效果，required 时不显示关闭按钮 | |
| */ | |
| export const isTagClosable = () => (item) => !item.required | |
| /** | |
| * Handle display types for multi-select disabled states across different themes | |
| * In default and smb themes, shows tagText when displayOnly is true; otherwise, shows tag | |
| * In aurora theme, shows tagText when displayOnly or disabled is true; otherwise, shows tag | |
| */ | |
| export const computedShowTagText = | |
| ({ state }) => | |
| () => | |
| state.isDisplayOnly | |
| /** | |
| * Manage tag closability for multi-select across different themes when disabled or required | |
| * In default theme, the close button is shown when disabled; currently, required behaves like aurora theme and does not display the close button (pending design updates) | |
| * In aurora theme, disabled state has no effect; the close button is not displayed when required | |
| */ | |
| export const isTagClosable = () => (item) => !item.required |
| <p>默认</p> | ||
| <tiny-base-select v-model="value1" multiple :options="options"></tiny-base-select> |
There was a problem hiding this comment.
💡 Codebase verification
Based on the shell script output, I can see that across various component demos in the codebase, there is a consistent pattern for size ordering. Looking at components like tree-select, select, base-select, etc., the common order is:
- medium
- default
- small
- mini
The current order in base-select size demo does not follow the established convention
The current order in base-select/size.vue places default after medium, which is inconsistent with the pattern seen across other components in the codebase. This should be reordered to maintain consistency with the design system.
🔗 Analysis chain
Verify size ordering convention
The default size variant is now placed after the medium size. Please verify this ordering aligns with your design system conventions, as typically default variants are shown first.
Let's check if this ordering is consistent across other component demos:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check size ordering patterns in other component demos
# Look for size demo files and check their content for size ordering
# Find all size.vue files in component demos
fd 'size.vue$' examples/sites/demos/pc/app -x grep -l "size=" {} | while read -r file; do
echo "=== $file ==="
# Show the order of size variants
grep -A 1 "size=" "$file"
done
Length of output: 300
Script:
#!/bin/bash
# Let's try a different approach to find size ordering patterns
# Search for files containing size demos and examine their content
# First, let's look for size-related demo files
fd 'size.vue$' examples/sites/demos/pc/app -x cat {}
Length of output: 62408
| isDisabled: computed(() => props.disabled || (parent.form || {}).disabled), | ||
| isShowTagText: computed(() => api.computedShowTagText()), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify using optional chaining
The current implementation could be improved to prevent potential undefined errors and improve readability.
Apply this diff:
- isDisabled: computed(() => props.disabled || (parent.form || {}).disabled),
+ isDisabled: computed(() => props.disabled || parent.form?.disabled),📝 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.
| isDisabled: computed(() => props.disabled || (parent.form || {}).disabled), | |
| isShowTagText: computed(() => api.computedShowTagText()), | |
| isDisabled: computed(() => props.disabled || parent.form?.disabled), | |
| isShowTagText: computed(() => api.computedShowTagText()), |
🧰 Tools
🪛 Biome
[error] 274-275: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
PR
主要修复 base-select 组件的以下问题：
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
Release Notes
New Features
showAllTextTagand improved search capabilities in the dropdown.Bug Fixes
Style
base-selectcomponent, particularly for suffix elements.Tests
These updates enhance user experience by improving usability and visual presentation across the component suite.