feat(tree-select): [tree-select] add data init and optimize demo docs #2538
Conversation
WalkthroughThe pull request introduces several modifications across various Vue component files related to the tree-select functionality. Key changes include updates to the default values for the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TinyTreeSelect
participant TreeSelectAPI
User->>TinyTreeSelect: Select a node
TinyTreeSelect->>TreeSelectAPI: nodeClick()
TreeSelectAPI-->>TinyTreeSelect: Update selected data
TinyTreeSelect->>User: Display updated selection
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
kagol
changed the title
|
[e2e-test-warn] The title of the Pull request should look like "fix(vue-renderless): [action-menu, alert] fix xxx bug". Please make sure you've read our contributing guide |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (8)
examples/sites/demos/pc/app/tree-select/multiple-composition-api.vue (1)
9-9: Consider adding comments to explain the pre-selected values.Adding a brief comment explaining why these specific nodes are pre-selected would improve the demo's documentation and make it more maintainable.
+// Pre-select "三级 1-1-1" (9) and "二级 2-2" (6) to demonstrate multiple selection const value = ref([9, 6])examples/sites/demos/pc/app/tree-select/basic-usage.vue (1)
14-14: Consider adding a comment to explain the pre-selected valueTo improve maintainability, consider adding a comment explaining that
10corresponds to "三级 1-1-2" and why this specific node was chosen as the default selection.+ // Pre-select "三级 1-1-2" node to demonstrate initial state value: 10,examples/sites/demos/pc/app/tree-select/multiple.vue (1)
14-14: Consider adding comments to explain pre-selected valuesTo improve demo documentation, consider adding a comment explaining why these specific nodes are pre-selected and what they demonstrate.
+ // Pre-select nodes from different levels (level 3 and level 2) + // to demonstrate hierarchical multiple selection capability value: [9, 6],examples/sites/demos/pc/app/tree-select/collapse-tags-composition-api.vue (1)
20-20: LGTM! Consider adding a comment explaining the initial selection.The initial value
[9, 6]is well-chosen for demonstrating tag collapsing functionality, as it includes both a deep-nested selection and a long-text selection.Consider adding a comment explaining the selection:
+// Initialize with "三级 1-1-1" and a long text node to demonstrate tag collapsing const value = ref([9, 6])examples/sites/demos/pc/app/tree-select/size.vue (1)
7-8: Further optimize the size demonstration orderWhile the reordering is a step in the right direction, consider these improvements for better clarity and user experience:
- Since 'default' and 'medium' sizes are equivalent, consider removing one to avoid redundancy
- Maintain a clear size progression from largest to smallest
Consider this structure:
- <p>medium</p> - <tiny-tree-select v-model="value" size="medium" multiple :tree-op="treeOp"></tiny-tree-select> - <p>默认</p> - <tiny-tree-select v-model="value" multiple :tree-op="treeOp"></tiny-tree-select> - <p>small</p> - <tiny-tree-select v-model="value" size="small" multiple :tree-op="treeOp"> </tiny-tree-select> - <p>mini</p> - <tiny-tree-select v-model="value" size="mini" multiple :tree-op="treeOp"> </tiny-tree-select> + <p>默认 (Medium)</p> + <tiny-tree-select v-model="value" multiple :tree-op="treeOp"></tiny-tree-select> + <p>Small</p> + <tiny-tree-select v-model="value" size="small" multiple :tree-op="treeOp"> </tiny-tree-select> + <p>Mini</p> + <tiny-tree-select v-model="value" size="mini" multiple :tree-op="treeOp"> </tiny-tree-select>examples/sites/demos/pc/app/tree-select/collapse-tags.vue (1)
25-25: Consider adding comments to explain the pre-selected values.To improve maintainability, consider adding a comment explaining that these values were chosen to demonstrate collapsible behavior with varied label lengths (short: "三级 1-1-1" and long: OpenTiny description).
+ // Pre-selected values to demonstrate collapsible behavior: + // 9: "三级 1-1-1" (short label) + // 6: "OpenTiny..." (long label) value: [9, 6],packages/vue/src/tree-select/src/pc.vue (1)
19-25: Consider adding test cases for the filter and node selection functionality.The addition of filter-node-method and node-click handler enhances the component's functionality. To ensure reliability:
- Add test cases for filtering behavior
- Add test cases for node selection in both single and multiple modes
- Document the expected behavior in the component's documentation
packages/renderless/src/tree-select/index.ts (1)
3-5: Ensure consistency in code comments languageThe code comments are currently written in Chinese. For consistency and to accommodate international collaboration, please translate the comments into English.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (11)
examples/sites/demos/pc/app/tree-select/basic-usage-composition-api.vue(1 hunks)examples/sites/demos/pc/app/tree-select/basic-usage.vue(1 hunks)examples/sites/demos/pc/app/tree-select/collapse-tags-composition-api.vue(1 hunks)examples/sites/demos/pc/app/tree-select/collapse-tags.vue(1 hunks)examples/sites/demos/pc/app/tree-select/multiple-composition-api.vue(1 hunks)examples/sites/demos/pc/app/tree-select/multiple.vue(1 hunks)examples/sites/demos/pc/app/tree-select/size-composition-api.vue(1 hunks)examples/sites/demos/pc/app/tree-select/size.vue(1 hunks)packages/renderless/src/tree-select/index.ts(3 hunks)packages/renderless/src/tree-select/vue.ts(2 hunks)packages/vue/src/tree-select/src/pc.vue(1 hunks)
🧰 Additional context used
🪛 Biome
packages/renderless/src/tree-select/vue.ts
[error] 9-9: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (16)
examples/sites/demos/pc/app/tree-select/basic-usage-composition-api.vue (1)
9-9: LGTM! Verify the default selection.
The initialization of value to 4 is valid and corresponds to "二级 1-1" in the tree structure. This change aligns with the PR objectives to fix initialization issues.
examples/sites/demos/pc/app/tree-select/multiple-composition-api.vue (1)
9-9: LGTM! Please verify the pre-selected values.
The initialization with [9, 6] (corresponding to "三级 1-1-1" and "二级 2-2") aligns with the PR objectives to fix initialization issues. Please verify that these specific values effectively demonstrate the intended multiple selection behavior in the demo.
examples/sites/demos/pc/app/tree-select/basic-usage.vue (1)
14-14: LGTM! Initialization improvement
The change from empty string to 10 provides a better demonstration of the component by showing a pre-selected state. This value corresponds to the "三级 1-1-2" node, which is a good choice for demonstrating the tree-select's capabilities.
examples/sites/demos/pc/app/tree-select/multiple.vue (1)
14-14: LGTM! Pre-selected values improve demo clarity
The initialization with [9, 6] effectively demonstrates the multiple selection capability by pre-selecting "三级 1-1-1" and "二级 2-2" nodes.
packages/renderless/src/tree-select/vue.ts (4)
1-3: LGTM! Imports and exports are well-organized.
The new utility imports align with the PR objectives to fix initialization issues, and the API surface remains stable despite reordering.
5-5: LGTM! Appropriate lifecycle hook addition.
Adding onMounted to the function signature is a good change that supports proper component initialization.
18-25: LGTM! Well-structured API composition.
The API object is properly composed with all necessary methods, and each method receives appropriate dependencies. The new methods support the PR objectives for improving initialization and optimization.
Line range hint 28-34: Consider watch cleanup in component unmount.
While the watch implementation is correct, consider whether cleanup is needed when the component unmounts, especially if the watch might affect external state or resources.
examples/sites/demos/pc/app/tree-select/size-composition-api.vue (1)
Line range hint 1-65: Implementation looks good overall!
The component follows Vue 3 best practices using the Composition API, has proper type definitions, and maintains consistent styling.
examples/sites/demos/pc/app/tree-select/collapse-tags.vue (1)
25-25: LGTM! Initialization fixes the collapsible label demo issue.
The pre-selected values [9, 6] properly demonstrate the component's behavior with both short and long labels, effectively testing the collapsible functionality.
packages/vue/src/tree-select/src/pc.vue (3)
7-9: LGTM! Props ordering change looks good.
The reordering of filterable and multiple props maintains functionality while improving consistency.
14-16: Verify initialization behavior in all scenarios.
The changes to current-node-key and default-checked-keys look correct and should address the initialization issues mentioned in the PR objectives. However, please verify the behavior in:
- Single selection mode
- Multiple selection mode
- When switching between modes
Line range hint 1-29: Add tests and update documentation as mentioned in PR objectives.
The implementation looks good, but as noted in the PR objectives, tests and documentation updates are still pending. Please ensure:
- Add unit tests for the modified initialization logic
- Update component documentation to reflect the changes
- Add examples demonstrating proper usage in different scenarios (single/multiple selection, filtering)
Would you like help with generating test cases or documentation updates?
packages/renderless/src/tree-select/index.ts (3)
121-127: Verify that state.selected is properly defined
In getCheckedData, when state.selected is not an array, the code accesses state.selected[props.valueField]. If state.selected is undefined or does not have the props.valueField property, this could cause a runtime error. Please verify that state.selected is properly defined and contains the expected properties.
138-158: Initialize initialNodes properly in multiple selection
In the multiple selection block, if api.getPluginOption(value) returns an empty array, concatenating it to initialNodes may not include the expected nodes. Ensure that initialNodes accumulates all valid nodes, and handle cases where option could be empty to prevent unintended behavior.
101-107:
Handle potential undefined selNode in getPluginOption
In getPluginOption, if no matching node is found, selNode will be undefined. Accessing selNode.currentLabel without checking can lead to runtime errors. Please ensure selNode is defined before accessing its properties.
Apply this diff to handle undefined selNode:
const selNode = find(sourceData, (item) => item[valueField] === value)
const items = []
if (selNode) {
selNode.currentLabel = selNode[textField]
items.push(selNode)
}
+ else {
+ // Handle the case when selNode is undefined, if necessary
+ }Likely invalid or redundant comment.
| childrenName: computed(() => (props.treeOp.props && props.treeOp.props.children) || 'children'), | ||
| currentKey: props.modelValue, | ||
| defaultCheckedKeys: [], | ||
| remoteData: [], | ||
| treeData: props.treeOp.data, | ||
| value: computed(() => props.modelValue) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve props access safety with optional chaining.
The childrenName computed property should use optional chaining to safely handle cases where props.treeOp.props might be undefined.
- childrenName: computed(() => (props.treeOp.props && props.treeOp.props.children) || 'children'),
+ childrenName: computed(() => props.treeOp.props?.children || 'children'),📝 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.
| childrenName: computed(() => (props.treeOp.props && props.treeOp.props.children) || 'children'), | |
| currentKey: props.modelValue, | |
| defaultCheckedKeys: [], | |
| remoteData: [], | |
| treeData: props.treeOp.data, | |
| value: computed(() => props.modelValue) | |
| childrenName: computed(() => props.treeOp.props?.children || 'children'), | |
| currentKey: props.modelValue, | |
| defaultCheckedKeys: [], | |
| remoteData: [], | |
| treeData: props.treeOp.data, | |
| value: computed(() => props.modelValue) |
🧰 Tools
🪛 Biome
[error] 9-9: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
| <p>small</p> | ||
| <tiny-tree-select v-model="value" size="small" multiple :tree-op="treeOp"> </tiny-tree-select> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Reorder size variants from largest to smallest
The current order of size variants doesn't follow a logical progression. To improve user comprehension, consider reordering the components from largest to smallest size:
- medium
- default
- small
- mini
Apply this reordering to match the PR objective of displaying options from large to small:
<p>medium</p>
<tiny-tree-select v-model="value" size="medium" multiple :tree-op="treeOp"></tiny-tree-select>
<p>默认</p>
<tiny-tree-select v-model="value" multiple :tree-op="treeOp"></tiny-tree-select>
- <p>small</p>
- <tiny-tree-select v-model="value" size="small" multiple :tree-op="treeOp"> </tiny-tree-select>
<p>mini</p>
<tiny-tree-select v-model="value" size="mini" multiple :tree-op="treeOp"> </tiny-tree-select>
+ <p>small</p>
+ <tiny-tree-select v-model="value" size="small" multiple :tree-op="treeOp"> </tiny-tree-select>📝 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.
| <p>small</p> | |
| <tiny-tree-select v-model="value" size="small" multiple :tree-op="treeOp"> </tiny-tree-select> | |
| <p>medium</p> | |
| <tiny-tree-select v-model="value" size="medium" multiple :tree-op="treeOp"></tiny-tree-select> | |
| <p>默认</p> | |
| <tiny-tree-select v-model="value" multiple :tree-op="treeOp"></tiny-tree-select> | |
| <p>mini</p> | |
| <tiny-tree-select v-model="value" size="mini" multiple :tree-op="treeOp"> </tiny-tree-select> | |
| <p>small</p> | |
| <tiny-tree-select v-model="value" size="small" multiple :tree-op="treeOp"> </tiny-tree-select> |
| data.forEach((node) => { | ||
| node.pId = pId | ||
| nodes.push(node) | ||
|
|
||
| if (node[state.childrenName] && node[state.childrenName].length > 0) { | ||
| getChild(node[state.childrenName], node[props.valueField]) | ||
| } | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid mutating input data in getTreeData function
The getTreeData function mutates the input data by assigning node.pId = pId. This could lead to unexpected side effects if the original data is used elsewhere. Consider creating a new node object to prevent data mutation.
Apply this diff to avoid mutating the input data:
const getChild = (data, pId) => {
data.forEach((node) => {
+ const newNode = { ...node, pId }
+ nodes.push(newNode)
nodes.push(node)
- node.pId = pId
- nodes.push(node)
- if (node[state.childrenName] && node[state.childrenName].length > 0) {
+ if (newNode[state.childrenName] && newNode[state.childrenName].length > 0) {
getChild(newNode[state.childrenName], newNode[props.valueField])
}
})
}Committable suggestion skipped: line range outside the PR's diff.
| if (!state.value || state.value.length === 0) return | ||
|
|
||
| if (props.multiple) { | ||
| let initialNodes = [] | ||
| if (Array.isArray(state.value)) { | ||
| state.value.forEach((value) => { | ||
| const option = api.getPluginOption(value) | ||
| initialNodes = initialNodes.concat(option) | ||
| }) | ||
| } | ||
|
|
||
| vm.$refs.baseSelectRef.updateSelectedData( | ||
| initialNodes.map((node) => { | ||
| return { | ||
| ...node, | ||
| currentLabel: node[props.textField], | ||
| value: node[props.valueField], | ||
| isTree: true | ||
| } | ||
| }) | ||
| ) | ||
|
|
||
| state.defaultCheckedKeys = api.getCheckedData()[0] | ||
| } else { | ||
| const data = api.getPluginOption(state.value)[0] | ||
| vm.$refs.baseSelectRef.updateSelectedData({ | ||
| ...data, | ||
| currentLabel: data[props.textField], | ||
| value: data[props.valueField], | ||
| state: { | ||
| currentLabel: data[props.textField] | ||
| } | ||
| }) | ||
|
|
||
| state.currentKey = data[props.valueField] | ||
| } |
There was a problem hiding this comment.
Safely handle undefined data in mounted function
In the mounted function, api.getPluginOption(state.value) may return an empty array if no matching node is found, resulting in data being undefined. Accessing properties of data without checking could lead to runtime errors. Please ensure data is defined before proceeding.
Apply this diff to handle potential undefined data:
const data = api.getPluginOption(state.value)[0]
+ if (data) {
vm.$refs.baseSelectRef.updateSelectedData({
...data,
currentLabel: data[props.textField],
value: data[props.valueField],
state: {
currentLabel: data[props.textField]
}
})
state.currentKey = data[props.valueField]
+ } else {
+ // Handle the case when data is undefined
+ }Committable suggestion skipped: line range outside the PR's diff.
PR
主要解决 tree-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
tiny-tree-selectcomponent updated to improve user experience with pre-selected nodes.Bug Fixes
Documentation