-
Notifications
You must be signed in to change notification settings - Fork 400
fix: Modify the block adding page. Click Select All and search. The data disappears. #1504
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: release/v2.7.x
Are you sure you want to change the base?
Conversation
and search ,the data disappears
WalkthroughThe update modifies the filtering logic in the BlockGroupPanel component so that blocks already selected are no longer excluded from the filtered block list. Additionally, in the SelectAll component, a new computed property Changes
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
and search ,the data disappears
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/common/component/SelectAll.vue (2)
28-28
: Good fix for filtering logic, but consider renaming for clarity.The
filterList
computed property correctly identifies items that are both visible (allItems
) and selected (selected
). This addresses the core issue where the "Select All" checkbox should reflect the state of currently visible items, not all items ever selected.However, the name
filterList
is somewhat misleading as it doesn't filter anything - it finds the intersection of visible and selected items. Consider renaming tovisibleSelectedItems
orselectedVisibleItems
for better clarity.-const filterList = computed(() => props.allItems.filter((item) => props.selected.some((i) => item.id === i.id))) +const visibleSelectedItems = computed(() => props.allItems.filter((item) => props.selected.some((i) => item.id === i.id)))
28-28
: Consider performance optimization for large datasets.The current implementation has O(n*m) complexity due to the nested
filter
andsome
operations. For large datasets, this could impact performance.Consider optimizing with a Set lookup if performance becomes an issue:
-const filterList = computed(() => props.allItems.filter((item) => props.selected.some((i) => item.id === i.id))) +const filterList = computed(() => { + const selectedIds = new Set(props.selected.map(item => item.id)) + return props.allItems.filter(item => selectedIds.has(item.id)) +})
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/common/component/SelectAll.vue
(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: gene9831
PR: opentiny/tiny-engine#1041
File: packages/plugins/datasource/src/DataSourceList.vue:138-138
Timestamp: 2025-01-14T10:06:25.508Z
Learning: PR #1041 in opentiny/tiny-engine is specifically for reverting Prettier v3 formatting to v2, without any logical code changes or syntax improvements.
Learnt from: rhlin
PR: opentiny/tiny-engine#1011
File: packages/canvas/render/src/RenderMain.ts:82-88
Timestamp: 2025-01-14T08:50:50.226Z
Learning: For PR #1011, the focus is on resolving conflicts and migrating code, with architectural improvements deferred for future PRs.
packages/common/component/SelectAll.vue (2)
Learnt from: gene9831
PR: opentiny/tiny-engine#1011
File: packages/configurator/src/router-select-configurator/RouterSelectConfigurator.vue:95-98
Timestamp: 2025-01-14T06:55:59.692Z
Learning: The tiny-select component from @opentiny/vue library ensures selected options are valid internally, requiring no additional validation in the change handler.
Learnt from: gene9831
PR: opentiny/tiny-engine#830
File: packages/common/component/MetaChildItem.vue:50-56
Timestamp: 2024-10-15T02:45:17.168Z
Learning: In `packages/common/component/MetaChildItem.vue`, when checking if `text` is an object in the computed property `title`, ensure that `text` is not `null` because `typeof null === 'object'` in JavaScript. Use checks like `text && typeof text === 'object'` to safely handle `null` values.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: push-check
🔇 Additional comments (2)
packages/common/component/SelectAll.vue (2)
32-32
: Correct logic fix for "Select All" state.The updated logic correctly determines if all visible items are selected by comparing
allItems.length
withfilterList.length
. This ensures the checkbox reflects the state of currently visible items after filtering/searching, which resolves the reported bug.
43-43
: Consistent update to indeterminate state logic.The indeterminate state now correctly uses
filterList.length
to determine if some (but not all) visible items are selected. This maintains consistency with theselectedAll
logic and ensures proper checkbox visual state.
and search ,the data disappears
English | 简体中文
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Background and solution
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