Skip to content
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

feat(grid-select): [grid-select] add grid select component and implement single/multiple select features #2509

Merged
merged 2 commits into from
Nov 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 84 additions & 0 deletions examples/sites/demos/apis/grid-select.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
export default {
mode: ['pc'],
apis: [
{
name: 'grid-select',
type: 'component',
props: [
{
name: 'grid-op',
typeAnchorName: 'IGridOption',
type: 'IGridOption',
defaultValue: '',
desc: {
'zh-CN': '下拉表格时,内置表格组件的配置,用法同 Grid 组件。',
'en-US': ''
},
mode: ['pc'],
pcDemo: 'basic-usage'
},
{
name: 'modelValue / v-model',
type: 'string | number | Array<string|number>',
defaultValue: '',
desc: {
'zh-CN': '绑定值',
'en-US': 'Bind value'
},
mode: ['pc'],
pcDemo: 'basic-usage'
},
{
name: 'multiple',
type: 'boolean',
defaultValue: 'false',
desc: {
'zh-CN': '是否允许选择多个选项',
'en-US': 'Allow multiple options to be selected'
},
mode: ['pc'],
pcDemo: 'multiple'
},
{
name: 'text-field',
type: 'string',
defaultValue: "''",
desc: {
'zh-CN': '显示值字段',
'en-US': 'Show Value Fields'
},
mode: ['pc'],
pcDemo: 'basic-usage'
},
{
name: 'value-field',
type: 'string',
defaultValue: "''",
desc: {
'zh-CN': '绑定值字段',
'en-US': 'Bind Value Field'
},
mode: ['pc'],
pcDemo: 'basic-usage'
}
]
}
],
types: [
{
name: 'IGridOption',
type: 'interface',
code: `
interface IGridOption {
data: Record<string, any>
columns: {
type: string
field: string
title: string
width: number
}[] // 表格列数据,用法同 Grid
}
`
}
Comment on lines +68 to +82
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance type definitions and documentation in IGridOption interface.

The interface could benefit from more specific type definitions and English documentation:

 interface IGridOption {
-  data: Record<string, any>
+  data: Record<string, unknown>
   columns: {
-    type: string
-    field: string
-    title: string
-    width: number
-  }[] // 表格列数据,用法同 Grid
+    type: 'text' | 'number' | 'date' // Add specific column types
+    field: string
+    title: string
+    width: number
+    sortable?: boolean
+    filterable?: boolean
+  }[] // Column configuration, similar to Grid component
 }

This change:

  1. Replaces any with unknown for better type safety
  2. Adds specific column types and optional features
  3. Adds English comments

Committable suggestion skipped: line range outside the PR's diff.

]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<template>
<tiny-grid-select v-model="value" :grid-op="gridOpSingle" value-field="id" text-field="city"></tiny-grid-select>
</template>

<script setup>
import { ref, reactive } from 'vue'
import { TinyGridSelect } from '@opentiny/vue'

const value = ref('')

const gridOpSingle = reactive({
data: [
{ id: '001', area: '华南区', province: '广东省', city: '广州市' },
{ id: '002', area: '华南区', province: '广东省', city: '深圳市' },
{ id: '003', area: '华南区', province: '广东省', city: '珠海市' },
{ id: '004', area: '华南区', province: '广东省', city: '佛山市' },
{ id: '005', area: '华南区', province: '广东省', city: '中山市' }
],
columns: [
{ type: 'radio', title: '' },
{ field: 'area', title: '区域', width: 90 },
{ field: 'province', title: '省份', width: 60 },
{ field: 'city', title: '城市', width: 60 }
]
})
Comment on lines +11 to +25
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance grid configuration for better usability and type safety.

Several improvements could be made to the grid configuration:

  1. Column widths seem constrained for the content
  2. Missing type definitions for the data structure
  3. No handling of selection changes

Consider applying these improvements:

+interface CityData {
+  id: string
+  area: string
+  province: string
+  city: string
+}
+
 const gridOpSingle = reactive({
   data: [
     { id: '001', area: '华南区', province: '广东省', city: '广州市' },
     { id: '002', area: '华南区', province: '广东省', city: '深圳市' },
     { id: '003', area: '华南区', province: '广东省', city: '珠海市' },
     { id: '004', area: '华南区', province: '广东省', city: '佛山市' },
     { id: '005', area: '华南区', province: '广东省', city: '中山市' }
-  ],
+  ] as CityData[],
   columns: [
     { type: 'radio', title: '' },
-    { field: 'area', title: '区域', width: 90 },
-    { field: 'province', title: '省份', width: 60 },
-    { field: 'city', title: '城市', width: 60 }
+    { field: 'area', title: '区域', width: 120 },
+    { field: 'province', title: '省份', width: 100 },
+    { field: 'city', title: '城市', width: 100 }
   ]
 })
+
+const handleSelectionChange = (newValue: string) => {
+  console.log('Selected:', newValue)
+  // Add your selection handling logic here
+}

Committable suggestion skipped: line range outside the PR's diff.

</script>

<style scoped>
.tiny-grid-select {
width: 280px;
}
</style>
Comment on lines +1 to +32
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Tests are missing for the GridSelect component

Based on the verification results:

  • The component is defined in packages/vue/src/grid-select/index.ts
  • No test files (*.spec.ts, *.test.ts, *.spec.js, *.test.js) were found for the GridSelect component
  • No test descriptions or test blocks were found containing "grid-select"

Please add test coverage for:

  • Component initialization and rendering
  • Single selection functionality
  • Data binding with v-model
  • Props validation (value-field, text-field)
  • Grid operations and data display
🔗 Analysis chain

Verify test coverage for the component.

According to the PR objectives, tests are missing for this implementation. Please ensure that test cases are added to cover:

  1. Initial rendering
  2. Selection functionality
  3. Data binding
  4. Error states

Let me try another verification strategy to check for test coverage.


Let me try one more search strategy with corrected command flags.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for existing test files
echo "Checking for existing test files..."
fd -e spec.ts -e spec.js "grid-select"

# Check for test patterns in source
echo "Checking for test patterns in source..."
rg -i "(describe|it|test|expect)" -g "grid-select*"

Length of output: 495


Script:

#!/bin/bash
# Check for test files in the entire repository structure
echo "Searching for grid-select related test files..."
fd "grid-select.*\.(spec|test)\.(js|ts|vue)" 

# Check for test files in __tests__ directories
echo "Searching in __tests__ directories..."
fd -t f . -p "**/__tests__/**/grid-select*"

# Look for grid-select component definition to find its package location
echo "Finding component source location..."
rg -l "export.*GridSelect" --type ts --type vue

Length of output: 559


Script:

#!/bin/bash
# Search for GridSelect component definition
echo "Finding component source..."
rg -t js -t ts "export.*GridSelect"

# Search for test files in a different way
echo "Searching for test files..."
find . -type f \( -name "grid-select*.spec.ts" -o -name "grid-select*.test.ts" -o -name "grid-select*.spec.js" -o -name "grid-select*.test.js" \)

# Look for any test-related content for grid-select
echo "Searching for test content..."
rg -i "describe.*grid.?select|test.*grid.?select" -g "*.{js,ts,vue}"

Length of output: 1223

25 changes: 25 additions & 0 deletions examples/sites/demos/pc/app/grid-select/basic-usage.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { expect, test } from '@playwright/test'

test('测试下拉表格单选', async ({ page }) => {
page.on('pageerror', (exception) => expect(exception).toBeNull())
await page.goto('grid-select#basic-usage')

const wrap = page.locator('#basic-usage')
const select = wrap.locator('.tiny-grid-select').nth(0)
const input = select.locator('.tiny-input__inner')
const dropdown = page.locator('body > .tiny-select-dropdown')
const suffixSvg = select.locator('.tiny-base-select__caret')
const row = dropdown.getByRole('row')

await expect(suffixSvg).toHaveCount(1)
await expect(suffixSvg).toBeVisible()

await input.click()
await expect(dropdown).toBeVisible()
await expect(row).toHaveCount(6)

await row.nth(1).getByRole('cell').first().click()
await expect(input).toHaveValue('广州市')
await input.click()
await expect(row.filter({ hasText: '广州市' })).toHaveClass(/tiny-grid-body__row row__radio/)
})
39 changes: 39 additions & 0 deletions examples/sites/demos/pc/app/grid-select/basic-usage.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
<template>
<tiny-grid-select v-model="value" :grid-op="gridOpSingle" value-field="id" text-field="city"></tiny-grid-select>
</template>

<script>
import { TinyGridSelect } from '@opentiny/vue'

export default {
components: {
TinyGridSelect
},
data() {
return {
value: '',
gridOpSingle: {
data: [
{ id: '001', area: '华南区', province: '广东省', city: '广州市' },
{ id: '002', area: '华南区', province: '广东省', city: '深圳市' },
{ id: '003', area: '华南区', province: '广东省', city: '珠海市' },
{ id: '004', area: '华南区', province: '广东省', city: '佛山市' },
{ id: '005', area: '华南区', province: '广东省', city: '中山市' }
],
columns: [
{ type: 'radio', title: '' },
{ field: 'area', title: '区域', width: 90 },
{ field: 'province', title: '省份', width: 60 },
{ field: 'city', title: '城市', width: 60 }
]
}
}
}
Comment on lines +13 to +31
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add TypeScript interfaces and event handling.

The data structure needs better type safety and event handling:

  1. Consider adding TypeScript interfaces for the data structure
  2. Add documentation for the expected data format
  3. Implement selection event handlers
  4. Review column widths for content overflow scenarios
+interface GridData {
+  id: string;
+  area: string;
+  province: string;
+  city: string;
+}
+
+interface GridOptions {
+  data: GridData[];
+  columns: Array<{
+    type?: string;
+    field?: string;
+    title: string;
+    width?: number;
+  }>;
+}
+
 export default {
   components: {
     TinyGridSelect
   },
   data() {
     return {
       value: '',
+      loading: false,
+      error: null,
       gridOpSingle: {
         data: [
           { id: '001', area: '华南区', province: '广东省', city: '广州市' },
           { id: '002', area: '华南区', province: '广东省', city: '深圳市' },
           { id: '003', area: '华南区', province: '广东省', city: '珠海市' },
           { id: '004', area: '华南区', province: '广东省', city: '佛山市' },
           { id: '005', area: '华南区', province: '广东省', city: '中山市' }
         ],
         columns: [
           { type: 'radio', title: '' },
-          { field: 'area', title: '区域', width: 90 },
-          { field: 'province', title: '省份', width: 60 },
-          { field: 'city', title: '城市', width: 60 }
+          { field: 'area', title: '区域', width: 100, overflow: 'ellipsis' },
+          { field: 'province', title: '省份', width: 80, overflow: 'ellipsis' },
+          { field: 'city', title: '城市', width: 80, overflow: 'ellipsis' }
         ]
       }
     }
-  }
+  },
+  methods: {
+    handleSelectionChange(newValue) {
+      console.log('Selection changed:', newValue);
+      // Emit event or handle selection change
+    }
+  }
 }

Committable suggestion skipped: line range outside the PR's diff.

}
</script>

<style scoped>
.tiny-grid-select {
width: 280px;
}
</style>
Comment on lines +1 to +39
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add unit tests for the component.

As mentioned in the PR objectives, tests are missing for this new component. Please add unit tests to cover:

  • Component initialization
  • Selection functionality
  • Data binding
  • Props validation
  • Event handling

Would you like me to help generate the unit test suite for this component?

Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
<template>
<tiny-grid-select
v-model="value"
multiple
:grid-op="gridOpMulti"
value-field="id"
text-field="city"
></tiny-grid-select>
</template>
Comment on lines +1 to +9
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Initialize v-model value as an array for multiple selection.

The value ref is initialized as an empty string but since this is a multiple selection component (:multiple prop is set), it should be initialized as an array to properly handle multiple selections.

Apply this diff to fix the initialization:

-const value = ref('')
+const value = ref([])

Committable suggestion skipped: line range outside the PR's diff.


<script setup>
import { ref, reactive } from 'vue'
import { GridSelect as TinyGridSelect } from '@opentiny/vue'

const value = ref('')

const gridOpMulti = reactive({
data: [
{ id: '001', area: '华南区', province: '广东省', city: '广州市' },
{ id: '002', area: '华南区', province: '广东省', city: '深圳市' },
{ id: '003', area: '华南区', province: '广东省', city: '珠海市' },
{ id: '004', area: '华南区', province: '广东省', city: '佛山市' },
{ id: '005', area: '华南区', province: '广东省', city: '中山市' }
],
columns: [
{ type: 'selection', title: '' },
{ field: 'area', title: '区域', width: 90 },
{ field: 'province', title: '省份', width: 60 },
{ field: 'city', title: '城市', width: 60 }
]
})
</script>
Comment on lines +11 to +32
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider adding TypeScript types and selection event handling.

The component could benefit from better type safety and event handling:

  1. Add TypeScript interfaces for the data structure
  2. Implement selection change handling
  3. Consider adding sorting and filtering capabilities

Here's a suggested implementation:

-<script setup>
+<script setup lang="ts">
 import { ref, reactive } from 'vue'
 import { GridSelect as TinyGridSelect } from '@opentiny/vue'
 
+interface CityData {
+  id: string
+  area: string
+  province: string
+  city: string
+}
+
 const value = ref([])
+const isLoading = ref(false)
+
+const handleSelectionChange = (selection: CityData[]) => {
+  console.log('Selected items:', selection)
+}
 
 const gridOpMulti = reactive({
   data: [
     { id: '001', area: '华南区', province: '广东省', city: '广州市' },
     // ... other data
   ],
   columns: [
     { type: 'selection', title: '' },
-    { field: 'area', title: '区域', width: 90 },
-    { field: 'province', title: '省份', width: 60 },
-    { field: 'city', title: '城市', width: 60 }
+    { field: 'area', title: '区域', width: 120, sortable: true },
+    { field: 'province', title: '省份', width: 100, sortable: true },
+    { field: 'city', title: '城市', width: 100, sortable: true }
   ]
 })
 </script>

Committable suggestion skipped: line range outside the PR's diff.


<style scoped>
.tiny-grid-select {
width: 280px;
}
</style>
45 changes: 45 additions & 0 deletions examples/sites/demos/pc/app/grid-select/multiple.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
<template>
<tiny-grid-select
v-model="value"
multiple
:grid-op="gridOpMulti"
value-field="id"
text-field="city"
></tiny-grid-select>
</template>

<script>
import { TinyGridSelect } from '@opentiny/vue'

export default {
components: {
TinyGridSelect
},
data() {
return {
value: '',
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Initialize value properly for multiple selection mode.

When multiple prop is enabled, the value should be initialized as an array rather than an empty string to properly handle multiple selections.

-      value: '',
+      value: [],
📝 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.

Suggested change
value: '',
value: [],

treeOp: {
data: [
{ id: '001', area: '华南区', province: '广东省', city: '广州市' },
{ id: '002', area: '华南区', province: '广东省', city: '深圳市' },
{ id: '003', area: '华南区', province: '广东省', city: '珠海市' },
{ id: '004', area: '华南区', province: '广东省', city: '佛山市' },
{ id: '005', area: '华南区', province: '广东省', city: '中山市' }
],
columns: [
{ type: 'selection', title: '' },
{ field: 'area', title: '区域', width: 90 },
{ field: 'province', title: '省份', width: 60 },
{ field: 'city', title: '城市', width: 60 }
]
}
}
}
}
</script>

<style scoped>
.tiny-grid-select {
width: 280px;
}
</style>
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
title: GridSelect 下拉表格选择器
---

# GridSelect 下拉表格选择器

结合了 BaseSelect 和 Grid 组件的选择器,用于从一个下拉表格中选择一个或多个选项。
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
title: GridSelect
---

# GridSelect

A selector that combines the BaseSelect and Grid components to select one or more options from a drop-down table.
35 changes: 35 additions & 0 deletions examples/sites/demos/pc/app/grid-select/webdoc/grid-select.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
export default {
column: '2',
owner: '',
meta: {
experimental: '3.20.0'
},
demos: [
{
demoId: 'basic-usage',
name: {
'zh-CN': '基本用法',
'en-US': 'Basic Usage'
},
desc: {
'zh-CN':
'<p>最基础的用法,通过 <code>grid-op</code> 设置下拉表格的数据源,<code>v-model</code> 设置绑定值。</p>',
'en-US': ''
},
codeFiles: ['basic-usage.vue']
},
{
demoId: 'multiple',
name: {
'zh-CN': '多选',
'en-US': 'Multiple'
},
desc: {
'zh-CN':
'<p>通过 <code>multiple</code> 属性启用多选功能,此时 <code>v-model</code> 的值为当前选中值所组成的数组,默认选中值会以标签形式展示。</p>',
'en-US': ''
},
codeFiles: ['multiple.vue']
}
]
}
8 changes: 8 additions & 0 deletions examples/sites/demos/pc/menus.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,14 @@ export const cmpMenus = [
}
},
{ 'nameCn': '表单', 'name': 'Form', 'key': 'form' },
{
'nameCn': '下拉表格选择器',
'name': 'GridSelect',
'key': 'grid-select',
'meta': {
'experimental': '3.20.0'
}
},
{ 'nameCn': '输入框', 'name': 'Input', 'key': 'input' },
{ 'nameCn': ' IP地址输入框', 'name': 'IpAddress', 'key': 'ip-address' },
{ 'nameCn': '数字输入框', 'name': 'Numeric', 'key': 'numeric' },
Expand Down
2 changes: 1 addition & 1 deletion examples/sites/src/views/components/cmp-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const faqMdConfig = {
}

const getWebdocPath = (path) => {
if (path?.startsWith('grid-')) {
if (path?.startsWith('grid-') && path !== 'grid-select') {
return 'grid'
} else if (path?.startsWith('chart-')) {
return 'chart'
Expand Down
6 changes: 6 additions & 0 deletions packages/modules.json
Original file line number Diff line number Diff line change
Expand Up @@ -1119,6 +1119,12 @@
"type": "component",
"exclude": false
},
"GridSelect": {
"path": "vue/src/grid-select/index.ts",
"type": "component",
"exclude": false,
"mode": ["pc"]
},
"GridToolbar": {
"path": "vue/src/grid-toolbar/index.ts",
"type": "component",
Expand Down
38 changes: 38 additions & 0 deletions packages/renderless/src/grid-select/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
export const radioChange =
({ props, vm, emit }) =>
({ row }) => {
if (!props.multiple) {
vm.$refs.baseSelectRef.updateSelectedData({
...row,
currentLabel: row[props.textField],
value: row[props.valueField],
state: {
currentLabel: row[props.textField]
}
})

vm.$refs.baseSelectRef.hidePanel()

emit('update:modelValue', row)
emit('change', row)
}
}
Comment on lines +1 to +19
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add type safety and error handling to radioChange function.

The function implementation needs improvements in type safety and error handling:

-export const radioChange =
-  ({ props, vm, emit }) =>
-  ({ row }) => {
+export const radioChange = ({
+  props,
+  vm,
+  emit
+}: {
+  props: { multiple?: boolean; textField: string; valueField: string };
+  vm: { $refs: { baseSelectRef: any } };
+  emit: (event: string, value: any) => void;
+}) =>
+  ({ row }: { row: Record<string, any> }) => {
+    if (!row) {
+      console.warn('GridSelect: row is required for radioChange')
+      return
+    }
+
+    if (!props.textField || !props.valueField) {
+      console.warn('GridSelect: textField and valueField are required props')
+      return
+    }

     if (!props.multiple) {
+      const baseSelectRef = vm.$refs.baseSelectRef
+      if (!baseSelectRef) {
+        console.warn('GridSelect: baseSelectRef is not available')
+        return
+      }

Additionally:

  1. Consider adding validation for row[textField] and row[valueField] existence
  2. Consider adding JSDoc comments to document the function's purpose and parameters

Committable suggestion skipped: line range outside the PR's diff.


export const selectChange =
({ props, vm, emit }) =>
({ $table, selection, checked, row }) => {
if (props.multiple) {
vm.$refs.baseSelectRef.updateSelectedData(
selection.map((node) => {
return {
...node,
currentLabel: node[props.textField],
value: node[props.valueField]
}
})
)

emit('update:modelValue', selection)
emit('change', selection)
}
}
Loading
Loading