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.

Sign up for GitHub

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

Jump to bottom

feat(pro:table): the layout tool support visible, changeSize, resetable #1488

Merged
merged 1 commit into from
Mar 6, 2023
Merged

feat(pro:table): the layout tool support visible, changeSize, resetable #1488

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 2 additions & 0 deletions packages/pro/config/src/defaultConfig.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ export const defaultConfig: ProGlobalConfig = {
},
table: {
layoutTool: {
changeSize: false,
resetable: false,
searchable: false,
},
},
Copy link

Choose a reason for hiding this comment

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

the code review

The given code patch looks good. It's adding two new properties to the layoutTool object, namely changeSize and resetable, and setting them to false. There does not seem to be any bugs present in the code patch. However, one suggestion for improvement would be to add comments to the code so that it is easier to understand what each part of the code is doing.

Expand Down
2 changes: 2 additions & 0 deletions packages/pro/config/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ export interface ProTableConfig {
// @deprecated please use `columnIndexable` of TableConfig instead'
columnIndexable?: Omit<ProTableColumnIndexable, 'type'>
layoutTool: {
changeSize: boolean
resetable: boolean
searchable: boolean
}
}
Copy link

Choose a reason for hiding this comment

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

the code review
1.The first point of code review is to check for any syntax errors or typos.In the given code fragment, there are no syntax errors and typos.
2.The second point in code review is to check for logical errors.In this code fragment,the logical error is not present.
3.The third point in code review is to check for security risks.In this code fragment,there are no security risks present.
4.The fourth point in code review is to check for performance issues.In this code fragment,there are no performance issues present.
5.The fifth point in code review is to check for code optimization.In this code fragment, the code can be optimized by adding comments which can help in understanding the code better.

Expand Down
9 changes: 7 additions & 2 deletions packages/pro/table/docs/Api.zh.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,5 +75,10 @@ export type ProTableColumn<T = any, V = any, CT = 'input'> =
| 名称 | 说明 | 类型 | 默认值 | 全局配置 | 备注 |
| --- | --- | --- | --- | --- | --- |
| `v-model:searchValue` | 搜索的文本 | `string` | - | - | - |
| `placeholder` | 搜索框的占位符 | `string` | ✅ | - | 通过 locale 全局配置 |
| `searchable` | 是否开启搜索功能 | `boolean` | ✅ | - | - |
| `v-model:visible` | 浮层的显示隐藏 | `boolean` | - | - | - |
| `changeSize` | 是否支持修改表格的尺寸 | `boolean` | `true` | ✅ |- |
| `className` | 自定义浮层的 `class` | `string` | - | - |- |
| `placeholder` | 搜索框的占位符 | `string` | `搜索关键字` | ✅ | 通过 locale 全局配置 |
| `resetable` | 是否支持重置布局设置 | `boolean` | `true` | ✅ |- |
| `searchable` | 是否开启搜索功能 | `boolean` | `false` | ✅ | - |
| `onReset` | 点击重置按钮后的回调 | `(evt: MouseEvent) => boolean \| void` | - | - | 返回 `false` 会阻止组件内部的重置操作 |
Copy link

Choose a reason for hiding this comment

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

with code review:

  1. Make sure the variables used for v-model and placeholder are correct. For example, the placeholder should be a string value instead of a boolean value.
  2. Check to see if the types of the variables are correct and if they are consistent with their default values. For example, resetable is a boolean but the default value is a string.
  3. Check that all necessary attributes are included in the table and that they are named correctly.
  4. Ensure that all functions have the correct parameters and that they are properly documented.
  5. Check that the code is well-structured, organized, and easy to read.
  6. Verify that the code follows best practices and coding standards.
  7. Look for any potential security risks and vulnerabilities.
  8. Test the code to ensure that it works as intended.

9 changes: 5 additions & 4 deletions packages/pro/table/src/ProTable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

import { computed, defineComponent, provide, ref, watch } from 'vue'

import { isString } from 'lodash-es'
import { isObject, isString } from 'lodash-es'

import { type VirtualScrollToOptions } from '@idux/cdk/scroll'
import { useState } from '@idux/cdk/utils'
Expand Down Expand Up @@ -44,7 +44,6 @@ export default defineComponent({

provide(proTableToken, {
props,
slots,
config,
locale,
mergedPrefixCls,
Expand All @@ -59,10 +58,12 @@ export default defineComponent({
})

const renderLayoutTool = () => {
if (!props.layoutTool) {
const { layoutTool } = props
if (!layoutTool) {
return null
}
return <ProTableLayoutTool />
const toolProps = isObject(layoutTool) ? layoutTool : undefined
return <ProTableLayoutTool {...toolProps} />
}

const renderToolbar = () => {
Copy link

Choose a reason for hiding this comment

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

with the code review.

First, check the imports of 'lodash-es'. It looks like you're importing two functions that you're using in the code. This is good practice as it makes the code more readable and less prone to bugs.

Next, check the use of provide. It looks like you're providing props and config values for the proTableToken. This is a good practice as it ensures that all components are using the same variables.

Then, check the renderLayoutTool() function. It looks like you're checking if the layoutTool prop is present and returning null if it's not. This is good as it will prevent any errors from occurring. Additionally, you've added a check to see if layoutTool is an object and passed it as a prop if it is. This is also good practice.

Finally, check the renderToolbar() function. It looks like this function is responsible for rendering the toolbar, but there is no code present. Make sure to add the necessary code to make this function work properly.

Expand Down
43 changes: 30 additions & 13 deletions packages/pro/table/src/ProTableLayoutTool.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* found in the LICENSE file at https://github.com/IDuxFE/idux/blob/main/LICENSE
*/

import { defineComponent, inject, normalizeClass } from 'vue'
import { computed, defineComponent, inject, normalizeClass } from 'vue'

import { IxIcon } from '@idux/components/icon'
import { IxPopover } from '@idux/components/popover'
Expand All @@ -27,7 +27,11 @@ export default defineComponent({
name: 'IxProTableLayoutTool',
props: proTableLayoutToolProps,
setup(props, { slots }) {
const { locale, mergedPrefixCls, mergedSize, setMergedSize } = inject(proTableToken)!
const { config, locale, mergedPrefixCls, mergedSize, setMergedSize } = inject(proTableToken)!

const mergedChangeSize = computed(() => props.changeSize ?? config.layoutTool.changeSize)
const mergedResetable = computed(() => props.resetable ?? config.layoutTool.resetable)
const mergedSearchable = computed(() => props.searchable ?? config.layoutTool.searchable)

return () => {
const prefixCls = `${mergedPrefixCls.value}-layout-tool`
Expand All @@ -49,14 +53,32 @@ export default defineComponent({
/>
)
}
const suffixNode = (
const suffixNode = mergedChangeSize.value ? (
<IxSpace>
{renderIcon('sm')}
{renderIcon('md')}
{renderIcon('lg')}
</IxSpace>
)
const popoverHeader = { title: layoutLocale.title, suffix: suffixNode }
) : undefined

const contentProps = {
placeholder: props.placeholder,
resetable: mergedResetable.value,
searchable: mergedSearchable.value,
searchValue: props.searchValue,
'onUpdate:searchValue': props['onUpdate:searchValue'],
onReset: props.onReset,
}
const popoverProps = {
class: normalizeClass([prefixCls, props.className]),
header: { title: layoutLocale.title, suffix: suffixNode },
offset: defaultOffset,
placement: 'bottomEnd',
showArrow: false,
trigger: 'click',
visible: props.visible,
'onUpdate:visible': props['onUpdate:visible'],
} as const

return (
<IxPopover
Expand All @@ -68,15 +90,10 @@ export default defineComponent({
<IxIcon name="ellipsis" />
</span>
)),
content: () => <LayoutToolContent {...props} />,
content: () => <LayoutToolContent {...contentProps} />,
}}
class={prefixCls}
header={popoverHeader}
offset={defaultOffset}
placement="bottomEnd"
showArrow={false}
trigger="click"
></IxPopover>
{...popoverProps}
/>
)
}
},
Copy link

Choose a reason for hiding this comment

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

with the code review:

  1. The code patch imports computed from vue, which is used to create a computed property that holds the value of props.changeSize and is used to conditionally render icons in the layout tool.
  2. The code patch adds contentProps to the IxPopover component, which contains placeholder, resetable, searchable, searchValue, onUpdate:searchValue, and onReset. This allows for the user to customize the layout tool by changing these values.
  3. The code patch adds popoverProps to the IxPopover component, which contains class, header, offset, placement, showArrow, trigger and visible. This allows for the user to customize the layout tool by changing these values.
  4. The code patch adds a suffixNode to the IxPopover component, which is used to render the icons for small, medium, and large size options. This allows for the user to select the size of the layout tool.
  5. The code patch adds a popoverHeader which includes a title and suffixNode. This allows the user to customize the layout tool with a title and the size options.

In conclusion, the code patch adds functionality to the layout tool such as customizing the layout tool with contentProps, popoverProps and popoverHeader; as well as adding the ability to select the size of the layout tool with the suffixNode.

Expand Down
40 changes: 16 additions & 24 deletions packages/pro/table/src/contents/LayoutToolContent.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,49 +7,39 @@

import { computed, defineComponent, inject, normalizeClass } from 'vue'

import { isObject } from 'lodash-es'

import { useControlledProp } from '@idux/cdk/utils'
import { callEmit, useControlledProp } from '@idux/cdk/utils'
import { ɵInput } from '@idux/components/_private/input'
import { IxButton } from '@idux/components/button'
import { IxCheckbox } from '@idux/components/checkbox'

import LayoutToolTree from './LayoutToolTree'
import { proTableToken } from '../token'
import { type ProTableColumn, proTableLayoutToolProps } from '../types'
import { type ProTableColumn, proTableLayoutToolContentProps } from '../types'

export default defineComponent({
props: proTableLayoutToolProps,
props: proTableLayoutToolContentProps,
setup(props) {
const {
props: tableProps,
config,
locale,
mergedPrefixCls,
mergedColumns,
setMergedColumns,
mergedColumnMap,
resetColumns,
} = inject(proTableToken)!
const { locale, mergedPrefixCls, mergedColumns, setMergedColumns, mergedColumnMap, resetColumns } =
inject(proTableToken)!

// 判断是否有子节点，处理tree展开节点样式
const hasChildren = computed(() => mergedColumns.value.length !== mergedColumnMap.value.size)

// 只需要判断第一层的即可
const hiddenColumns = computed(() => mergedColumns.value.filter(column => column.visible === false))

const mergedSearchable = computed(
() =>
props.searchable ??
(isObject(tableProps.layoutTool) ? tableProps.layoutTool.searchable : config.layoutTool.searchable),
)
const [searchValue, setSearchValue] = useControlledProp(props, 'searchValue')

const handleInput = (evt: Event) => {
const inputValue = (evt.target as HTMLInputElement).value
setSearchValue(inputValue)
}

const handleReset = (evt: MouseEvent) => {
const result = callEmit(props.onReset, evt)
result !== false && resetColumns()
}

const handleCheckAll = (checked: boolean) => {
loopColumns(mergedColumns.value, column => {
// undefined or true
Expand Down Expand Up @@ -101,7 +91,7 @@ export default defineComponent({
const _searchValue = searchValue.value
return (
<div class={classes}>
{mergedSearchable.value && (
{props.searchable && (
<div class={`${prefixCls}-search-wrapper`}>
<ɵInput
placeholder={props.placeholder ?? placeholder}
Expand All @@ -114,9 +104,11 @@ export default defineComponent({
)}
<div class={`${prefixCls}-select-wrapper`}>
<IxCheckbox checked={checked} indeterminate={indeterminate} label={all} onChange={handleCheckAll} />
<IxButton size="sm" mode="link" onClick={resetColumns}>
{reset}
</IxButton>
{props.resetable && (
<IxButton size="sm" mode="link" onClick={handleReset}>
{reset}
</IxButton>
)}
</div>
<div class={`${prefixCls}-tree-wrapper`}>
{hasStartFixed && (
Copy link

Choose a reason for hiding this comment

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

with the code review.

First, in line 4, you imported the wrong utility. The correct one should be useControlledProp.

In line 35, you can call the emit function from the props to handle the resetting of columns.

In line 89, it looks like you are using the "mergedSearchable" computed value, which is not defined anywhere. You probably meant to use the "props.searchable" value instead.

In line 110, you are using the "mergedPrefixCls" value, which is also not defined anywhere. You probably meant to use the "prefixCls" value instead.

Lastly, in line 120, you are using the "resetColumns" function, which is not defined anywhere. You probably meant to use the "handleReset" function instead.

I hope this helps!

Expand Down
3 changes: 1 addition & 2 deletions packages/pro/table/src/token.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,10 @@ import type { ColumnsContext } from './composables/useColumns'
import type { ProTableProps } from './types'
import type { ProTableConfig } from '@idux/pro/config'
import type { ProLocale } from '@idux/pro/locales'
import type { ComputedRef, InjectionKey, Slots } from 'vue'
import type { ComputedRef, InjectionKey } from 'vue'

export interface ProTableContext extends ColumnsContext {
props: ProTableProps
slots: Slots
config: ProTableConfig
locale: ProLocale
mergedPrefixCls: ComputedRef<string>
Copy link

Choose a reason for hiding this comment

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

the code review

  1. The code patch looks like it is changing the import type of Slots from 'vue' to be removed. This change may have an impact on the functionality of the ProTableContext as the Slots type provides a way for components to create slots for other components to fill in with their own content.

  2. It is also worth considering if the ProTableContext will be affected by the removal of the Slots type, and if so, what changes need to be made to account for this.

  3. It is also important to consider how this change will impact the performance of the ProTableContext and what optimization techniques can be applied to ensure that the application remains performant.

  4. Finally, it is important to consider the maintainability of the code patch and what measures can be taken to ensure that the code is easy to read and maintain going forward.

Expand Down
17 changes: 17 additions & 0 deletions packages/pro/table/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,17 @@ export type ProTableComponent = DefineComponent<
export type ProTableInstance = InstanceType<DefineComponent<ProTableProps, ProTableBindings>>

export const proTableLayoutToolProps = {
changeSize: { type: Boolean, default: undefined },
className: { type: String, default: undefined },
placeholder: { type: String, default: undefined },
resetable: { type: Boolean, default: undefined },
searchable: { type: Boolean, default: undefined },
searchValue: { type: String, default: undefined },
visible: { type: Boolean, default: undefined },

'onUpdate:searchValue': [Function, Array] as PropType<MaybeArray<(searchValue: string) => void>>,
'onUpdate:visible': [Function, Array] as PropType<MaybeArray<(visible: boolean) => void>>,
onReset: [Function, Array] as PropType<MaybeArray<(evt: MouseEvent) => boolean | void>>,
} as const

export type ProTableLayoutToolProps = ExtractInnerPropTypes<typeof proTableLayoutToolProps>
Expand Down Expand Up @@ -101,3 +107,14 @@ export type ProTableColumnResizable = {
minWidth?: number | string
resizable?: boolean
}

// private
export const proTableLayoutToolContentProps = {
placeholder: { type: String, default: undefined },
resetable: { type: Boolean, required: true },
searchable: { type: Boolean, required: true },
searchValue: { type: String, default: undefined },

'onUpdate:searchValue': [Function, Array] as PropType<MaybeArray<(searchValue: string) => void>>,
onReset: [Function, Array] as PropType<MaybeArray<(evt: MouseEvent) => boolean | void>>,
} as const
Copy link

Choose a reason for hiding this comment

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

  1. It looks like the code patch is missing some necessary imports, such as the ProTableProps, ProTableBindings, and MaybeArray types.

  2. In the proTableLayoutToolProps object, the types for the changeSize and className properties appear to be incorrect. The type for changeSize should be Boolean, and the type for className should be String.

  3. In the proTableLayoutToolContentProps object, the resetable and searchable properties are marked as required, but they don't have default values set. This could lead to errors if the properties are not set.

  4. The proTableLayoutToolContentProps object does not have a visible property, which may cause problems if the component is relying on that property for visibility.