feat(grid): add expand trigger slot #3518
Conversation
WalkthroughThe changes introduce a customizable expand row trigger for a grid component by adding an Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant GridComponent
participant ColumnConfig
participant CustomSlot
User->>GridComponent: Clicks expand trigger
GridComponent->>ColumnConfig: Checks for 'expand-trigger' slot
alt Slot exists
ColumnConfig->>CustomSlot: Render custom expand trigger (with params)
CustomSlot-->>GridComponent: Custom trigger content
else No slot
GridComponent-->>User: Render default expand icon
end
GridComponent->>User: Toggle row expansion and show expanded content
Poem
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
packages/vue/src/grid/src/cell/src/cell.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-vue". (The package "eslint-plugin-vue" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-vue" was referenced from the config file in ".eslintrc.js » @antfu/eslint-config » @antfu/eslint-config-vue". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. 📜 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
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 (
|
![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
🔭 Outside diff range comments (1)
examples/sites/demos/apis/grid.js (1)
3356-3361: Typo breaks TS literal union
cellis missing quotes.- trigger?: 'default' | 'row' | cell + trigger?: 'default' | 'row' | 'cell'This currently makes the whole
examples/sites/demos/apis/grid.jsfile invalid TypeScript.
♻️ Duplicate comments (1)
examples/sites/demos/pc/app/grid/expand/expand-trigger-slot-composition-api.vue (1)
77-84: SameJSON.stringifyissue as the options-API demoSee previous comment – dropping
JSON.stringifysimplifies the code.
🧹 Nitpick comments (6)
examples/sites/demos/pc/app/grid/webdoc/grid-expand.js (1)
18-28: Document both demo variants & tighten EN descriptionThere is a Composition-API variant (
expand-trigger-slot-composition-api.vue) that is currently undiscoverable from the docs array. Also, the EN description is missing the article “an”.- desc: { - 'zh-CN': ` - <p>通过 <code>expand-trigger</code> 插槽可以自定义展开行图标。</p> - `, - 'en-US': '<p>You can customize the expand row icon through the <code>expand-trigger</code> slot. </p>\n' - }, - codeFiles: ['expand/expand-trigger-slot.vue'] + desc: { + 'zh-CN': '<p>通过 <code>expand-trigger</code> 插槽可以自定义展开行图标。</p>', + 'en-US': '<p>You can customise an expand-row icon through the <code>expand-trigger</code> slot.</p>' + }, + codeFiles: [ + 'expand/expand-trigger-slot.vue', + 'expand/expand-trigger-slot-composition-api.vue' + ]packages/vue/src/grid/src/cell/src/cell.ts (1)
655-666: Nit: misleading variable name & single-state icon
hideExpandactually means “row is expandable”. Renaming would improve readability:- let hideExpand = typeof expandMethod === 'function' ? expandMethod(row) : true + const isExpandable = typeof expandMethod === 'function' ? expandMethod(row) : true(remember to update the two usages below).
Additionally, the fallback icon
<i class="tiny-grid__expand-icon">is static; it does not reflect theexpandActivestate. Consider toggling a modifier class or re-usingiconExpand/iconPutAwayfor visual consistency when no slot is supplied.examples/sites/demos/pc/app/grid/expand/expand-trigger-slot.vue (2)
6-8: Minor: use computed label instead of inline ternaryFor clarity and reusability:
- <tiny-button>{{ $table.hasRowExpand(row) ? '收起' : '展开' }}</tiny-button> + <tiny-button>{{ $table.hasRowExpand(row) ? '收起' : '展开' }}</tiny-button>becomes
<template #expand-trigger="{ row, $table }"> <tiny-button>{{ getTriggerLabel(row, $table) }}</tiny-button> </template>with
methods: { getTriggerLabel(row, $table) { return $table.hasRowExpand(row) ? '收起' : '展开' }, … }Keeps the template lean and avoids duplicating the ternary in the composition-api demo.
85-92: AvoidJSON.stringifyfor plain numbers
rowIndex + 1is an integer; wrapping it withJSON.stringifyis unnecessary:- message: `当前展开行：${JSON.stringify(rowIndex + 1)}`, + message: `当前展开行：${rowIndex + 1}`,examples/sites/demos/apis/grid.js (2)
2970-2983: Slot added but demo link is 404-proneGreat to see
expand-triggerexposed!
Small follow-ups to avoid broken docs:- pcDemo: 'grid-expand#expand-trigger-slot' + pcDemo: 'grid-expand#expand-trigger-slot' // ✅ Make sure this anchor exists in the md/vue demoIf the demo file/anchor doesn’t exist yet, the docs site will render an empty link.
4178-4200:ICellClassNameArgsduplicates much ofIClassNameArgsThe new interface just adds
level,dataand splits column indices. Consider extending instead of duplicating to avoid drift:-interface ICellClassNameArgs { /* … all fields … */ } +interface ICellClassNameArgs extends IClassNameArgs { + level: number + data: IRow[] +}Keeps maintenance simpler while still delivering the extra metadata.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
examples/sites/demos/apis/grid.js(4 hunks)examples/sites/demos/pc/app/grid/expand/expand-trigger-slot-composition-api.vue(1 hunks)examples/sites/demos/pc/app/grid/expand/expand-trigger-slot.spec.ts(1 hunks)examples/sites/demos/pc/app/grid/expand/expand-trigger-slot.vue(1 hunks)examples/sites/demos/pc/app/grid/webdoc/grid-expand.js(1 hunks)packages/vue/src/grid/src/cell/src/cell.ts(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: PR E2E Test (pnpm test:e2e3)
🔇 Additional comments (2)
packages/vue/src/grid/src/cell/src/cell.ts (1)
690-691: Guard against empty trigger slot
[isExpandable && triggerContent]rendersfalsewhen the slot returnsundefined, producing a stray comment node in Vue. Wrap withtriggerContent &&:- [isExpandable && triggerContent] + [isExpandable && triggerContent && triggerContent]Likely an incorrect or invalid review comment.
examples/sites/demos/apis/grid.js (1)
59-62: Anchor change looks good – double-check downstream tooling
typeAnchorNameswitched toICellClassNameArgsand the function signature now uses the same type.
All good as long as:
- The new interface is really exported (it is, see ~4178).
- Doc-site generators & IDE plugins know about the new anchor.
No action required if those pipelines already pass.
| page.on('pageerror', (exception) => expect(exception).toBeNull()) | ||
| await page.goto('grid-expand#expand-trigger-slot') | ||
| await page.locator('#expand-trigger-slot .tiny-grid-body__row').first().locator('.tiny-button').click() | ||
| await expect(page.locator('div').filter({ hasText: '当前展开行：1' }).nth(1)).toBeVisible() | ||
| await expect(page.locator('.tiny-grid-body__expanded-cell')).toHaveText( | ||
| '公司名称：GFD 科技 YX 公司区域：华东区员工数：800公司简介：公司技术和研发实力雄厚，是国家 863 项目的参与者，并被政府认定为“高新技术企业”。' | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
❓ Verification inconclusive
Make the assertions less brittle
nth(1)assumes there are exactly two matching nodes; the test will silently pass even if an unrelated element slips in ahead of the expected one.toHaveText(...)with the full Chinese string will fail on any extra whitespace or line-breaks inserted by i18n tooling or prettifiers.
- await expect(page.locator('div').filter({ hasText: '当前展开行：1' }).nth(1)).toBeVisible()
- await expect(page.locator('.tiny-grid-body__expanded-cell')).toHaveText(
+ await expect(page.locator('div:has-text("当前展开行：1")').first()).toBeVisible()
+ await expect(
+ page.locator('.tiny-grid-body__expanded-cell')
+ ).toContainText('公司名称：GFD 科技 YX 公司')The revised selectors are unambiguous and the partial-text assertion guards against harmless formatting changes.
Make the assertions less brittle
nth(1)assumes there are exactly two matching nodes; the test will silently pass even if an unrelated element slips in ahead of the expected one.toHaveText(...)with the full Chinese string will fail on any extra whitespace or line-breaks inserted by i18n tooling or prettifiers.
- await expect(page.locator('div').filter({ hasText: '当前展开行：1' }).nth(1)).toBeVisible()
- await expect(page.locator('.tiny-grid-body__expanded-cell')).toHaveText(
+ await expect(page.locator('div:has-text("当前展开行：1")').first()).toBeVisible()
+ await expect(
+ page.locator('.tiny-grid-body__expanded-cell')
+ ).toContainText('公司名称：GFD 科技 YX 公司')The revised selectors are unambiguous and the partial-text assertion guards against harmless formatting changes.
📝 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.
| page.on('pageerror', (exception) => expect(exception).toBeNull()) | |
| await page.goto('grid-expand#expand-trigger-slot') | |
| await page.locator('#expand-trigger-slot .tiny-grid-body__row').first().locator('.tiny-button').click() | |
| await expect(page.locator('div').filter({ hasText: '当前展开行：1' }).nth(1)).toBeVisible() | |
| await expect(page.locator('.tiny-grid-body__expanded-cell')).toHaveText( | |
| '公司名称：GFD 科技 YX 公司区域：华东区员工数：800公司简介：公司技术和研发实力雄厚，是国家 863 项目的参与者，并被政府认定为“高新技术企业”。' | |
| ) | |
| page.on('pageerror', (exception) => expect(exception).toBeNull()) | |
| await page.goto('grid-expand#expand-trigger-slot') | |
| await page.locator('#expand-trigger-slot .tiny-grid-body__row').first().locator('.tiny-button').click() | |
| await expect(page.locator('div:has-text("当前展开行：1")').first()).toBeVisible() | |
| await expect( | |
| page.locator('.tiny-grid-body__expanded-cell') | |
| ).toContainText('公司名称：GFD 科技 YX 公司') |
🤖 Prompt for AI Agents
In examples/sites/demos/pc/app/grid/expand/expand-trigger-slot.spec.ts around
lines 4 to 10, the test assertions are brittle because nth(1) assumes exactly
two matching elements and toHaveText uses a full Chinese string that can fail
due to whitespace or formatting changes. To fix this, update the selector to
target the specific element unambiguously without relying on nth(1), and replace
the full text assertion with a partial text match that ignores minor formatting
differences, making the test more robust.
| // 功能列的类型， 'index'行索引，'radio' 单选行， 'selection' 多选行 | ||
| type: 'index' | 'radio' | 'selection' | ||
| // 列id | ||
| id: string | ||
| prop: string | ||
| // 校验规则 | ||
| rules: IValidRules | ||
| // 是否必填 | ||
| required: boolean | ||
| property: string | ||
| title: string | ||
| label: string | ||
| // 列宽度 | ||
| width: string | number | ||
| // 自动分配宽度时的最小宽度 | ||
| minWidth: string | number | ||
| // 是否可以调整列宽 | ||
| resizable: boolean | ||
| fixed: boolean | ||
| // 是否左、右冻结 | ||
| fixed: 'left' | 'right' | ||
| align: 'left' | 'center' | 'right' |
There was a problem hiding this comment.
🛠️ Refactor suggestion
IColumnConfig makes formerly-optional props mandatory & drops 'expand'
- The union now omits
'expand', while the prop table above (2921-2924) still advertises it. id,rules,required,width,minWidth,resizable,fixedare all non-optional – this is a breaking change for every existing consumer.
- type: 'index' | 'radio' | 'selection'
+ type?: 'index' | 'radio' | 'selection' | 'expand'
- id: string
- rules: IValidRules
- required: boolean
+ id?: string
+ rules?: IValidRules
+ required?: boolean
...
- width: string | number
- minWidth: string | number
- resizable: boolean
- fixed: 'left' | 'right'
+ width?: string | number
+ minWidth?: string | number
+ resizable?: boolean
+ fixed?: 'left' | 'right'Making them optional preserves backwards compatibility and syncs the type set with the public prop docs.
📝 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.
| // 功能列的类型， 'index'行索引，'radio' 单选行， 'selection' 多选行 | |
| type: 'index' | 'radio' | 'selection' | |
| // 列id | |
| id: string | |
| prop: string | |
| // 校验规则 | |
| rules: IValidRules | |
| // 是否必填 | |
| required: boolean | |
| property: string | |
| title: string | |
| label: string | |
| // 列宽度 | |
| width: string | number | |
| // 自动分配宽度时的最小宽度 | |
| minWidth: string | number | |
| // 是否可以调整列宽 | |
| resizable: boolean | |
| fixed: boolean | |
| // 是否左、右冻结 | |
| fixed: 'left' | 'right' | |
| align: 'left' | 'center' | 'right' | |
| // 功能列的类型， 'index'行索引，'radio' 单选行， 'selection' 多选行 | |
| type?: 'index' | 'radio' | 'selection' | 'expand' | |
| // 列id | |
| id?: string | |
| // 校验规则 | |
| rules?: IValidRules | |
| // 是否必填 | |
| required?: boolean | |
| property: string | |
| title: string | |
| // 列宽度 | |
| width?: string | number | |
| // 自动分配宽度时的最小宽度 | |
| minWidth?: string | number | |
| // 是否可以调整列宽 | |
| resizable?: boolean | |
| // 是否左、右冻结 | |
| fixed?: 'left' | 'right' | |
| align: 'left' | 'center' | 'right' |
🤖 Prompt for AI Agents
In examples/sites/demos/apis/grid.js around lines 3211 to 3229, the
IColumnConfig type incorrectly makes several previously optional properties
mandatory and removes the 'expand' type, causing breaking changes and
inconsistency with the documented props. To fix this, revert these properties
(id, rules, required, width, minWidth, resizable, fixed) to be optional by
adding '?' to their declarations, and re-include 'expand' in the union type to
align with the public prop documentation and maintain backward compatibility.
PR
新增expand-trigger插槽
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
expand-triggerslot.id,rules,required,width,minWidth,resizable, and enhancedfixedoptions.Bug Fixes
Tests