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

fix(comp:table): virtual + fixed column abnormal display with chrome83 #1473

Merged
merged 2 commits into from
Feb 27, 2023
Merged

fix(comp:table): virtual + fixed column abnormal display with chrome83 #1473

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8eca155
fix(comp:table): virtual + fixed column abnormal display with chrome83
danranVm Feb 24, 2023
5d61204
fix(comp:table): the radio of selectable support reverse selected
danranVm Feb 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 5 additions & 5 deletions packages/components/collapse/__tests__/__snapshots__/collapse.spec.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
exports[`Collapse > panel work > header work 1`] = `
"<div class=\\"ix-collapse ix-collapse-md\\">
<div class=\\"ix-collapse-panel\\">
<div class=\\"ix-header ix-header-md\\"><span class=\\"ix-header-prefix\\"><i class=\\"ix-icon ix-icon-right\\" style=\\"transform: rotate(0deg);\\" role=\\"img\\" aria-label=\\"right\\"></i></span>
<div class=\\"ix-header ix-header-md\\"><span class=\\"ix-header-prefix\\"><i class=\\"ix-icon ix-icon-right ix-collapse-panel-expand-icon\\" style=\\"transform: rotate(0deg);\\" role=\\"img\\" aria-label=\\"right\\"></i></span>
<!---->
<div class=\\"ix-header-content\\">
<div class=\\"ix-header-title-wrapper\\"><span class=\\"ix-header-title\\">header 0</span>
Expand All @@ -27,7 +27,7 @@ exports[`Collapse > panel work > header work 1`] = `
exports[`Collapse > panel work > header work 2`] = `
"<div class=\\"ix-collapse ix-collapse-md\\">
<div class=\\"ix-collapse-panel\\">
<div class=\\"ix-header ix-header-md\\"><span class=\\"ix-header-prefix\\"><i class=\\"ix-icon ix-icon-right\\" style=\\"transform: rotate(0deg);\\" role=\\"img\\" aria-label=\\"right\\"><svg viewBox=\\"0 0 1024 1024\\" focusable=\\"false\\" aria-hidden=\\"true\\" data-icon=\\"right\\"><path d=\\"m411.776 294.784 208.192 208.192a12.8 12.8 0 0 1 0 18.048L411.776 729.216a12.8 12.8 0 0 0 0 18.112l27.2 27.136a12.8 12.8 0 0 0 18.048 0l253.44-253.44a12.8 12.8 0 0 0 0-18.048l-253.44-253.44a12.8 12.8 0 0 0-18.048 0l-27.2 27.136a12.8 12.8 0 0 0 0 18.112z\\"></path></svg></i></span>
<div class=\\"ix-header ix-header-md\\"><span class=\\"ix-header-prefix\\"><i class=\\"ix-icon ix-icon-right ix-collapse-panel-expand-icon\\" style=\\"transform: rotate(0deg);\\" role=\\"img\\" aria-label=\\"right\\"><svg viewBox=\\"0 0 1024 1024\\" focusable=\\"false\\" aria-hidden=\\"true\\" data-icon=\\"right\\"><path d=\\"m411.776 294.784 208.192 208.192a12.8 12.8 0 0 1 0 18.048L411.776 729.216a12.8 12.8 0 0 0 0 18.112l27.2 27.136a12.8 12.8 0 0 0 18.048 0l253.44-253.44a12.8 12.8 0 0 0 0-18.048l-253.44-253.44a12.8 12.8 0 0 0-18.048 0l-27.2 27.136a12.8 12.8 0 0 0 0 18.112z\\"></path></svg></i></span>
<!---->
<div class=\\"ix-header-content\\">
<div class=\\"ix-header-title-wrapper\\"><span class=\\"ix-header-title\\">hello header</span>
Expand All @@ -50,7 +50,7 @@ exports[`Collapse > panel work > header work 2`] = `
exports[`Collapse > render work 1`] = `
"<div class=\\"ix-collapse ix-collapse-md\\">
<div class=\\"ix-collapse-panel\\">
<div class=\\"ix-header ix-header-md\\"><span class=\\"ix-header-prefix\\"><i class=\\"ix-icon ix-icon-right\\" style=\\"transform: rotate(0deg);\\" role=\\"img\\" aria-label=\\"right\\"></i></span>
<div class=\\"ix-header ix-header-md\\"><span class=\\"ix-header-prefix\\"><i class=\\"ix-icon ix-icon-right ix-collapse-panel-expand-icon\\" style=\\"transform: rotate(0deg);\\" role=\\"img\\" aria-label=\\"right\\"></i></span>
<!---->
<div class=\\"ix-header-content\\">
<div class=\\"ix-header-title-wrapper\\"><span class=\\"ix-header-title\\">header 0</span>
Expand All @@ -69,7 +69,7 @@ exports[`Collapse > render work 1`] = `
</transition-stub>
</div>
<div class=\\"ix-collapse-panel ix-collapse-panel-expanded\\">
<div class=\\"ix-header ix-header-md\\"><span class=\\"ix-header-prefix\\"><i class=\\"ix-icon ix-icon-right\\" style=\\"transform: rotate(90deg);\\" role=\\"img\\" aria-label=\\"right\\"></i></span>
<div class=\\"ix-header ix-header-md\\"><span class=\\"ix-header-prefix\\"><i class=\\"ix-icon ix-icon-right ix-collapse-panel-expand-icon\\" style=\\"transform: rotate(90deg);\\" role=\\"img\\" aria-label=\\"right\\"></i></span>
<!---->
<div class=\\"ix-header-content\\">
<div class=\\"ix-header-title-wrapper\\"><span class=\\"ix-header-title\\">header 1</span>
Expand All @@ -88,7 +88,7 @@ exports[`Collapse > render work 1`] = `
</transition-stub>
</div>
<div class=\\"ix-collapse-panel\\">
<div class=\\"ix-header ix-header-md\\"><span class=\\"ix-header-prefix\\"><i class=\\"ix-icon ix-icon-right\\" style=\\"transform: rotate(0deg);\\" role=\\"img\\" aria-label=\\"right\\"></i></span>
<div class=\\"ix-header ix-header-md\\"><span class=\\"ix-header-prefix\\"><i class=\\"ix-icon ix-icon-right ix-collapse-panel-expand-icon\\" style=\\"transform: rotate(0deg);\\" role=\\"img\\" aria-label=\\"right\\"></i></span>
<!---->
<div class=\\"ix-header-content\\">
<div class=\\"ix-header-title-wrapper\\"><span class=\\"ix-header-title\\">header 2</span>
Expand Down
37 changes: 14 additions & 23 deletions packages/components/table/src/main/body/BodyCell.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,11 +98,6 @@ export default defineComponent({
}
})

const handleClick = (evt: Event) => {
// see https://github.com/IDuxFE/idux/issues/547
evt.stopPropagation()
}

return () => {
const { column } = props

Expand All @@ -111,7 +106,7 @@ export default defineComponent({
let title: string | undefined

if (type === 'selectable') {
children = renderSelectableChildren(props, slots, selectable, handleClick, config, mergedPagination)
children = renderSelectableChildren(props, slots, selectable, config, mergedPagination)
} else if (type === 'indexable') {
children = renderIndexableChildren(props, slots, column as TableColumnIndexable, mergedPagination)
} else {
Expand Down Expand Up @@ -209,35 +204,31 @@ function renderSelectableChildren(
props: TableBodyCellProps,
slots: Slots,
selectable: ComputedRef<TableColumnMergedSelectable | undefined>,
onClick: (evt: Event) => void,
config: TableConfig,
mergedPagination: ComputedRef<TablePagination | null>,
) {
const { selected: checked, indeterminate, disabled, isHover, handleSelect: onChange } = props
const { showIndex, multiple, customCell, trigger } = selectable.value!
const { showIndex, multiple, customCell } = selectable.value!
const onClick = (evt: Event) => {
// see https://github.com/IDuxFE/idux/issues/547
evt.stopPropagation()
// radio 支持反选
if (!multiple && checked && !disabled && onChange) {
onChange()
}
}

if (!checked && !isHover && showIndex) {
return renderIndexableChildren(props, slots, config.columnIndexable as TableColumnIndexable, mergedPagination)
}

const customRender = isString(customCell) ? slots[customCell] : customCell
if (multiple) {
// 存在trigger时将事件代理到bodyCell进行处理
const exitTriggerCheckboxProps = { checked, disabled, indeterminate }
const checkboxProps = { ...exitTriggerCheckboxProps, onChange, onClick }
return customRender ? (
customRender(trigger ? exitTriggerCheckboxProps : checkboxProps)
) : (
<IxCheckbox {...(trigger ? exitTriggerCheckboxProps : checkboxProps)}></IxCheckbox>
)
const checkboxProps = { checked, disabled, indeterminate, onChange, onClick }
return customRender ? customRender(checkboxProps) : <IxCheckbox {...checkboxProps}></IxCheckbox>
} else {
const exitTriggerRadioProps = { checked, disabled }
const radioProps = { ...exitTriggerRadioProps, onChange, onClick }
return customRender ? (
customRender(radioProps)
) : (
<IxRadio {...(trigger ? exitTriggerRadioProps : radioProps)}></IxRadio>
)
const radioProps = { checked, disabled, onChange, onClick }
return customRender ? customRender(radioProps) : <IxRadio {...radioProps}></IxRadio>
}
}

Copy link

Choose a reason for hiding this comment

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

the brief code review:

The patch looks good, overall. It appears that the code is removing the unnecessary "handleClick" function and replacing it with an "onClick" function that includes the additional logic of allowing for radio selection reversal when clicked multiple times. This is a good improvement and should not introduce any bugs.

There are a few minor things to consider for further improvement, however. Firstly, the code could be made more concise by using a ternary operator instead of an if-else statement for the "multiple" condition. Additionally, the logic for the "trigger" variable may still need to be considered, as it doesn't appear to be used in the code, even though it is declared.

Expand Down
7 changes: 6 additions & 1 deletion packages/components/table/style/index.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@

& tr&-row-selected {
&:hover > td {
background-color: @table-body-row-background-color-hover;
background-color: @table-body-row-background-color-hover;
}
& > td {
background: @table-body-row-background-color-selected;
Expand Down Expand Up @@ -356,4 +356,9 @@
}
}
}

// chrome 83: virtual + fixed column
.cdk-virtual-scroll-content {
display: block;
}
}
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 first patch changes the background color of the table rows when they are hovered over. This should improve user experience and make the table more visually appealing.

  2. The second patch adds some code to enable support for Chrome version 83 which includes virtual and fixed column support. This should improve the compatibility of the table with the latest versions of Chrome.

6 changes: 3 additions & 3 deletions packages/components/tree-select/__tests__/__snapshots__/treeSelect.spec.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ exports[`TreeSelect > single work > searchFn work 2`] = `
<!---->
</div>
<div class=\\"ix-selector-suffix\\"><i class=\\"ix-icon ix-icon-down\\" style=\\"transform: rotate(0deg);\\" role=\\"img\\" aria-label=\\"down\\"><svg viewBox=\\"0 0 1024 1024\\" focusable=\\"false\\" aria-hidden=\\"true\\" data-icon=\\"down\\">
<path d=\\"m294.784 411.776 208.192 208.192a12.8 12.8 0 0 0 18.048 0l208.192-208.192a12.8 12.8 0 0 1 18.112 0l27.136 27.2a12.8 12.8 0 0 1 0 18.048l-253.44 253.44a12.8 12.8 0 0 1-18.048 0l-253.44-253.44a12.8 12.8 0 0 1 0-18.048l27.136-27.2a12.8 12.8 0 0 1 18.112 0z\\"></path>
<path d=\\"m294.777 395.325 208.172 208.172c4.999 5 13.103 5 18.102 0l208.172-208.172c4.999-4.999 13.103-4.999 18.102 0l27.153 27.153c4.999 4.999 4.999 13.103 0 18.102L521.051 694.007c-4.999 4.999-13.103 4.999-18.102 0L249.522 440.58c-4.999-4.999-4.999-13.103 0-18.102l27.153-27.153c4.999-4.999 13.103-4.999 18.102 0Z\\"></path>
</svg></i></div>
</div>
</div>"
Expand All @@ -94,7 +94,7 @@ exports[`TreeSelect > single work > searchFn work 3`] = `
<!---->
</div>
<div class=\\"ix-selector-suffix\\"><i class=\\"ix-icon ix-icon-down\\" style=\\"transform: rotate(0deg);\\" role=\\"img\\" aria-label=\\"down\\"><svg viewBox=\\"0 0 1024 1024\\" focusable=\\"false\\" aria-hidden=\\"true\\" data-icon=\\"down\\">
<path d=\\"m294.784 411.776 208.192 208.192a12.8 12.8 0 0 0 18.048 0l208.192-208.192a12.8 12.8 0 0 1 18.112 0l27.136 27.2a12.8 12.8 0 0 1 0 18.048l-253.44 253.44a12.8 12.8 0 0 1-18.048 0l-253.44-253.44a12.8 12.8 0 0 1 0-18.048l27.136-27.2a12.8 12.8 0 0 1 18.112 0z\\"></path>
<path d=\\"m294.777 395.325 208.172 208.172c4.999 5 13.103 5 18.102 0l208.172-208.172c4.999-4.999 13.103-4.999 18.102 0l27.153 27.153c4.999 4.999 4.999 13.103 0 18.102L521.051 694.007c-4.999 4.999-13.103 4.999-18.102 0L249.522 440.58c-4.999-4.999-4.999-13.103 0-18.102l27.153-27.153c4.999-4.999 13.103-4.999 18.102 0Z\\"></path>
</svg></i></div>
</div>
</div>"
Expand Down Expand Up @@ -125,7 +125,7 @@ exports[`TreeSelect > single work > searchable work 2`] = `
<!---->
</div>
<div class=\\"ix-selector-suffix\\"><i class=\\"ix-icon ix-icon-down\\" style=\\"transform: rotate(180deg);\\" role=\\"img\\" aria-label=\\"down\\"><svg viewBox=\\"0 0 1024 1024\\" focusable=\\"false\\" aria-hidden=\\"true\\" data-icon=\\"down\\">
<path d=\\"m294.784 411.776 208.192 208.192a12.8 12.8 0 0 0 18.048 0l208.192-208.192a12.8 12.8 0 0 1 18.112 0l27.136 27.2a12.8 12.8 0 0 1 0 18.048l-253.44 253.44a12.8 12.8 0 0 1-18.048 0l-253.44-253.44a12.8 12.8 0 0 1 0-18.048l27.136-27.2a12.8 12.8 0 0 1 18.112 0z\\"></path>
<path d=\\"m294.777 395.325 208.172 208.172c4.999 5 13.103 5 18.102 0l208.172-208.172c4.999-4.999 13.103-4.999 18.102 0l27.153 27.153c4.999 4.999 4.999 13.103 0 18.102L521.051 694.007c-4.999 4.999-13.103 4.999-18.102 0L249.522 440.58c-4.999-4.999-4.999-13.103 0-18.102l27.153-27.153c4.999-4.999 13.103-4.999 18.102 0Z\\"></path>
</svg></i></div>
</div>
</div>
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 code patch looks like it is making some changes to a tree selector in the codebase. The first thing that can be noticed is the change in the path attribute of the SVG element. It looks like the old path was changed with a new one and the coordinates were adjusted. This could be a fix to improve the visual look of the selector or make it more efficient.

It is also possible to suggest some improvements to this code. It would be good to add checks for whether the new coordinates are valid or not, as it could cause issues if the new coordinates are invalid. This can be done by adding an if condition before the change is made to ensure it is valid. Additionally, it would be beneficial to add comments to explain the purpose of the change, which will make it easier for future developers to understand the code.

Expand Down