feat(grid-select):Development of the grid-select table component #3832
Conversation
WalkthroughThe PR reorganizes and significantly expands the grid-select component with new demo files showcasing additional features (initial queries, big data, extra query parameters, parent-child dependencies), removes outdated demo files, updates configuration mappings, and substantially enhances the renderless logic and Vue template with new public APIs for selection synchronization, state management, and initialization. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Component as pc.vue
participant Renderless as renderless
participant Grid as TinyGrid
User->>Component: Mount component
Component->>Renderless: api.mounted()
Renderless->>Renderless: initQuery() if available
Renderless->>Grid: Set initial selection via syncGridSelection
Grid-->>Renderless: Grid ready
User->>Component: Select row / Change value
Component->>Renderless: radioChange or selectChange
Renderless->>Renderless: Update state (modelValue, selected)
Renderless->>Grid: syncGridSelection
Renderless->>Component: emit update:modelValue
Component->>User: Display updated value
User->>Component: Open dropdown
Component->>Renderless: handleVisibleChange(true)
Renderless->>Grid: syncGridSelection
Grid-->>User: Show selections highlighted
sequenceDiagram
participant Parent as Parent GridSelect
participant Child as Child GridSelect
participant RemoteAPI as Remote Handler
User->>Parent: Select parent item(s)
Parent->>Parent: handleParentChange
Parent->>Child: Update via extraQueryParams
Child->>RemoteAPI: remoteChildren(value, extraQueryParams)
RemoteAPI->>RemoteAPI: filterChildren by parent IDs
RemoteAPI-->>Child: Filtered children options
Child->>Child: Update gridOp.data
User-->>Child: Display filtered children
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas requiring extra attention:
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (12)
examples/sites/demos/apis/grid-select.js (1)
9-120: VerifypcDemomappings actually showcase each propThe updated
pcDemovalues (e.g. mappingclearabletobasic-usageandfilterable/filter-method/multiple/remote/remote-method/reserve-keywordtoremote) look consistent with the new demo grouping, but it’s not obvious that those demos visibly exercise all of these props (especiallyclearableandfilter-method).Consider double-checking that each mapped demo clearly demonstrates the corresponding prop, or pointing to a more specific example if available, so the API docs stay discoverable and accurate.
examples/sites/demos/pc/app/grid-select/extra-query-params-composition-api.vue (1)
80-102: Defensively defaultparentIds/parentsto avoid potential runtime errorsThe parent–child filtering and selection sync look correct, but both helpers assume non-null arrays:
const filterChildren = (parentIds, keyword = '') => { ... parentIds.includes(...) ... } const handleParentChange = (parents) => { ... parents.includes(...) ... }If
extra-query-paramsor thechangeevent ever passundefined/null(for example on initialisation or full clear), this will throw. You can make this more robust with default parameters:-const filterChildren = (parentIds, keyword = '') => +const filterChildren = (parentIds = [], keyword = '') => new Promise((resolve) => { const list = childOptions .filter((child) => parentIds.includes(child.parent)) .filter((child) => child.label.includes(keyword || '')) setTimeout(() => resolve(list), 300) }) -const handleParentChange = (parents) => { +const handleParentChange = (parents = []) => { const childIds = childOptions .filter((child) => parents.includes(child.parent)) .map((child) => child.id) childValue.value = childValue.value.filter((value) => childIds.includes(value)) }This keeps the behavior the same in normal cases while preventing hard failures if the upstream component ever omits those arguments.
examples/sites/demos/pc/app/grid-select/remote.spec.ts (1)
1-23: Replace fixed 1s timeouts with condition-based waits for robustnessThe flow matches the remote demo, but the two
waitForTimeout(1000)calls add latency and can still be flaky on slower environments. Playwright already auto-waits, and you can wait on concrete state instead:- await input.click() - await page.waitForTimeout(1000) - await expect(dropdown).toBeVisible() + await input.click() + await expect(dropdown).toBeVisible() - await input.fill('10') - await input.press('Enter') - await page.waitForTimeout(1000) + await input.fill('10') + await input.press('Enter') + await expect( + page.getByRole('row', { name: /区域10 省份10 城市/ }) + ).toBeVisible()This keeps the intent but relies on UI conditions instead of arbitrary sleeps.
examples/sites/demos/pc/app/grid-select/radio-bigdata-composition-api.vue (1)
16-36: Big-data setup looks correct; optionally guard thewindowwriteThe composition-API demo correctly builds 800 rows and exposes the count via:
onMounted(() => { gridOp.data = Array.from({ length: 800 }, (item, index) => ({ ... })) window.gridSelectRadioBigData = gridOp.data.length })This matches the E2E test expectations. If this demo is ever used in a non-browser/SSR context, consider guarding the global write:
if (typeof window !== 'undefined') { window.gridSelectRadioBigData = gridOp.data.length }For the current docs-only browser usage, the existing code is fine.
examples/sites/demos/pc/app/grid-select/config.spec.ts (1)
13-23: Strengthen dropdown waits and selector scoping to reduce flakinessThe flow looks correct, but two robustness tweaks would help:
- After
await singleInput.click(), explicitly wait for the dropdown to be visible before querying rows (e.g.,await expect(dropdown).toBeVisible()), instead of relying on implicit auto-waits when clicking cells.- For the enabled-row assertion, consider scoping the row query to the dropdown (e.g.,
dropdown.getByRole('row', { name: '华南区 广东省 深圳市' })) rather than using a page‑levelgetByRole, to avoid accidental matches if similar text appears elsewhere.examples/sites/demos/pc/app/grid-select/extra-query-params.spec.ts (1)
6-25: Avoid hard-coded timeouts and brittle SVG/path selectorsThe test logic matches the demo intent, but there are two fragility points:
- Replace
waitForTimeout(1000)/waitForTimeout(400)with condition-based waits (e.g., waiting for the target grid rows or dropdown to be visible) so the test is less timing‑sensitive.- The tree expansion clicks like
locator('path').nth(2).click()and genericlocator('div').filter({ hasText: /^指南$/ })are tightly coupled to the current DOM/SVG structure; if possible, prefer role-based or data‑attribute selectors (e.g.,getByRole('row', { name: '指南' })scoped to the relevant grid, or dedicated test IDs for expand icons).These changes should make the test more resilient to UI refactors while keeping the same coverage.
examples/sites/demos/pc/app/grid-select/init-query.spec.ts (1)
13-29: Reduce dependence on timeouts and positional indices in init-query testThe assertions match the expected behavior, but the test could be more stable by:
- Replacing the repeated
waitForTimeout(1000)calls with waits on meaningful conditions (e.g., waiting for the dropdown to be visible and for rows to be populated) rather than fixed delays.- Avoiding positional selectors like
rows.nth(1).getByRole('cell').nth(0)where possible; using a row identified by its accessible name (or another deterministic attribute) will better tolerate column/order changes.These adjustments should keep semantics intact while making the test less brittle.
examples/sites/demos/pc/app/grid-select/radio-bigdata.vue (1)
34-43: Guardwindowaccess in case of non-browser executionExposing
gridSelectRadioBigDataonwindowis useful for tests, but if this demo is ever rendered in a non‑browser context (SSR, prerender, or tests withoutwindow),window.gridSelectRadioBigData = ...will throw.A small safeguard like:
if (typeof window !== 'undefined') { window.gridSelectRadioBigData = data.length }keeps the current behavior in browsers while avoiding accidental runtime errors elsewhere.
examples/sites/demos/pc/app/grid-select/extra-query-params.vue (1)
93-119: MakefilterChildrenmore defensive and clarifyhandleParentChangeexpectationsThe parent/child wiring is clear, but you can harden the helpers a bit:
filterChildren(parentIds, keyword = '')assumesparentIdsis always a non‑null array. Ifextra-query-paramsis everundefinedornull,parentIds.includes(...)will throw. A safer signature is:filterChildren(parentIds = [], keyword = '') { const list = this.childOptions .filter((child) => parentIds.includes(child.parent)) .filter((child) => child.label.includes(keyword || '')) return new Promise((resolve) => { setTimeout(() => resolve(list), 300) }) }
handleParentChange(parents)relies onparentsbeing an array of parent IDs (not objects). That’s consistent with howv-model="parentValue"usually works, but it’s worth double‑checking againstTinyGridSelect’schangeevent contract so this doesn’t break if the event payload shape changes.These tweaks will make the demo more robust without changing its behavior.
examples/sites/demos/pc/app/grid-select/init-query.vue (1)
105-108: Preferconstfor variables that aren't reassigned.The
filterDatavariable is not reassigned after initialization.Apply this diff:
remoteMethod(value) { - let filterData = this.allData.filter((item) => { + const filterData = this.allData.filter((item) => { return item.city.includes(value) })packages/vue/src/grid-select/src/pc.vue (1)
38-39: Consider simplifying the data binding fallback logic.The data binding on line 39 has a redundant fallback:
state.gridData?.data || state.gridData || []. Since we already check ifstate.gridDatais an array, the third option (state.gridData) will never be an array at that point. Consider simplifying:- :data="Array.isArray(state.gridData) ? state.gridData : state.gridData?.data || state.gridData || []" + :data="Array.isArray(state.gridData) ? state.gridData : (state.gridData?.data || [])"This makes the logic clearer: if
gridDatais an array, use it; otherwise, usegridData.dataor an empty array.packages/renderless/src/grid-select/index.ts (1)
64-66: Empty catch block may hide real errors.The empty catch block silently swallows all errors, which could mask legitimate exceptions beyond "table not loaded yet." Similar patterns appear throughout this file (lines 354, 368, 414-416, 450-452, 498-500, 614-616).
Consider one of these approaches:
- Check for specific error conditions:
} catch (e) { - // 如果表格还没有加载，忽略错误 + // Ignore errors only if table is not loaded + if (e.message && !e.message.includes('getTableData')) { + console.warn('Unexpected error in syncGridSelection:', e) + } }
- Or add development-mode logging:
} catch (e) { - // 如果表格还没有加载，忽略错误 + // Table not loaded yet + if (process.env.NODE_ENV === 'development') { + console.debug('syncGridSelection: table not ready', e) + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
examples/sites/demos/apis/grid-select.js(7 hunks)examples/sites/demos/pc/app/grid-select/basic-usage.vue(1 hunks)examples/sites/demos/pc/app/grid-select/config.spec.ts(1 hunks)examples/sites/demos/pc/app/grid-select/config.vue(1 hunks)examples/sites/demos/pc/app/grid-select/extra-query-params-composition-api.vue(1 hunks)examples/sites/demos/pc/app/grid-select/extra-query-params.spec.ts(1 hunks)examples/sites/demos/pc/app/grid-select/extra-query-params.vue(1 hunks)examples/sites/demos/pc/app/grid-select/filter-composition-api.vue(0 hunks)examples/sites/demos/pc/app/grid-select/filter.vue(0 hunks)examples/sites/demos/pc/app/grid-select/init-query-composition-api.vue(1 hunks)examples/sites/demos/pc/app/grid-select/init-query.spec.ts(1 hunks)examples/sites/demos/pc/app/grid-select/init-query.vue(1 hunks)examples/sites/demos/pc/app/grid-select/multiple-composition-api.vue(0 hunks)examples/sites/demos/pc/app/grid-select/multiple.vue(0 hunks)examples/sites/demos/pc/app/grid-select/radio-bigdata-composition-api.vue(1 hunks)examples/sites/demos/pc/app/grid-select/radio-bigdata.spec.ts(1 hunks)examples/sites/demos/pc/app/grid-select/radio-bigdata.vue(1 hunks)examples/sites/demos/pc/app/grid-select/remote.spec.ts(1 hunks)examples/sites/demos/pc/app/grid-select/remote.vue(2 hunks)examples/sites/demos/pc/app/grid-select/webdoc/grid-select.js(1 hunks)examples/sites/demos/pc/menus.js(2 hunks)packages/renderless/src/grid-select/index.ts(3 hunks)packages/renderless/src/grid-select/vue.ts(1 hunks)packages/vue/src/grid-select/src/pc.vue(5 hunks)
💤 Files with no reviewable changes (4)
- examples/sites/demos/pc/app/grid-select/filter-composition-api.vue
- examples/sites/demos/pc/app/grid-select/multiple.vue
- examples/sites/demos/pc/app/grid-select/multiple-composition-api.vue
- examples/sites/demos/pc/app/grid-select/filter.vue
🧰 Additional context used
🧬 Code graph analysis (4)
examples/sites/demos/pc/app/grid-select/radio-bigdata.spec.ts (1)
examples/sites/demos/pc/app/grid/editor/inner-editor.spec.js (1)
input(8-8)
examples/sites/demos/pc/app/grid-select/init-query.spec.ts (1)
examples/sites/demos/pc/app/grid/editor/inner-editor.spec.js (1)
input(8-8)
packages/renderless/src/grid-select/vue.ts (1)
packages/renderless/src/grid-select/index.ts (12)
buildRadioConfig(88-96)buildSelectConfig(79-86)filter(98-142)getcheckedData(148-167)getPluginOption(174-196)initQuery(198-229)mounted(231-317)radioChange(506-531)selectChange(533-620)syncGridSelection(28-68)handleVisibleChange(70-77)watchValue(319-504)
packages/renderless/src/grid-select/index.ts (3)
packages/renderless/src/grid-select/vue.ts (1)
api(16-30)packages/renderless/src/base-select/vue.ts (1)
api(105-160)packages/renderless/src/base-select/index.ts (4)
getPluginOption(335-353)initQuery(1567-1591)mounted(1593-1636)watchValue(1082-1115)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: PR E2E Test (pnpm test:e2e3)
🔇 Additional comments (6)
examples/sites/demos/pc/menus.js (1)
185-193: LGTM! GridSelect component properly added to menu.The new GridSelect menu entry is correctly structured and appropriately placed in the Form components section. The experimental version '3.27.0' aligns with the PR objectives, and the naming conventions are consistent with other component entries.
examples/sites/demos/pc/app/grid-select/basic-usage.vue (1)
2-4: Wrapping demo in#basic-usagelooks good and non-breakingAdding the
id="basic-usage"wrapper keeps behavior unchanged while making the demo easier to target from docs/tests; scoped styles on.tiny-grid-selectstill apply as before.examples/sites/demos/pc/app/grid-select/remote.vue (1)
2-23:id="remote"andreserve-keywordusage are consistent with the remote demo designThe root
id="remote"aligns this demo with the new test (remote.spec.ts) and thepcDemo: 'remote'mappings, and addingreserve-keywordonly to the multi-select remote grid-select is correct for showcasing that behavior without affecting the single-select case.examples/sites/demos/pc/app/grid-select/config.vue (1)
2-23: Rootid="config"is a safe, useful additionAdding
id="config"to the root container doesn’t alter component behavior and makes it easier to anchor from routing, docs, or Playwright tests (e.g.config.spec.ts).examples/sites/demos/pc/app/grid-select/radio-bigdata.spec.ts (1)
1-19: E2E flow for big-data demo is sound and avoids arbitrary sleepsUsing
waitForFunction(() => window.gridSelectRadioBigData === 800)to gate on data readiness, then selecting the second row and asserting'广州市 0'matches the demo’s behavior and keeps the test deterministic without fixed delays.examples/sites/demos/pc/app/grid-select/webdoc/grid-select.js (1)
4-91: Demo metadata aligns well with new grid-select features
show/cloudflags, the updatedexperimentalversion, and the refreshed demo entries (config,remote,init-query,extra-query-params,radio-bigdata) are consistent and point to the correct.vuefiles. The zh‑CN/en‑US copy also clearly reflects each scenario.
| <tiny-grid-select | ||
| ref="select" | ||
| v-model="radioValue1" | ||
| placeholder="请输入关键词" | ||
| clearable | ||
| :init-query="initQuery" | ||
| :remote-method="remoteMethod" | ||
| :remote-config="{ autoSearch: true, clearData: true, showIcon: true }" | ||
| remote | ||
| filterable | ||
| value-field="id" | ||
| text-field="city" | ||
| :grid-op="gridOpRadio" | ||
| ></tiny-grid-select> | ||
| <br /> | ||
| <br /> | ||
| <div>场景 2：下拉表格远程搜索初始化查询（多选）</div> | ||
| <br /> | ||
| <tiny-grid-select | ||
| ref="select" | ||
| v-model="radioValue2" | ||
| placeholder="请输入关键词" | ||
| clearable | ||
| :init-query="initQuery" | ||
| :remote-method="remoteMethod" | ||
| :remote-config="{ autoSearch: true, clearData: true, showIcon: true }" | ||
| remote | ||
| filterable | ||
| value-field="id" | ||
| text-field="city" | ||
| multiple | ||
| :grid-op="gridOpRadio2" | ||
| ></tiny-grid-select> | ||
| </div> | ||
| </template> | ||
|
|
||
| <script setup> | ||
| import { ref, reactive } from 'vue' | ||
| import { TinyGridSelect } from '@opentiny/vue' | ||
| const radioValue1 = ref('001') | ||
| const radioValue2 = ref(['001', '002']) | ||
| const allData = Array.from({ length: 10000 }, (item, index) => { | ||
| return { | ||
| id: '00' + index, | ||
| area: '华南区' + index, | ||
| province: '广东省' + index, | ||
| city: '广州市' + index | ||
| } | ||
| }) | ||
| const gridOpRadio = reactive({ | ||
| data: [], | ||
| height: 300, | ||
| optimization: { | ||
| animat: true, | ||
| delayHover: 250, | ||
| scrollX: { gt: 20 }, | ||
| scrollY: { gt: 20 } | ||
| }, | ||
| columns: [ | ||
| { type: 'radio', title: '' }, | ||
| { field: 'area', title: '区域' }, | ||
| { field: 'province', title: '省份' }, | ||
| { field: 'city', title: '城市' } | ||
| ] | ||
| }) | ||
| const gridOpRadio2 = reactive({ | ||
| data: [], | ||
| height: 300, | ||
| optimization: { | ||
| animat: true, | ||
| delayHover: 250, | ||
| scrollX: { gt: 20 }, | ||
| scrollY: { gt: 20 } | ||
| }, | ||
| columns: [ | ||
| { type: 'selection', title: '' }, | ||
| { field: 'area', title: '区域' }, | ||
| { field: 'province', title: '省份' }, | ||
| { field: 'city', title: '城市' } | ||
| ] | ||
| }) | ||
| const initQuery = (value) => { | ||
| return new Promise((resolve) => { | ||
| setTimeout(() => { | ||
| resolve(allData) | ||
| }, 500) | ||
| }) | ||
| } | ||
| const remoteMethod = (value) => { | ||
| let filterData = allData.filter((item) => { | ||
| return item.city.includes(value) | ||
| }) | ||
| return new Promise((resolve) => { | ||
| setTimeout(() => { | ||
| resolve(filterData) | ||
| }, 500) | ||
| }) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Clean up unused refs and harden remoteMethod against empty search values
A couple of small points to tighten this demo:
-
Both grid-selects use
ref="select", but there is no correspondingconst select = ref(null)in<script setup>, and the ref is never read. This can trigger template‑ref warnings and is redundant; either declare and use the ref(s), or simply drop therefattributes. -
In
remoteMethod, callingitem.city.includes(value)assumesvalueis a non‑empty string. If the component ever callsremote-methodwith''orundefined, you’ll end up filtering out all data. A defensive pattern like:const keyword = value || '' const filterData = allData.filter((item) => item.city.includes(keyword))
makes the behavior explicit and avoids surprises.
The rest of the setup (large allData, initQuery preloading, and grid options) looks consistent with the init‑query feature.
I'll verify the review comment by examining the actual file and checking for the declared concerns.
---
🏁 Script executed:
```shell
#!/bin/bash
# Find and examine the Vue file mentioned in the review
fd -t f "init-query-composition-api.vue" --exec cat {}
Length of output: 5025
Both issues confirmed; fix unused ref and add defensive check to remoteMethod
Verification confirms the review findings:
-
Undeclared ref: The template declares
ref="select"on both grid-select components, but there is no correspondingconst select = ref(...)in the<script setup>. This ref is never read anywhere in the file, causing unnecessary template ref warnings. Either declare and use the ref or remove the attributes. -
Unsafe filter in
remoteMethod: The code callsitem.city.includes(value)without guarding against falsy or non-string inputs. If the component passes an empty string, all items will match (since every string contains''); ifundefinedornullare passed, the behavior is unpredictable. A defensive approach is recommended:const keyword = value || '' const filterData = allData.filter((item) => item.city.includes(keyword))
🤖 Prompt for AI Agents
In examples/sites/demos/pc/app/grid-select/init-query-composition-api.vue around
lines 5 to 109, remove the unused template refs or declare and use them: either
delete ref="select" from the two tiny-grid-select tags (since the ref is not
used) or add a matching const select = ref(null) (or two separate refs) in the
<script setup> and use them where needed; additionally, make remoteMethod
defensive by normalizing the incoming value to a safe string and guarding
item.city before calling includes (e.g. treat undefined/null as empty string and
ensure item.city is a string) so filtering never throws and behaves predictably
for empty or non-string inputs.
| <div>场景 1：下拉表格远程搜索初始化查询（单选）</div> | ||
| <br /> | ||
| <tiny-grid-select | ||
| ref="select" |
There was a problem hiding this comment.
Duplicate ref names will cause ref collision.
Both grid-select instances use ref="select", which means only the second instance will be accessible via this.$refs.select. If you need to programmatically access these components, use unique ref names like ref="select1" and ref="select2".
Apply this diff:
<tiny-grid-select
- ref="select"
+ ref="select1"
v-model="radioValue1" <tiny-grid-select
- ref="select"
+ ref="select2"
v-model="radioValue2"Also applies to: 24-24
🤖 Prompt for AI Agents
In examples/sites/demos/pc/app/grid-select/init-query.vue around lines 6 and 24,
both grid-select components use the same ref "select", causing ref collisions;
change them to unique names (e.g., ref="select1" and ref="select2") and update
any code that accesses these refs (this.$refs.select → this.$refs.select1 /
this.$refs.select2) so programmatic access points to the correct component
instances.
| export const watchValue = | ||
| ({ api, props, vm, state }) => | ||
| (newValue: any, oldValue: any) => { | ||
| if (props.multiple) { | ||
| let initialNodes: any[] = [] | ||
| if (Array.isArray(newValue) && newValue.length > 0) { | ||
| // 从表格的完整数据中查找选中的行 | ||
| const isRemote = | ||
| (props.filterable || props.searchable) && | ||
| props.remote && | ||
| (typeof props.remoteMethod === 'function' || typeof props.initQuery === 'function') | ||
| const sourceData: any[] = isRemote | ||
| ? state.remoteData | ||
| : Array.isArray(state.gridData) | ||
| ? state.gridData | ||
| : state.gridData?.data || [] | ||
|
|
||
| // 如果表格已加载，从表格的完整数据中查找 | ||
| if (vm.$refs.gridRef) { | ||
| try { | ||
| const tableData = vm.$refs.gridRef.getTableData() | ||
| const fullData = tableData?.fullData || sourceData | ||
|
|
||
| newValue.forEach((value: any) => { | ||
| const foundRow = fullData.find((item: any) => item[props.valueField] === value) | ||
| if (foundRow) { | ||
| initialNodes.push(foundRow) | ||
| } else { | ||
| // 如果表格中没有找到，尝试从 state.selected 中查找 | ||
| const existing = state.selected.find((item: any) => item[props.valueField] === value) | ||
| if (existing) { | ||
| initialNodes.push(existing) | ||
| } | ||
| } | ||
| }) | ||
| } catch (e) { | ||
| // 如果表格还没有加载，从源数据中查找 | ||
| newValue.forEach((value: any) => { | ||
| const foundRow = sourceData.find((item: any) => item[props.valueField] === value) | ||
| if (foundRow) { | ||
| initialNodes.push(foundRow) | ||
| } else { | ||
| // 如果源数据中没有找到，尝试从 state.selected 中查找 | ||
| const existing = state.selected.find((item: any) => item[props.valueField] === value) | ||
| if (existing) { | ||
| initialNodes.push(existing) | ||
| } | ||
| } | ||
| }) | ||
| } | ||
| } else { | ||
| // 表格还没有加载，从源数据中查找 | ||
| newValue.forEach((value: any) => { | ||
| const foundRow = sourceData.find((item: any) => item[props.valueField] === value) | ||
| if (foundRow) { | ||
| initialNodes.push(foundRow) | ||
| } else { | ||
| // 如果源数据中没有找到，尝试从 state.selected 中查找 | ||
| const existing = state.selected.find((item: any) => item[props.valueField] === value) | ||
| if (existing) { | ||
| initialNodes.push(existing) | ||
| } | ||
| } | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| const selected = initialNodes.map((node: any) => { | ||
| return { | ||
| ...node, | ||
| currentLabel: node[props.textField], | ||
| value: node[props.valueField] | ||
| } | ||
| }) | ||
|
|
||
| // 更新输入框中选中的标签 | ||
| if (vm.$refs.baseSelectRef) { | ||
| updateBaseSelect({ vm, props, data: selected }) | ||
| } | ||
| state.selected = selected | ||
|
|
||
| // 更新下拉面板中选中的表格行（使用 nextTick 确保表格已渲染） | ||
| if (vm.$refs.gridRef) { | ||
| vm.$refs.gridRef.clearSelection() | ||
| if (selected.length > 0) { | ||
| // 从表格的完整数据中获取实际的行对象 | ||
| try { | ||
| const tableData = vm.$refs.gridRef.getTableData() | ||
| const fullData = tableData?.fullData || [] | ||
| const rowsToSelect = fullData.filter((row: any) => | ||
| selected.some((sel: any) => sel[props.valueField] === row[props.valueField]) | ||
| ) | ||
| if (rowsToSelect.length > 0) { | ||
| vm.$refs.gridRef.setSelection(rowsToSelect, true) | ||
| } | ||
| } catch (e) { | ||
| // 如果表格还没有加载，直接使用 selected | ||
| vm.$refs.gridRef.setSelection(selected, true) | ||
| } | ||
| } | ||
| } | ||
| } else { | ||
| if (!newValue) { | ||
| state.selected = {} | ||
| state.currentKey = '' | ||
| if (vm.$refs.gridRef) { | ||
| vm.$refs.gridRef.clearRadioRow() | ||
| } | ||
| if (vm.$refs.baseSelectRef) { | ||
| updateBaseSelect({ vm, props, data: null }) | ||
| } | ||
| return | ||
| } | ||
|
|
||
| // 从表格的完整数据中查找选中的行 | ||
| const isRemote = | ||
| (props.filterable || props.searchable) && | ||
| props.remote && | ||
| (typeof props.remoteMethod === 'function' || typeof props.initQuery === 'function') | ||
| const sourceData: any[] = isRemote | ||
| ? state.remoteData | ||
| : Array.isArray(state.gridData) | ||
| ? state.gridData | ||
| : state.gridData?.data || [] | ||
|
|
||
| let data = null | ||
| if (vm.$refs.gridRef) { | ||
| try { | ||
| const tableData = vm.$refs.gridRef.getTableData() | ||
| const fullData = tableData?.fullData || sourceData | ||
| data = fullData.find((item: any) => item[props.valueField] === newValue) | ||
| } catch (e) { | ||
| data = sourceData.find((item: any) => item[props.valueField] === newValue) | ||
| } | ||
| } else { | ||
| data = sourceData.find((item: any) => item[props.valueField] === newValue) | ||
| } | ||
|
|
||
| // 如果表格中没有找到，尝试从 state.selected 中查找 | ||
| if ( | ||
| !data && | ||
| state.selected && | ||
| typeof state.selected === 'object' && | ||
| !Array.isArray(state.selected) && | ||
| state.selected[props.valueField] === newValue | ||
| ) { | ||
| data = state.selected | ||
| } | ||
|
|
||
| if (data && typeof data === 'object' && !Array.isArray(data)) { | ||
| updateBaseSelect({ | ||
| vm, | ||
| props, | ||
| data: { | ||
| ...(data as any), | ||
| currentLabel: data[props.textField], | ||
| value: data[props.valueField], | ||
| state: { | ||
| currentLabel: data[props.textField] | ||
| } | ||
| } | ||
| }) | ||
|
|
||
| state.selected = data | ||
| state.currentKey = data[props.valueField] | ||
|
|
||
| // 更新下拉面板中选中的表格行 | ||
| if (vm.$refs.gridRef) { | ||
| vm.$refs.gridRef.clearRadioRow() | ||
| // 从表格的完整数据中获取实际的行对象 | ||
| try { | ||
| const tableData = vm.$refs.gridRef.getTableData() | ||
| const fullData = tableData?.fullData || [] | ||
| const rowToSelect = fullData.find((row: any) => row[props.valueField] === newValue) | ||
| if (rowToSelect) { | ||
| vm.$refs.gridRef.setRadioRow(rowToSelect) | ||
| } else { | ||
| vm.$refs.gridRef.setRadioRow(data) | ||
| } | ||
| } catch (e) { | ||
| vm.$refs.gridRef.setRadioRow(data) | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Function complexity is very high; consider refactoring.
The watchValue function is 186 lines long with significant code duplication:
- Duplicated source data logic (lines 326-334, 434-442): The logic to determine
sourceDataappears in multiple places - Repeated row lookup patterns (lines 337-383, 445-455): Three similar code blocks for finding rows
- Duplicated fallback logic (lines 346-352, 361-366, 376-381): Identical patterns for falling back to
state.selected
Consider extracting helper functions to improve maintainability:
// Extract source data determination
const getSourceData = (props, state) => {
const isRemote = (props.filterable || props.searchable) &&
props.remote &&
(typeof props.remoteMethod === 'function' || typeof props.initQuery === 'function')
return isRemote
? state.remoteData
: Array.isArray(state.gridData) ? state.gridData : state.gridData?.data || []
}
// Extract row finding logic with fallback
const findRowByValue = (value, props, state, vm) => {
const sourceData = getSourceData(props, state)
// Try grid's full data first
if (vm.$refs.gridRef) {
try {
const fullData = vm.$refs.gridRef.getTableData()?.fullData || sourceData
const found = fullData.find(item => item[props.valueField] === value)
if (found) return found
} catch (e) {
// Fall through to sourceData
}
}
// Try source data
const found = sourceData.find(item => item[props.valueField] === value)
if (found) return found
// Fall back to state.selected
if (Array.isArray(state.selected)) {
return state.selected.find(item => item[props.valueField] === value)
}
return state.selected?.[props.valueField] === value ? state.selected : null
}Then refactor watchValue to use these helpers, significantly reducing its size and improving readability.
🤖 Prompt for AI Agents
In packages/renderless/src/grid-select/index.ts around lines 319 to 504, the
watchValue function is overly long and duplicates source-data determination, row
lookup and fallback-to-state.selected logic in multiple places; extract a small
helper getSourceData(props, state) that computes isRemote and returns
state.remoteData or the correct gridData array, and extract a
findRowByValue(value, props, state, vm) that (1) tries
vm.$refs.gridRef.getTableData()?.fullData, (2) falls back to sourceData, and (3)
finally falls back to state.selected (handling array vs single object), then
replace all duplicated blocks with calls to these helpers; ensure the helpers
are used both in the multiple-select and single-select branches and that
updateBaseSelect and the gridRef selection/clear calls use the resolved row
objects from findRowByValue (or mapped arrays) so behavior remains identical.
| const remoteItem = (row: any) => { | ||
| const removeItem = state.selected.find((item: any) => item[valueField] === row[valueField]) | ||
| if (removeItem) { | ||
| const index = state.selected.indexOf(removeItem) | ||
| state.selected.splice(index, 1) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Misleading function name.
The function is named remoteItem but it actually removes an item from the selected array. This naming is confusing.
Apply this diff:
- const remoteItem = (row: any) => {
+ const removeItem = (row: any) => {
const removeItem = state.selected.find((item: any) => item[valueField] === row[valueField])
if (removeItem) {
const index = state.selected.indexOf(removeItem)
state.selected.splice(index, 1)
}
}Also update the calls on lines 559 and 579:
} else {
- remoteItem(row)
+ removeItem(row)
}
} else { tableData.forEach((row: any) => {
- remoteItem(row)
+ removeItem(row)
})📝 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 remoteItem = (row: any) => { | |
| const removeItem = state.selected.find((item: any) => item[valueField] === row[valueField]) | |
| if (removeItem) { | |
| const index = state.selected.indexOf(removeItem) | |
| state.selected.splice(index, 1) | |
| } | |
| } | |
| const removeItem = (row: any) => { | |
| const foundItem = state.selected.find((item: any) => item[valueField] === row[valueField]) | |
| if (foundItem) { | |
| const index = state.selected.indexOf(foundItem) | |
| state.selected.splice(index, 1) | |
| } | |
| } |
🤖 Prompt for AI Agents
packages/renderless/src/grid-select/index.ts around lines 538 to 579: the local
function named `remoteItem` is misnamed — it actually removes an item from
`state.selected`; rename the function to `removeItem` (or another clear verb
like `removeSelectedItem`) and update its two call sites referenced on lines 559
and 579 to use the new name; ensure any other internal references
(closures/exports) are also updated so there are no unresolved identifiers.
| selected: props.multiple ? [] : {}, | ||
| currentKey: props.multiple ? '' : props.modelValue, | ||
| previousQuery: null, | ||
| modelValue: props.multiple ? (Array.isArray(props.modelValue) ? [...props.modelValue] : []) : props.modelValue, |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the necessity of array copying and fallback behavior.
The modelValue initialization creates a shallow copy when in multiple mode: [...props.modelValue]. Additionally, if props.modelValue is not an array in multiple mode, it defaults to [].
While defensive, this could silently hide type mismatches. Consider:
- Adding a comment explaining why the array copy is necessary (e.g., to prevent external mutations)
- Adding a development warning if
props.multipleis true butprops.modelValueis not an array
Run this script to check if there are any scenarios where modelValue could be non-array in multiple mode:
🏁 Script executed:
#!/bin/bash
# Search for patterns where modelValue might not be an array in multiple mode
rg -n -C3 'multiple.*true' --type=vue examples/sites/demos/pc/app/grid-select/ | rg -C3 'v-model'Length of output: 27
🏁 Script executed:
#!/bin/bash
# Find test and example files for grid-select
fd -e 'spec\.|test\.|\.spec\.|\.test\.|demo|example' grid-select --type fLength of output: 0
🏁 Script executed:
#!/bin/bash
# Search for prop definitions or type definitions for grid-select
rg -n 'modelValue' packages/renderless/src/grid-select/ -A2 -B2Length of output: 7185
🏁 Script executed:
#!/bin/bash
# Look for how modelValue is mutated or passed internally
rg -n 'modelValue\s*=' packages/renderless/src/grid-select/ -C2Length of output: 1534
🏁 Script executed:
#!/bin/bash
# Check for prop type definitions/interfaces that might define modelValue shape
fd grid-select -e ts -e tsx -e js -e vue | xargs rg -l 'interface|type.*Props' 2>/dev/null | head -5Length of output: 40
🏁 Script executed:
#!/bin/bash
# Read the API definitions file to understand modelValue prop contract
cat examples/sites/demos/apis/grid-select.jsLength of output: 4647
🏁 Script executed:
#!/bin/bash
# Also check the full vue.ts file to see if there are any comments about why copying is necessary
head -70 packages/renderless/src/grid-select/vue.ts | tail -40Length of output: 1381
🏁 Script executed:
#!/bin/bash
# Check if there are any comments in index.ts around modelValue handling
rg -n 'modelValue' packages/renderless/src/grid-select/index.ts -B1 | head -30Length of output: 1253
Add clarifying comments and consider development-time warnings for defensive array handling.
The array copying at line 56 is justified—it prevents external mutations of the prop array. Since the modelValue prop accepts string | number | Array<string|number>, the fallback to [] in multiple mode correctly handles cases where non-array values are provided.
To improve code clarity and catch potential issues during development:
- Add a comment explaining why the shallow copy
[...props.modelValue]is necessary (prevents reactive issues from external mutations) - Consider a development warning when
props.multiple === truebutArray.isArray(props.modelValue) === false, as this indicates incorrect usage of the multiple selection mode
The code is functionally sound, but these additions would enhance maintainability and help developers avoid type-related mistakes.
🤖 Prompt for AI Agents
In packages/renderless/src/grid-select/vue.ts around line 56, the initializer
shallow-copies props.modelValue when multiple is true to avoid external mutation
of the prop, but it lacks an explanatory comment and a dev-time warning for
misuse; add a short comment explaining that [...props.modelValue] prevents
external/reactive mutation of the prop array, and add a development-only
console.warn (guarded by process.env.NODE_ENV !== 'production' or a similar
runtime flag) that alerts when props.multiple === true but
Array.isArray(props.modelValue) === false to help catch incorrect usage during
development.
2 participants
PR
feat:下拉表格组件开发
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
Release Notes
New Features
Demo Enhancements
✏️ Tip: You can customize this high-level summary in your review settings.