-
Notifications
You must be signed in to change notification settings - Fork 342
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
fix: injectPlaceholder compact Template Node #1099
fix: injectPlaceholder compact Template Node #1099
Conversation
WalkthroughThis change updates the conditional logic within the injectPlaceHolder function in the rendering module. The function now checks if the componentName is either defined as a container in the configuration object or explicitly equals "Template" when the children array is empty. No other parts of the code or public declarations have been modified. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant RenderFunction as injectPlaceHolder
Caller->>RenderFunction: Call with (children, componentName, configure)
alt children are empty and (component is container or "Template")
RenderFunction-->>Caller: Return placeholder component
else
RenderFunction-->>Caller: Proceed with normal rendering
end
Suggested labels
Suggested reviewers
Poem
✨ 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 (
|
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 (1)
packages/canvas/render/src/render.ts (1)
144-156
: LGTM! Consider a minor readability improvement.The change correctly extends placeholder injection to Template components, aligning with the PR objectives. The logic is sound and maintains backward compatibility.
Consider extracting the condition into a descriptive variable for better readability:
const injectPlaceHolder = (componentName, children) => { const isEmptyArr = Array.isArray(children) && !children.length + const shouldInjectPlaceholder = (configure[componentName]?.isContainer || componentName === 'Template') - if ((configure[componentName]?.isContainer || componentName === 'Template') && (!children || isEmptyArr)) { + if (shouldInjectPlaceholder && (!children || isEmptyArr)) { return [ { componentName: 'CanvasPlaceholder' } ] } return children }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/canvas/render/src/render.ts
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
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
【问题描述】
组件打开插槽后,不显示 placeholder,无法往插槽里面拖入内容。
【问题分析】
v2.2 版本引入的问题。
CanvasBox
的默认插槽 placeholder 被删除了。CanvasBox
进行渲染。但是由于2的改动,导致 template 节点无法显示 placeholder 插槽【修复方案】
在
injectPlaceHolder
中,增加对Template
节点的判断,如果是Template
节点,且空 children,则插入 placeholderWhat is the current behavior?
Issue Number: #1091
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit