fix(tree): [tree] fix parent node toggle when click child node margin #2501
Conversation
WalkthroughThe pull request introduces several modifications to the tree component's functionality, primarily in the Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
packages/renderless/src/tree-node/index.ts (3)
89-94: Add null check for event target.The implementation effectively prevents unintended parent node collapse when clicking whitespace. However, consider adding a null check for the event target to improve robustness.
- if (contentElm && !contentElm.contains(e?.target)) { + if (contentElm && e?.target && !contentElm.contains(e.target)) {
Line range hint
441-456: Consider improvements to icon configuration.The implementation is solid but could benefit from these enhancements:
- Extract icon names as constants to improve maintainability
- Translate Chinese comments to English for better international collaboration
+const ICON_NAMES = { + DEFAULT: 'icon-chevron-right', + EXPANDED: 'icon-expand', + COLLAPSE: 'icon-put-away' +} export const computedExpandIcon = ({ designConfig }) => (treeRoot, state) => { if (state.tree.icon) { return state.tree.icon } - // tiny 新增的判断。 显示线时强制切换图标，仅smb定制了 + // Force icon switch when showing lines (SMB customization only) if (treeRoot.showLine) { - const expandIcon = designConfig?.icons?.expanded || 'icon-expand' - const collapseIcon = designConfig?.icons?.collapse || 'icon-put-away' + const expandIcon = designConfig?.icons?.expanded || ICON_NAMES.EXPANDED + const collapseIcon = designConfig?.icons?.collapse || ICON_NAMES.COLLAPSE return state.expanded ? expandIcon : collapseIcon } - return 'icon-chevron-right' + return ICON_NAMES.DEFAULT }
Line range hint
457-461: Enhance indentation calculation robustness.The implementation works but could benefit from these improvements:
- Add radix to parseInt
- Validate for negative values
- Use template literals for better readability
export const computedIndent = () => ({ node, showLine }, { tree }) => { - return (node.level > 1 ? 1 : 0) * (parseInt(tree.indent) + (showLine ? 8 : 0)) + parseInt(tree.baseIndent) + 'px' + const level = node.level > 1 ? 1 : 0; + const indent = Math.max(0, parseInt(tree.indent, 10)); + const baseIndent = Math.max(0, parseInt(tree.baseIndent, 10)); + const lineSpacing = showLine ? 8 : 0; + return `${level * (indent + lineSpacing) + baseIndent}px`; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
packages/renderless/src/tree-node/index.ts(1 hunks)packages/vue/src/tree/package.json(1 hunks)packages/vue/src/tree/src/tree-node.vue(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/vue/src/tree/package.json
🔇 Additional comments (2)
packages/vue/src/tree/src/tree-node.vue (2)
50-50: LGTM: Content ref addition helps fix margin click issues.
Adding the content ref enables precise click target detection, which aligns with the PR objective of preventing unintended parent node toggles when clicking child node margins.
Line range hint 94-102: Verify checkbox click behavior after removing @click.stop.
The removal of @click.stop from the checkbox allows event propagation. While this change is part of fixing the margin click issues, we should verify that:
- Checkbox clicks still work as expected
- Checkbox clicks don't trigger unintended parent node toggles
Let's check for any similar click.stop patterns in the codebase:
✅ Verification successful
Based on the search results, I can now make a determination about the checkbox click.stop removal:
Removal of @click.stop from checkbox is safe and intentional
The codebase search reveals:
-
The checkbox click handling is properly managed through the component's internal logic in
packages/vue/src/tree/src/tree-node.vueand related renderless code. -
The parent node click handling is separately controlled by
@click.stop="handleClick"on the tree node content wrapper, which prevents event bubbling for the node click itself. -
The comment "tiny: 去掉 @click.stop的绑定" indicates this was an intentional change to fix click handling behavior.
-
The checkbox component has its own click handling that manages propagation appropriately through its renderless architecture, making the extra @click.stop unnecessary and potentially problematic for proper event handling.
The removal aligns with Vue's event handling best practices by allowing natural event propagation while still maintaining proper component-level click handling through the checkbox's internal logic.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for similar click.stop patterns in tree components
# This helps understand the event handling patterns and potential impacts
# Search for click.stop patterns in tree-related files
echo "Searching for click.stop patterns in tree components..."
rg "@click\.stop" --type vue
# Search for checkbox usage in tree components
echo "Searching for checkbox usage in tree components..."
rg "tiny-checkbox" --type vue -A 5
Length of output: 353
Script:
#!/bin/bash
# Let's try without file type filtering since vue type is not recognized
# Search for click.stop patterns
echo "Searching for click.stop patterns..."
rg "@click\.stop"
# Search for checkbox usage and its event handling
echo "Searching for checkbox usage..."
rg "tiny-checkbox" -A 5
# Search for tree-node related click handlers
echo "Searching for tree node click handlers..."
rg "handleClick|toggleExpand|toggleCheck" -A 3
Length of output: 282229
… close #2491
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
close #2491
Summary by CodeRabbit
New Features
refattribute for improved manipulation of tree node content.Bug Fixes
Chores
@opentiny/vue-treepackage from 3.19.1 to 3.19.2.