Skip to content

refactor(continue): fixed shadow using logical position #1244

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

Merged
merged 4 commits into from
Mar 10, 2025
Merged

refactor(continue): fixed shadow using logical position #1244

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0859d1b
feat: position fixed also use logical position
zombieJ Mar 10, 2025
80573f2
test: fix part
zombieJ Mar 10, 2025
c0cf709
test: test case
zombieJ Mar 10, 2025
a79e7d0
chore: fix lint
zombieJ Mar 10, 2025
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
26 changes: 16 additions & 10 deletions assets/index.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,18 +112,24 @@
}
}

&-ping-left {
.@{tablePrefixCls}-cell-fix-left-first::after,
.@{tablePrefixCls}-cell-fix-left-last::after {
box-shadow: inset 10px 0 8px -8px green;
}
&-fix-start-shadow::before,
&-fix-end-shadow::after {
content: '';
position: absolute;
z-index: 1;
top: 0;
bottom: 0;
width: 5px;
}

&-ping-right {
.@{tablePrefixCls}-cell-fix-right-first::after,
.@{tablePrefixCls}-cell-fix-right-last::after {
box-shadow: inset -10px 0 8px -8px green;
}
&-fix-start-shadow-show::before {
inset-inline-start: 0;
background: rgba(0, 0, 0, 0.5);
}

&-fix-end-shadow-show::after {
inset-inline-end: 0;
background: rgba(0, 0, 0, 0.5);
}

// ================= Expand =================
Expand Down
4 changes: 1 addition & 3 deletions src/Cell/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,7 @@ function Cell<RecordType>(props: CellProps<RecordType>) {
return [false, false];
}

const [scroll, scrollWidth] = scrollInfo;

const absScroll = Math.abs(scroll);
const [absScroll, scrollWidth] = scrollInfo;

const showStartShadow = isFixStart && fixedStartShadow && absScroll > fixStart;
const showEndShadow = isFixEnd && fixedEndShadow && scrollWidth - absScroll > fixEnd;
Expand Down
37 changes: 21 additions & 16 deletions src/Table.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -347,8 +347,8 @@ function Table<RecordType extends DefaultRecordType>(

// ====================== Scroll ======================
const scrollSummaryRef = React.useRef<HTMLDivElement>();
const [pingedLeft, setPingedLeft] = React.useState(false);
const [pingedRight, setPingedRight] = React.useState(false);
const [shadowStart, setShadowStart] = React.useState(false);
const [shadowEnd, setShadowEnd] = React.useState(false);
const [colsWidths, updateColsWidths] = useLayoutState(new Map<React.Key, number>());

// Convert map to number width
Expand Down Expand Up @@ -463,24 +463,27 @@ function Table<RecordType extends DefaultRecordType>(
: measureTarget.scrollWidth;
const clientWidth = measureTarget.clientWidth;

const absScrollStart = Math.abs(mergedScrollLeft);
setScrollInfo(ori => {
const nextScrollInfo: ScrollInfoType = [mergedScrollLeft, scrollWidth - clientWidth];
const nextScrollInfo: ScrollInfoType = [absScrollStart, scrollWidth - clientWidth];
return isEqual(ori, nextScrollInfo) ? ori : nextScrollInfo;
});

// There is no space to scroll
if (scrollWidth === clientWidth) {
setPingedLeft(false);
setPingedRight(false);
setShadowStart(false);
setShadowEnd(false);
return;
}
if (isRTL) {
setPingedLeft(-mergedScrollLeft < scrollWidth - clientWidth);
setPingedRight(-mergedScrollLeft > 0);
} else {
setPingedLeft(mergedScrollLeft > 0);
setPingedRight(mergedScrollLeft < scrollWidth - clientWidth);
}
// if (isRTL) {
// setPingedStart(-mergedScrollLeft < scrollWidth - clientWidth);
// setPingedEnd(-mergedScrollLeft > 0);
// } else {
// setPingedStart(mergedScrollLeft > 0);
// setPingedEnd(mergedScrollLeft < scrollWidth - clientWidth);
// }
setShadowStart(absScrollStart > 0);
setShadowEnd(absScrollStart < scrollWidth - clientWidth);
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

改进滚动逻辑以使用绝对值

使用 absScrollStart（mergedScrollLeft 的绝对值）统一处理 RTL 和 LTR 方向的滚动逻辑，简化了之前区分方向的实现方式。

不过有一个小问题：isRTL 被赋值了但未使用（参见被注释掉的旧代码）。

const isRTL = direction === 'rtl';
-const mergedScrollLeft =
-  typeof scrollLeft === 'number' ? scrollLeft : currentTarget.scrollLeft;
+const mergedScrollLeft =
+  typeof scrollLeft === 'number' ? scrollLeft : currentTarget.scrollLeft;

考虑移除未使用的变量以解决 CI 错误。

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

},
);
Expand All @@ -497,8 +500,8 @@ function Table<RecordType extends DefaultRecordType>(
scrollLeft: scrollBodyRef.current?.scrollLeft,
});
} else {
setPingedLeft(false);
setPingedRight(false);
setShadowStart(false);
setShadowEnd(false);
}
};

Expand Down Expand Up @@ -771,8 +774,10 @@ function Table<RecordType extends DefaultRecordType>(
<div
className={classNames(prefixCls, className, {
[`${prefixCls}-rtl`]: direction === 'rtl',
[`${prefixCls}-ping-left`]: pingedLeft,
[`${prefixCls}-ping-right`]: pingedRight,
[`${prefixCls}-fix-start-shadow`]: horizonScroll,
[`${prefixCls}-fix-end-shadow`]: horizonScroll,
[`${prefixCls}-fix-start-shadow-show`]: horizonScroll && shadowStart,
[`${prefixCls}-fix-end-shadow-show`]: horizonScroll && shadowEnd,
[`${prefixCls}-layout-fixed`]: tableLayout === 'fixed',
[`${prefixCls}-fixed-header`]: fixHeader,
/** No used but for compatible */
Expand Down
40 changes: 10 additions & 30 deletions tests/FixedColumn.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,12 +140,8 @@ describe('Table.FixedColumn', () => {
fireEvent.scroll(tableContent);
}
});
expect(
container.querySelector('.rc-table')?.classList.contains('rc-table-ping-left'),
).toBeTruthy();
expect(
container.querySelector('.rc-table')?.classList.contains('rc-table-ping-right'),
).toBeTruthy();
expect(container.querySelector('.rc-table')).toHaveClass('rc-table-fix-start-shadow-show');
expect(container.querySelector('.rc-table')).toHaveClass('rc-table-fix-end-shadow-show');

act(() => {
if (tableContent) {
Expand All @@ -155,12 +151,8 @@ describe('Table.FixedColumn', () => {
fireEvent.scroll(tableContent);
}
});
expect(
container.querySelector('.rc-table')?.classList.contains('rc-table-ping-left'),
).toBeFalsy();
expect(
container.querySelector('.rc-table')?.classList.contains('rc-table-ping-right'),
).toBeTruthy();
expect(container.querySelector('.rc-table')).not.toHaveClass('rc-table-fix-start-shadow-show');
expect(container.querySelector('.rc-table')).toHaveClass('rc-table-fix-end-shadow-show');

act(() => {
if (tableContent) {
Expand All @@ -170,12 +162,8 @@ describe('Table.FixedColumn', () => {
fireEvent.scroll(tableContent);
}
});
expect(
container.querySelector('.rc-table')?.classList.contains('rc-table-ping-left'),
).toBeTruthy();
expect(
container.querySelector('.rc-table')?.classList.contains('rc-table-ping-right'),
).toBeFalsy();
expect(container.querySelector('.rc-table')).toHaveClass('rc-table-fix-start-shadow-show');
expect(container.querySelector('.rc-table')).not.toHaveClass('rc-table-fix-end-shadow-show');

act(() => {
if (tableContent) {
Expand All @@ -185,12 +173,8 @@ describe('Table.FixedColumn', () => {
fireEvent.scroll(tableContent);
}
});
expect(
container.querySelector('.rc-table')?.classList.contains('rc-table-ping-left'),
).toBeFalsy();
expect(
container.querySelector('.rc-table')?.classList.contains('rc-table-ping-right'),
).toBeFalsy();
expect(container.querySelector('.rc-table')).not.toHaveClass('rc-table-fix-start-shadow-show');
expect(container.querySelector('.rc-table')).not.toHaveClass('rc-table-fix-end-shadow-show');
});

it('ellipsis will wrap additional dom', () => {
Expand Down Expand Up @@ -349,11 +333,7 @@ describe('Table.FixedColumn', () => {
fireEvent.scroll(tableContent);
}
});
expect(
container.querySelector('.rc-table')?.classList.contains('rc-table-ping-left'),
).toBeTruthy();
expect(
container.querySelector('.rc-table')?.classList.contains('rc-table-ping-right'),
).toBeTruthy();
expect(container.querySelector('.rc-table')).toHaveClass('rc-table-fix-start-shadow-show');
expect(container.querySelector('.rc-table')).toHaveClass('rc-table-fix-end-shadow-show');
});
});
8 changes: 2 additions & 6 deletions tests/Virtual.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -528,9 +528,7 @@ describe('Table.Virtual', () => {

await waitFakeTimer();

expect(
container.querySelector('.rc-table').classList.contains('rc-table-ping-right'),
).toBeTruthy();
expect(container.querySelector('.rc-table')).toHaveClass('rc-table-fix-end-shadow-show');
});

it('right shadow should display correctly when showHeader is false', async () => {
Expand Down Expand Up @@ -558,9 +556,7 @@ describe('Table.Virtual', () => {

await waitFakeTimer();

expect(
container.querySelector('.rc-table').classList.contains('rc-table-ping-right'),
).toBeTruthy();
expect(container.querySelector('.rc-table')).toHaveClass('rc-table-fix-end-shadow-show');
});
});
});
10 changes: 5 additions & 5 deletions tests/__snapshots__/ExpandRow.spec.jsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ exports[`Table.Expand > childrenColumnName 1`] = `

exports[`Table.Expand > does not crash if scroll is not set 1`] = `
<div
class="rc-table rc-table-scroll-horizontal"
class="rc-table rc-table-fix-start-shadow rc-table-fix-end-shadow rc-table-scroll-horizontal"
>
<div
class="rc-table-container"
Expand Down Expand Up @@ -229,7 +229,7 @@ exports[`Table.Expand > does not crash if scroll is not set 1`] = `

exports[`Table.Expand > does not crash if scroll is not set 2`] = `
<div
class="rc-table rc-table-scroll-horizontal"
class="rc-table rc-table-fix-start-shadow rc-table-fix-end-shadow rc-table-scroll-horizontal"
>
<div
class="rc-table-container"
Expand Down Expand Up @@ -438,7 +438,7 @@ exports[`Table.Expand > not use nest when children is invalidate 1`] = `

exports[`Table.Expand > renders fixed column correctly > work 1`] = `
<div
class="rc-table rc-table-fixed-column rc-table-scroll-horizontal rc-table-has-fix-left rc-table-has-fix-right"
class="rc-table rc-table-fix-start-shadow rc-table-fix-end-shadow rc-table-fixed-column rc-table-scroll-horizontal rc-table-has-fix-left rc-table-has-fix-right"
>
<div
class="rc-table-container"
Expand Down Expand Up @@ -906,7 +906,7 @@ exports[`Table.Expand > renders tree row correctly with different children 1`] =

exports[`Table.Expand > work in expandable fix 1`] = `
<div
class="rc-table rc-table-scroll-horizontal"
class="rc-table rc-table-fix-start-shadow rc-table-fix-end-shadow rc-table-scroll-horizontal"
>
<div
class="rc-table-container"
Expand Down Expand Up @@ -1028,7 +1028,7 @@ exports[`Table.Expand > work in expandable fix 1`] = `

exports[`Table.Expand > work in expandable fix 2`] = `
<div
class="rc-table rc-table-fixed-column rc-table-scroll-horizontal"
class="rc-table rc-table-fix-start-shadow rc-table-fix-end-shadow rc-table-fixed-column rc-table-scroll-horizontal"
>
<div
class="rc-table-container"
Expand Down
10 changes: 5 additions & 5 deletions tests/__snapshots__/FixedColumn.spec.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

exports[`Table.FixedColumn > fixed column renders correctly RTL 1`] = `
<div
class="rc-table rc-table-rtl rc-table-fixed-column rc-table-scroll-horizontal rc-table-has-fix-left"
class="rc-table rc-table-rtl rc-table-fix-start-shadow rc-table-fix-end-shadow rc-table-fixed-column rc-table-scroll-horizontal rc-table-has-fix-left"
>
<div
class="rc-table-container"
Expand Down Expand Up @@ -753,7 +753,7 @@ exports[`Table.FixedColumn > renders correctly > all column has width should use

exports[`Table.FixedColumn > renders correctly > scrollX - with data 1`] = `
<div
class="rc-table rc-table-fixed-column rc-table-scroll-horizontal rc-table-has-fix-left rc-table-has-fix-right"
class="rc-table rc-table-fix-start-shadow rc-table-fix-end-shadow rc-table-fixed-column rc-table-scroll-horizontal rc-table-has-fix-left rc-table-has-fix-right"
>
<div
class="rc-table-container"
Expand Down Expand Up @@ -1542,7 +1542,7 @@ exports[`Table.FixedColumn > renders correctly > scrollX - with data 1`] = `

exports[`Table.FixedColumn > renders correctly > scrollX - without data 1`] = `
<div
class="rc-table rc-table-fixed-column rc-table-scroll-horizontal rc-table-has-fix-left rc-table-has-fix-right"
class="rc-table rc-table-fix-start-shadow rc-table-fix-end-shadow rc-table-fixed-column rc-table-scroll-horizontal rc-table-has-fix-left rc-table-has-fix-right"
>
<div
class="rc-table-container"
Expand Down Expand Up @@ -1801,7 +1801,7 @@ exports[`Table.FixedColumn > renders correctly > scrollX - without data 1`] = `

exports[`Table.FixedColumn > renders correctly > scrollXY - with data 1`] = `
<div
class="rc-table rc-table-fixed-header rc-table-fixed-column rc-table-scroll-horizontal rc-table-has-fix-left rc-table-has-fix-right"
class="rc-table rc-table-fix-start-shadow rc-table-fix-end-shadow rc-table-fixed-header rc-table-fixed-column rc-table-scroll-horizontal rc-table-has-fix-left rc-table-has-fix-right"
>
<div
class="rc-table-container"
Expand Down Expand Up @@ -2644,7 +2644,7 @@ exports[`Table.FixedColumn > renders correctly > scrollXY - with data 1`] = `

exports[`Table.FixedColumn > renders correctly > scrollXY - without data 1`] = `
<div
class="rc-table rc-table-fixed-header rc-table-fixed-column rc-table-scroll-horizontal rc-table-has-fix-left rc-table-has-fix-right"
class="rc-table rc-table-fix-start-shadow rc-table-fix-end-shadow rc-table-fixed-header rc-table-fixed-column rc-table-scroll-horizontal rc-table-has-fix-left rc-table-has-fix-right"
>
<div
class="rc-table-container"
Expand Down
4 changes: 2 additions & 2 deletions tests/__snapshots__/Table.spec.jsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ exports[`Table.Basic > custom components > renders correctly 1`] = `

exports[`Table.Basic > custom components > renders fixed column and header correctly 1`] = `
<div
class="rc-table rc-table-fixed-header rc-table-fixed-column rc-table-scroll-horizontal rc-table-has-fix-left rc-table-has-fix-right"
class="rc-table rc-table-fix-start-shadow rc-table-fix-end-shadow rc-table-fixed-header rc-table-fixed-column rc-table-scroll-horizontal rc-table-has-fix-left rc-table-has-fix-right"
>
<div
class="rc-table-container"
Expand Down Expand Up @@ -209,7 +209,7 @@ exports[`Table.Basic > custom components > renders fixed column and header corre

exports[`Table.Basic > custom components > scroll content > with scroll 1`] = `
<div
class="rc-table rc-table-fixed-header rc-table-scroll-horizontal"
class="rc-table rc-table-fix-start-shadow rc-table-fix-end-shadow rc-table-fixed-header rc-table-scroll-horizontal"
>
<div
class="rc-table-container"
Expand Down
Loading