feat(dialog-select): supports transparent transmission of selectAll, selectChange and radioChange events #3398
Conversation
WalkthroughThe changes update the event handler signatures for multi-grid selection actions in the dialog-select module. Each handler now receives an additional Changes
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. ✨ Finishing Touches
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 (
|
WalkthroughThis PR enhances the Changes
|
![petercat-assistant[bot]](https://avatars.githubusercontent.com/in/875736?s=60&v=4){kind=small}
| @@ -61,7 +61,7 @@ export const queryGridData = | |||
|
|
|||
| export const multiGridSelectAll = | |||
| ({ api, props, state }) => | |||
| ({ selection, checked }) => { | |||
| ({ selection, checked, $table }, event) => { | |||
There was a problem hiding this comment.
Ensure that the $table and event parameters are correctly passed and utilized in the event handlers to avoid potential runtime errors.
| } | ||
|
|
||
| export const multiGridSelectChange = | ||
| ({ api, props, state, vm }) => | ||
| ({ row, checked }) => { | ||
| ({ row, checked, $table }, event) => { |
There was a problem hiding this comment.
Ensure that the $table and event parameters are correctly passed and utilized in the event handlers to avoid potential runtime errors.
| @@ -567,10 +577,15 @@ export const multiTreeRadio = | |||
|
|
|||
| export const multiGridRadioChange = | |||
| ({ props, state }) => | |||
| ({ row }) => { | |||
| ({ row, $table }, event) => { | |||
There was a problem hiding this comment.
Ensure that the $table and event parameters are correctly passed and utilized in the event handlers to avoid potential runtime errors.
![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 (2)
packages/renderless/src/dialog-select/index.ts (2)
62-95: Good implementation of transparent event propagation.The
multiGridSelectAllhandler now correctly forwards both the$tablereference and the original event object to the external handler. This enhancement properly implements the "transparent transmission" feature mentioned in the PR title.One suggestion to consider:
- // 透传配置式表格selectAll事件 + // 透传配置式表格selectAll事件 - Transparently transmit the table selectAll event with original event object and table reference if (props.gridOp?.events?.selectAll) { props.gridOp.events.selectAll({ selection, checked, $table }, event) }
97-143: Consistent implementation of event forwarding.The changes to
multiGridSelectChangefollow the same pattern asmultiGridSelectAll, ensuring consistent behavior across the component.Consider documenting the parameter changes in a JSDoc comment for better developer experience:
export const multiGridSelectChange = ({ api, props, state, vm }) => + /** + * Handles selection changes in the multi-grid component + * @param {Object} params - Selection change parameters + * @param {Object} params.row - The row that was selected/deselected + * @param {boolean} params.checked - Whether the row was selected or deselected + * @param {Object} params.$table - Reference to the table instance + * @param {Event} event - The original DOM event + */ ({ row, checked, $table }, event) => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/renderless/src/dialog-select/index.ts(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: PR E2E Test (pnpm test:e2e3)
🔇 Additional comments (2)
packages/renderless/src/dialog-select/index.ts (2)
64-64:✅ Verification successful
Verify backward compatibility.
The function signatures have been updated to include new parameters (
$tableandevent). These changes might break existing callers if they're directly invoking these functions.Make sure all existing callers are updated to match the new parameter structure:
Also applies to: 99-99, 580-580
🏁 Script executed:
#!/bin/bash # Check for direct invocations of these functions to ensure they're properly updated echo "Checking for direct invocations of the handler functions..." rg "\.multiGridSelectAll\(|\.multiGridSelectChange\(|\.multiGridRadioChange\(" --type tsLength of output: 219
Backward compatibility verified – no direct invocations found
- Searched for
.multiGridSelectAll(,.multiGridSelectChange(, and.multiGridRadioChange(in all TypeScript files; no direct calls were found.- Since no existing callers invoke these handlers directly, adding the
$tableandeventparameters does not introduce breaking changes.
578-589:❓ Verification inconclusive
Verify integration with calling components.
The implementation for
multiGridRadioChangefollows the same pattern as the other handlers. The event transparency feature is now properly implemented for all three event types.Make sure that components calling these methods pass the required
$tableparameter:
🏁 Script executed:
#!/bin/bash # Search for calls to these handlers to verify they pass the $table parameter echo "Searching for calls to the modified event handlers..." rg -A 2 "multiGridSelectAll|multiGridSelectChange|multiGridRadioChange" --type tsLength of output: 2559
🏁 Script executed:
# Inspect how multiGridRadioChange is registered and used in vue.ts rg -C10 "multiGridRadioChange" --type ts packages/renderless/src/dialog-select/vue.ts # Search for where radioChange event is emitted or passed into child components rg -C10 "radioChange" --type ts packages/renderless/src/dialog-select/vue.tsLength of output: 2192
Verify
$tableis passed tomultiGridRadioChangecallers.The new handler correctly forwards
radioChange({ row, $table }, event)toprops.gridOp.events.radioChange, but we didn’t find any usages in the TypeScript files. Please confirm in your Vue templates that every<Grid>(or similar) binding for@radioChangeis invoking the handler with both the event payload and the$tablereference. For example, run:rg -R "radioChange" -g "*.vue"and ensure each occurrence looks something like:
<MyGrid :data="items" @radioChange="api.multiGridRadioChange($event)" ref="tableRef" /> <!-- where $event includes row and $table -->or explicitly passes the component’s table instance:
@radioChange="api.multiGridRadioChange({ row: $event.row, $table: tableRef }, $event)"
feat(dialog-select): 支持透传selectAll、selectChange和radioChange事件
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