refactor(continue): fixed shadow using logical position #1244
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Walkthrough整个 PR 的更改主要集中在更新表格阴影效果的实现方式。旧的 CSS 类（用于 box-shadow）被替换为基于伪元素的新类，以更精确地控制阴影显示。同时，组件中的滚动相关变量顺序及状态更新逻辑也被调整，以适应新的阴影实现方式，相关测试用例也更新了断言以匹配新样式。 Changes
Sequence Diagram(s)sequenceDiagram
participant U as 用户
participant T as Table组件
participant C as Cell组件
participant B as 浏览器
U->>T: 触发滚动事件
T->>T: 计算 mergedScrollLeft，更新 shadowStart 与 shadowEnd
T->>C: 分发滚动数据
C->>B: 利用新 CSS 类渲染阴影效果
Possibly related PRs
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
src/Table.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "prettier" to extend from. Please check that the name of the config is correct. The config "prettier" was referenced from the config file in "/.eslintrc.js". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
src/Table.tsx (1)
443-443:⚠️ Potential issue修复未使用变量的问题
变量
isRTL被赋值但从未使用，这导致了 CI 失败。考虑移除该变量或在代码中使用它。-const isRTL = direction === 'rtl';🧰 Tools
🪛 GitHub Actions: ✅ test
[error] 443-443: 'isRTL' is assigned a value but never used
🧹 Nitpick comments (2)
src/Table.tsx (2)
357-357: React Hook 依赖数组问题React.useMemo Hook 存在依赖数组问题：
- 缺少
pureColWidths依赖- 依赖数组包含复杂表达式
pureColWidths.join('_')建议修改为：
-const colWidths = React.useMemo(() => pureColWidths, [pureColWidths.join('_')]); +const pureColWidthsKey = React.useMemo(() => pureColWidths.join('_'), [pureColWidths]); +const colWidths = React.useMemo(() => pureColWidths, [pureColWidthsKey]);🧰 Tools
🪛 GitHub Actions: ✅ test
[warning] 357-357: React Hook React.useMemo has a missing dependency: 'pureColWidths'. Either include it or remove the dependency array.
[warning] 357-357: React Hook React.useMemo has a complex expression in the dependency array. Extract it to a separate variable so it can be statically checked.
416-416: 其他 React Hook 依赖数组警告CI 警告显示多处 React Hook 依赖数组不完整：
- 第 416 行：React.useCallback 缺少
updateColsWidths依赖- 第 529 行：React.useEffect 缺少
triggerOnScroll依赖- 第 545 行：React.useEffect 缺少
tailor和useInternalHooks依赖- 第 591 行：React.useMemo 缺少
mergedScrollX依赖建议补充这些依赖以避免潜在的闭包陷阱问题。
Also applies to: 529-529, 545-545, 591-591
🧰 Tools
🪛 GitHub Actions: ✅ test
[warning] 416-416: React Hook React.useCallback has a missing dependency: 'updateColsWidths'. Either include it or remove the dependency array.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
tests/__snapshots__/ExpandRow.spec.jsx.snapis excluded by!**/*.snaptests/__snapshots__/FixedColumn.spec.tsx.snapis excluded by!**/*.snaptests/__snapshots__/Table.spec.jsx.snapis excluded by!**/*.snap
📒 Files selected for processing (5)
assets/index.less(1 hunks)src/Cell/index.tsx(1 hunks)src/Table.tsx(4 hunks)tests/FixedColumn.spec.tsx(5 hunks)tests/Virtual.spec.tsx(2 hunks)
🧰 Additional context used
🪛 GitHub Actions: ✅ test
src/Table.tsx
[error] 443-443: 'isRTL' is assigned a value but never used
[warning] 357-357: React Hook React.useMemo has a missing dependency: 'pureColWidths'. Either include it or remove the dependency array.
[warning] 357-357: React Hook React.useMemo has a complex expression in the dependency array. Extract it to a separate variable so it can be statically checked.
[warning] 416-416: React Hook React.useCallback has a missing dependency: 'updateColsWidths'. Either include it or remove the dependency array.
[warning] 529-529: React Hook React.useEffect has a missing dependency: 'triggerOnScroll'. Either include it or remove the dependency array.
[warning] 545-545: React Hook React.useEffect has missing dependencies: 'tailor' and 'useInternalHooks'. Either include them or remove the dependency array.
[warning] 591-591: React Hook React.useMemo has a missing dependency: 'mergedScrollX'. Either include it or remove the dependency array.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test / react component workflow
🔇 Additional comments (13)
src/Cell/index.tsx (1)
144-144: 变量顺序调整提升了代码可读性修改了
scrollInfo数组的解构顺序，将absScroll（绝对滚动值）放在了第一位。这个改动配合新的阴影实现方式，使变量命名和使用逻辑更加清晰。tests/Virtual.spec.tsx (2)
531-531: 更新了测试断言以匹配新的阴影类名将原来检查
rc-table-ping-right类的断言更新为检查rc-table-fix-end-shadow-show类，与CSS样式修改保持一致。测试逻辑本身没有变化，仅更新了类名。
559-559: 同步更新了无表头情况下的阴影类断言这里同样将断言从检查
rc-table-ping-right更新为rc-table-fix-end-shadow-show，确保在showHeader为false的场景下也能正确测试阴影效果。assets/index.less (2)
115-123: 新增了基于伪元素的阴影基础样式添加了使用伪元素(::before/::after)实现阴影效果的基础样式定义，取代了之前的box-shadow实现。这种方法能提供更精细的阴影控制，并且性能更好。
125-133: 实现了显示状态的逻辑位置阴影样式使用
inset-inline-start和inset-inline-end替代了传统的左右定位，这种逻辑属性更好地支持了RTL（从右到左）布局，提高了组件的国际化适应性。同时半透明的背景提供了更自然的阴影效果。tests/FixedColumn.spec.tsx (5)
143-144: 更新测试断言以匹配新的阴影类名将原来检查
rc-table-ping-left/right类的断言更新为检查rc-table-fix-start/end-shadow-show类，保持与新样式实现的一致性。
154-155: 更新了顶部未滚动状态的阴影断言在滚动位置为0时，更新了对应的断言，确保start阴影不显示而end阴影显示的状态检查正确。
165-166: 更新了底部完全滚动状态的阴影断言在滚动到底部时，确保start阴影显示而end阴影不显示的状态检查正确。
176-177: 更新了无需阴影状态的断言当滚动内容宽度等于容器宽度时，更新了检查两侧阴影都不显示的断言。
336-337: 更新了水平滚动场景的阴影检查断言确保在内容宽度大于滚动宽度的情况下，正确检查两侧阴影的显示状态。
src/Table.tsx (3)
350-351: 状态变量更名更符合语义将
pingedLeft和pingedRight重命名为shadowStart和shadowEnd，这更符合实际实现的视觉效果（阴影）而非 ping 概念。
503-504: 更新触发滚动方法中的状态设置正确地将
setPingedLeft/setPingedRight更新为setShadowStart/setShadowEnd，保持了代码一致性。
777-781: 更新 CSS 类名以使用阴影概念用新的基于阴影概念的类名替换了基于 ping 概念的类名，更好地描述了实际效果：
${prefixCls}-fix-start-shadow和${prefixCls}-fix-end-shadow用于设置阴影基础样式${prefixCls}-fix-start-shadow-show和${prefixCls}-fix-end-shadow-show用于控制阴影显示这种改变使得类名更符合实际的视觉效果。
| 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); | ||
| } |
There was a problem hiding this comment.
改进滚动逻辑以使用绝对值
使用 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.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1244 +/- ##
==========================================
+ Coverage 98.00% 98.02% +0.01%
==========================================
Files 76 76
Lines 7427 7399 -28
Branches 1145 1145
==========================================
- Hits 7279 7253 -26
+ Misses 142 140 -2
Partials 6 6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
1 participant
Summary by CodeRabbit