fix(grid): [grid] fix selection header disabled error when config che… #2647
Conversation
WalkthroughThe pull request introduces several changes to 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/vue/src/grid/package.json(2 hunks)packages/vue/src/grid/src/checkbox/src/methods.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/vue/src/grid/package.json
🔇 Additional comments (1)
packages/vue/src/grid/src/checkbox/src/methods.ts (1)
109-110: Verify the fix resolves the selection header disabled error
The implementation correctly handles the header checkbox disabled state when checkField is configured. However, let's verify that this fixes all edge cases.
Let's verify the usage of headerCheckDisabled across the codebase:
✅ Verification successful
The implementation correctly handles the header checkbox disabled state
The verification confirms that headerCheckDisabled is consistently used across the codebase:
- Properly initialized in the table component
- Correctly set in checkbox methods based on
checkMethodevaluation - Appropriately bound to the header checkbox's disabled state in the cell component
- Used in conjunction with other selection states (isAllSelected, isIndeterminate)
The changes align with the existing pattern and should effectively fix the selection header disabled error when configuring checkField.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for all usages of headerCheckDisabled to ensure consistent behavior
# Search for headerCheckDisabled usage
rg -A 3 'headerCheckDisabled'
# Search for related template bindings
ast-grep --pattern 'template {
$$$
:disabled="headerCheckDisabled"
$$$
}'
Length of output: 2675
| this.headerCheckDisabled = | ||
| checkMethod && afterFullData.length && afterFullData.every((row, rowIndex) => !checkMethod({ row, rowIndex })) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor duplicated headerCheckDisabled logic
The headerCheckDisabled logic is duplicated in both the if and else blocks of the checkSelectionStatus method. Additionally, the current implementation could be improved for better readability and performance.
Consider applying this refactor:
- this.headerCheckDisabled =
- checkMethod && afterFullData.length && afterFullData.every((row, rowIndex) => !checkMethod({ row, rowIndex }))
+ // Extract common logic to avoid duplication
+ const isHeaderCheckDisabled = () => {
+ if (!checkMethod || !afterFullData.length) {
+ return false;
+ }
+ return afterFullData.every((row, rowIndex) => !checkMethod({ row, rowIndex }));
+ };
+
+ this.headerCheckDisabled = isHeaderCheckDisabled();This refactor:
- Eliminates code duplication
- Adds explicit null checks
- Avoids unnecessary iteration when
afterFullDatais empty - Improves readability by extracting the logic into a named function
Committable suggestion skipped: line range outside the PR's diff.
…ckField
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Bug Fixes
Chores