fix: optimize block code saving and publishing logic #996
Conversation
WalkthroughThe pull request introduces modifications to two files: Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
Finishing Touches
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 (
|
github-actions
bot
added
bug
![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
🧹 Nitpick comments (1)
packages/plugins/block/src/js/blockSetting.jsx (1)
649-654: Add error logging for missing components.While the defensive programming is good, silently returning when a component is missing could make debugging difficult. Consider logging a warning message.
if (Array.isArray(children)) { children.forEach((child) => { const component = useMaterial().getMaterial(child.componentName) - if (!component) return + if (!component) { + console.warn(`Component not found: ${child.componentName}`) + return + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/common/component/BlockDeployDialog.vue(3 hunks)packages/plugins/block/src/js/blockSetting.jsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (4)
packages/common/component/BlockDeployDialog.vue (2)
84-84: LGTM: Import added for useCanvas.The import is correctly added to support the new save state synchronization feature.
175-176: LGTM: Save state synchronization implemented correctly.The implementation properly synchronizes the save state after successful block publication when the user opts to save the latest data. This fixes the issue where changes weren't being saved despite showing a successful publication status.
Also applies to: 191-193
packages/plugins/block/src/js/blockSetting.jsx (2)
655-677: LGTM: Robust dependency collection logic.The implementation correctly handles:
- NPM package information extraction
- Component name mapping
- Version tracking
- CSS dependencies collection
680-685: LGTM: Proper recursive traversal.The recursive call to process child components is correctly implemented with proper boundary checks.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/common/component/BlockDeployDialog.vue (1)
196-198: Consider adding error handling for setSaved.While the logic correctly combines the save preference with block matching, consider wrapping the
setSavedcall in a try-catch block to handle potential errors gracefully.- if (formState.needToSave && isSameBlock.value) { - setSaved(true) - } + if (formState.needToSave && isSameBlock.value) { + try { + setSaved(true) + } catch (error) { + useNotify({ + type: 'error', + message: '更新保存状态失败' + }) + } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/common/component/BlockDeployDialog.vue(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (4)
packages/common/component/BlockDeployDialog.vue (4)
74-74: LGTM! Import statements are well-organized.The new imports for Vue's
computedAPI and Tiny Engine's hooks (useBlock,useCanvas) are correctly added and align with the component's enhanced functionality.Also applies to: 84-84
175-176: LGTM! Canvas hook is properly integrated.The
setSavedfunction is correctly destructured from theuseCanvashook, enabling proper synchronization of the save button state.
177-180: LGTM! Block comparison logic is robust.The
isSameBlockcomputed property correctly implements the suggested check to ensure save state is only updated for the currently edited block. The use of optional chaining (?.) provides good null safety.
196-198: Tests needed for the new save state logic.The PR mentions that tests haven't been added. Consider adding unit tests to verify:
- Save state is updated only when both conditions are met
- Save state remains unchanged for non-matching blocks
- Error handling behavior (after implementing the suggested try-catch)
Would you like me to help generate test cases for these scenarios?
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?
区块修改并发布时，在勾选 发布成功后保存最新数据 并确认后，Toobar 区域 代码保存按钮 状态同步改变，
正常实现区块代码保存和发布
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Bug Fixes