-
Notifications
You must be signed in to change notification settings - Fork 401
feat: 添加区块双向绑定属性选择功能到事件表单 #1453
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: develop
Are you sure you want to change the base?
Conversation
在 BlockEventForm.vue 中添加了双向绑定属性选择器,允许用户通过悬浮图标查看并选择可用属性来自动生成v-model事件。
WalkthroughThe Changes
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)
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 (
|
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: 2
🧹 Nitpick comments (1)
packages/plugins/block/src/BlockEventForm.vue (1)
105-119
: Simplify the custom validator logic.The validation logic can be simplified for better readability and maintainability.
eventName: [ { pattern: REGEXP_EVENT_NAME, message: eventNameTip, - validator: (rule: any /* IFormInnerRule */, value: string, callback: (e?: Error) => void) => { - if (isUpdateEvent.value) return callback() - if (!rule.pattern.test(value)) { - callback(new Error(rule.message)) - } else { - callback() - } - }, + validator: (rule: any, value: string, callback: (e?: Error) => void) => { + // Skip validation for update events + if (isUpdateEvent.value) { + return callback() + } + // Apply pattern validation for regular events + const isValid = rule.pattern.test(value) + callback(isValid ? undefined : new Error(rule.message)) + }, trigger: 'change' } ]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/plugins/block/src/BlockEventForm.vue
(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (6)
packages/plugins/block/src/BlockEventForm.vue (6)
42-42
: LGTM: Imports are correctly structured.All new imports are properly organized and necessary for the new functionality.
Also applies to: 44-51
57-59
: LGTM: Component registration is correct.The new components are properly registered and follow the established pattern.
62-66
: LGTM: Reactive state initialization looks good.The reactive state object is well-structured and uses appropriate getter functions for initialization.
96-96
: LGTM: Clean computed property implementation.The
isUpdateEvent
computed property is simple, efficient, and clearly serves its purpose.
136-136
: LGTM: Return statement properly includes new properties.All new reactive properties and methods are correctly included in the return statement.
Also applies to: 143-145
158-186
: LGTM: CSS styles are well-structured and accessible.The styles provide good visual feedback and follow accessibility best practices by properly styling disabled states and hover effects.
- 将 popover 触发方式从 hover 改为 manual,让showPopover正确响应 - 修复不安全的 hasOwnProperty
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: 2
♻️ Duplicate comments (1)
packages/plugins/block/src/BlockEventForm.vue (1)
116-120
: Type-safety & error-handling forusePropertysToBeEvent
still missingThe earlier review already requested a typed signature plus guard logic; the current code is unchanged.
Please address the original suggestion (see past comments).
🧹 Nitpick comments (2)
packages/plugins/block/src/BlockEventForm.vue (2)
5-13
: Popover never closes on outside-click → switch offmanual
triggerWith
trigger="manual"
the popover stays open until the user re-clicks the icon or the collapse watcher fires.
This violates the UX feedback recorded in the PR description (“popup should close when clicking outside”).
Let TinyPopover handle this natively instead of rebuilding it ourselves.-<tiny-popover v-model="state.showPopover" placement="bottom-end" trigger="manual"> +<tiny-popover + v-model="state.showPopover" + placement="bottom-end" + trigger="click" <!-- built-in outside-click handling --> + @clickoutside="state.showPopover = false" +>Small change, large usability win.
177-181
: Redundant selector in:deep
rule
.tiny-input__inner.tiny-input__inner
duplicates the class name.- :deep(.tiny-input__inner.tiny-input__inner) { + :deep(.tiny-input__inner) {Minor tidy-up, no functional impact.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/plugins/block/src/BlockEventForm.vue
(5 hunks)packages/plugins/block/src/BlockSetting.vue
(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/plugins/block/src/BlockSetting.vue
const state = reactive({ | ||
showPopover: false, | ||
propertys: getEditBlockPropertyList(), | ||
events: getEditBlockEvents() | ||
}) |
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.
🛠️ Refactor suggestion
propertys
and events
snapshot on mount → lose reactivity later
Both lists are fetched once and frozen inside state
.
Any subsequent changes (e.g. user edits the block-schema in another panel) won’t propagate to the popover.
-const state = reactive({
- showPopover: false,
- propertys: getEditBlockPropertyList(),
- events: getEditBlockEvents()
-})
+const propertys = computed(() => getEditBlockPropertyList())
+const events = computed(() => getEditBlockEvents())
+
+const state = reactive({
+ showPopover: false
+})
Adapt template/computed usages to propertys.value
/ events.value
.
Fixes stale data without extra watchers.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const state = reactive({ | |
showPopover: false, | |
propertys: getEditBlockPropertyList(), | |
events: getEditBlockEvents() | |
}) | |
// replace the original block: | |
-const state = reactive({ | |
- showPopover: false, | |
- propertys: getEditBlockPropertyList(), | |
- events: getEditBlockEvents() | |
-}) | |
// with: | |
const propertys = computed(() => getEditBlockPropertyList()) | |
const events = computed(() => getEditBlockEvents()) | |
const state = reactive({ | |
showPopover: false | |
}) |
🤖 Prompt for AI Agents
In packages/plugins/block/src/BlockEventForm.vue around lines 65 to 69, the
state object initializes propertys and events by calling
getEditBlockPropertyList() and getEditBlockEvents() once, losing reactivity on
updates. To fix this, change propertys and events to refs holding the lists, and
update all template and computed references to use propertys.value and
events.value to maintain reactivity without adding watchers.
validator: (rule: any /* IFormInnerRule */, value: string, callback: (e?: Error) => void) => { | ||
if (isUpdateEvent.value) return callback() | ||
if (!rule.pattern.test(value)) { | ||
callback(new Error(rule.message)) | ||
} else { | ||
callback() | ||
} | ||
}, | ||
trigger: 'change' |
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.
🛠️ Refactor suggestion
Validation skips all onUpdate:
cases, including malformed names
Currently any string that merely starts with onUpdate:
passes, even onUpdate:
(empty property).
Consider enforcing onUpdate:<validProp>
instead of bypassing the regex completely.
-if (isUpdateEvent.value) return callback()
+if (isUpdateEvent.value) {
+ const matched = /^onUpdate:[a-zA-Z_$][\w$]*$/.test(value)
+ return matched ? callback() : callback(new Error('无效的 onUpdate 事件名'))
+}
Prevents silent acceptance of invalid update events.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
validator: (rule: any /* IFormInnerRule */, value: string, callback: (e?: Error) => void) => { | |
if (isUpdateEvent.value) return callback() | |
if (!rule.pattern.test(value)) { | |
callback(new Error(rule.message)) | |
} else { | |
callback() | |
} | |
}, | |
trigger: 'change' | |
validator: (rule: any /* IFormInnerRule */, value: string, callback: (e?: Error) => void) => { | |
if (isUpdateEvent.value) { | |
const matched = /^onUpdate:[a-zA-Z_$][\w$]*$/.test(value) | |
return matched | |
? callback() | |
: callback(new Error('无效的 onUpdate 事件名')) | |
} | |
if (!rule.pattern.test(value)) { | |
callback(new Error(rule.message)) | |
} else { | |
callback() | |
} | |
}, | |
trigger: 'change' |
🤖 Prompt for AI Agents
In packages/plugins/block/src/BlockEventForm.vue around lines 127 to 135, the
validator function currently skips validation for all strings starting with
"onUpdate:", allowing invalid or empty event names to pass. Modify the validator
to enforce that strings beginning with "onUpdate:" must be followed by a valid
property name matching the regex pattern, rather than bypassing validation
entirely. This ensures only properly formatted update events are accepted.
之前有尝试过,TinyPopover 的 click 也解决不了这问题,监听面板全局的收起变化可以,或者用回TinyPopover 的 hover(确认不会自动关闭),我先按照全局收起变化的方案修改 |
I have tried it before, and the click of TinyPopover cannot solve this problem. You can listen to the global folding changes of the panel, or use the TinyPopover hover (confirm that it will not be closed automatically). I will first modify it according to the global folding changes plan. |
全局收起还不够,如 问题示例1 ,或者是其他的会更改布局的操作,无法得到好的解决,再多加事件监听反而偏离主题了,有什么好的建议吗,舍弃一部分的操作体验使用 hover 可能好一些? |
It is not enough to close the globally. For example, in question example 1, or other operations that will change the layout, which cannot be solved well. Adding more event monitoring will deviate from the topic. Are there any good suggestions? It may be better to abandon some of the operation experience and use hover? |
添加showPopover状态控制悬浮框的显示,通过对 popover内容显隐使 popover 失去焦点,以达成关闭动作
在 BlockEventForm.vue 中添加了双向绑定属性选择器,允许用户通过悬浮图标查看并选择可用属性来自动生成v-model事件。
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
添加区块-事件管理中双向绑定属性的事件,对应schema events 的 onUpdate:eventName


Summary by CodeRabbit
New Features
Improvements