fix: 添加grid column 组件的type="operation" 和 operationConfig的文档, 删除无用缓存代码#4231
fix: 添加grid column 组件的type="operation" 和 operationConfig的文档, 删除无用缓存代码#4231shenjunjian wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds an operation column type to the grid API with ChangesOperation Column Feature Implementation
Sequence DiagramsequenceDiagram
participant User
participant OperationButton as Operation Button
participant Handler as handleItemClick
participant Modal as TinyModal
User->>OperationButton: clicks button
OperationButton->>Handler: call with itemData (row)
Handler->>Handler: extract realName from itemData.itemData.name
Handler->>Modal: show message with realName
Handler->>Handler: toggle row.flag if applicable
Modal-->>User: display confirmation
🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. 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. Comment |
shenjunjian
added
documentation
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
examples/sites/demos/apis/grid.js (2)
2956-2978:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep the anchored column type docs in sync with the new prop contract.
This adds
'operation'togrid-column.type, butIColumnConfiglater in the same file still documentstypeas'index' | 'radio' | 'selection'only on Line 3283. That leaves the prop docs and every API entry that points atIColumnConfigout of sync.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/sites/demos/apis/grid.js` around lines 2956 - 2978, The docs add 'operation' to grid-column.type but IColumnConfig (and any API entries referencing it) still list only 'index' | 'radio' | 'selection'; update IColumnConfig's type union to include 'operation' and ensure any API/type anchors that point to IColumnConfig (e.g., the grid-column.type docs and the API entries using typeAnchorName: 'IColumnConfig' / typeAnchorName: 'IOperationConfig') are updated so the documented type union matches the new prop contract across the file.
2980-2991:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix the
width.pcDemotarget.
operation-configalready points atgrid-operation-column#operation-column, and the registered demo lives underexamples/sites/demos/pc/app/grid/webdoc/grid-operation-column.js.pcDemo: 'operation-column'will not resolve through the usual<section>#<demoId>lookup.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/sites/demos/apis/grid.js` around lines 2980 - 2991, The pc demo target for the width prop is incorrect: update the width.pcDemo value (the property on the object with name: 'width') to point at the actual registered demo id instead of 'operation-column'; set it to the full section#demo id 'grid-operation-column#operation-column' so the normal <section>#<demoId> lookup resolves correctly (this replaces the current pcDemo: 'operation-column' and aligns with the existing operation-config / grid-operation-column registration).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/sites/demos/apis/grid.js`:
- Around line 4382-4394: The IOperationConfig type's buttons entry is incorrect:
make hidden and disabled optional and update the click signature to accept
(event, context) where context contains buttonConfig and the grid params;
specifically update IOperationConfig.buttons to be Array<{ name: string, icon:
Icon, click: (event: Event, ctx: { buttonConfig: any, params?: any }) => void,
hidden?: (row: any) => boolean, class?: string, disabled?: boolean | ((row: any)
=> boolean) }>, and ensure render and other props keep their existing names so
runtime callers match the type.
In `@packages/vue/src/grid/src/cell/src/cell.ts`:
- Around line 1036-1040: handleItemClick currently re-resolves actions by name
which breaks when labels duplicate; update it to prefer the full object payload
when available: if itemData.itemData (or itemData) already contains the button
config object use that directly instead of searching visibleButtons, otherwise
fall back to find by name; ensure you guard the result so you only call
buttonConfig.click(...) when buttonConfig is truthy and pass the same event and
params (preserving the existing window.event fallback).
---
Outside diff comments:
In `@examples/sites/demos/apis/grid.js`:
- Around line 2956-2978: The docs add 'operation' to grid-column.type but
IColumnConfig (and any API entries referencing it) still list only 'index' |
'radio' | 'selection'; update IColumnConfig's type union to include 'operation'
and ensure any API/type anchors that point to IColumnConfig (e.g., the
grid-column.type docs and the API entries using typeAnchorName: 'IColumnConfig'
/ typeAnchorName: 'IOperationConfig') are updated so the documented type union
matches the new prop contract across the file.
- Around line 2980-2991: The pc demo target for the width prop is incorrect:
update the width.pcDemo value (the property on the object with name: 'width') to
point at the actual registered demo id instead of 'operation-column'; set it to
the full section#demo id 'grid-operation-column#operation-column' so the normal
<section>#<demoId> lookup resolves correctly (this replaces the current pcDemo:
'operation-column' and aligns with the existing operation-config /
grid-operation-column registration).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0570da0c-50cd-4a21-aeaf-fcd3d34efce6
📒 Files selected for processing (5)
examples/sites/demos/apis/grid.jsexamples/sites/demos/pc/app/grid/operation-column/operation-column-composition-api.vueexamples/sites/demos/pc/app/grid/operation-column/operation-column.vueexamples/sites/demos/pc/app/grid/webdoc/grid-operation-column.jspackages/vue/src/grid/src/cell/src/cell.ts
| name: 'IOperationConfig', | ||
| type: 'type', | ||
| code: ` | ||
| interface IOperationConfig { | ||
| // 操作列的按钮配置 | ||
| buttons: Array<{name:string, icon:Icon, click:()=>void, hidden:(row)=> boolean, class?:string, disabled?:boolean| (row)=> boolean}> | ||
| // 最多显示按钮数，默认值为3 | ||
| max?: number | ||
| // 自定义操作列渲染函数， 优先级高 | ||
| render?: ({h, buttons, params}) => VNode | ||
| // 禁用时需要添加的class | ||
| disabledClass?:string | ||
| }` |
There was a problem hiding this comment.
IOperationConfig.buttons does not match the runtime API.
The runtime accepts optional hidden/disabled flags and invokes click with (event, { buttonConfig, ...params }), but this type makes hidden mandatory and documents click: () => void. Users following this contract will implement the wrong handler shape.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/sites/demos/apis/grid.js` around lines 4382 - 4394, The
IOperationConfig type's buttons entry is incorrect: make hidden and disabled
optional and update the click signature to accept (event, context) where context
contains buttonConfig and the grid params; specifically update
IOperationConfig.buttons to be Array<{ name: string, icon: Icon, click: (event:
Event, ctx: { buttonConfig: any, params?: any }) => void, hidden?: (row: any) =>
boolean, class?: string, disabled?: boolean | ((row: any) => boolean) }>, and
ensure render and other props keep their existing names so runtime callers match
the type.
| const handleItemClick = (itemData) => { | ||
| // 兼容不同itemData数据类型 | ||
| const realName = itemData?.name || itemData | ||
| // 兼容不同itemData数据类型, 第一段是兼容tiny的事件， 第二段是兼容 aurora design 的事件 | ||
| const realName = itemData?.itemData?.name || itemData?.name || itemData | ||
| const buttonConfig = visibleButtons.find(({ name: buttonName }) => buttonName === realName) | ||
| buttonConfig.click(window.event || {}, { buttonConfig, ...params }) |
There was a problem hiding this comment.
Do not resolve dropdown actions by label text.
DropdownItem now carries the full buttonConfig in itemData, but handleItemClick still re-finds the action by name. If two visible buttons share the same label, the first match wins and the wrong handler runs. Use the object payload when present, and guard the lookup before calling .click.
Suggested fix
const handleItemClick = (itemData) => {
- // 兼容不同itemData数据类型, 第一段是兼容tiny的事件， 第二段是兼容 aurora design 的事件
- const realName = itemData?.itemData?.name || itemData?.name || itemData
- const buttonConfig = visibleButtons.find(({ name: buttonName }) => buttonName === realName)
- buttonConfig.click(window.event || {}, { buttonConfig, ...params })
+ // 兼容不同 itemData 数据类型，优先使用完整配置对象，避免按名称匹配误命中
+ const buttonConfig =
+ itemData?.itemData ||
+ itemData?.name && visibleButtons.find(({ name: buttonName }) => buttonName === itemData.name) ||
+ visibleButtons.find(({ name: buttonName }) => buttonName === itemData)
+
+ if (buttonConfig?.click) {
+ buttonConfig.click(window.event || {}, { buttonConfig, ...params })
+ }
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/vue/src/grid/src/cell/src/cell.ts` around lines 1036 - 1040,
handleItemClick currently re-resolves actions by name which breaks when labels
duplicate; update it to prefer the full object payload when available: if
itemData.itemData (or itemData) already contains the button config object use
that directly instead of searching visibleButtons, otherwise fall back to find
by name; ensure you guard the result so you only call buttonConfig.click(...)
when buttonConfig is truthy and pass the same event and params (preserving the
existing window.event fallback).
2 participants
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
New Features
Documentation
Refactor