fix(backtop & menu): lint, code simplification, deprecated pageYOffset removed #2633
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## next #2633 +/- ##
==========================================
+ Coverage 82.98% 83.41% +0.42%
==========================================
Files 219 218 -1
Lines 17911 17892 -19
Branches 2548 2576 +28
==========================================
+ Hits 14864 14924 +60
+ Misses 3042 2963 -79
Partials 5 5 ☔ View full report in Codecov by Sentry. |
Walkthrough
Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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: 4
🧹 Outside diff range and nitpick comments (10)
src/utils/raf.ts (1)
5-5:requestAniFrame函数的改进使用
inBrowser常量来检查浏览器环境是一个很好的改进。这使得代码更加一致和易于维护。为了进一步提高一致性，建议将函数内部的
_window声明移到条件语句外部，类似于cancelRaf函数中的做法。这样可以使两个函数的结构更加统一。建议如下修改：
function requestAniFrame() { + const _window = window as any if (inBrowser) { - const _window = window as any return ( _window.requestAnimationFrame || _window.webkitRequestAnimationFrame || function (callback: any) { _window.setTimeout(callback, 1000 / 60) } ) } return (callback: any) => { setTimeout(callback, 1000 / 60) } }src/packages/backtop/backtop.taro.tsx (2)
59-59: 事件处理优化建议将
onClick处理程序直接内联到 div 元素中是个不错的简化。这减少了不必要的箭头函数嵌套，使代码更加简洁。建议考虑使用
useCallback钩子来优化goTop函数，特别是如果这个组件经常重新渲染的话。这可以进一步提高性能。例如：const goTop = useCallback((e: MouseEvent<HTMLDivElement>) => { onClick && onClick(e) pageScrollTo({ scrollTop: 0, duration: duration > 0 ? duration : 0, }) }, [onClick, duration])这样可以确保
goTop函数只在其依赖项（onClick和duration）发生变化时才重新创建。
36-36: 命名约定建议为了保持一致性和遵循 React 的常见命名约定，建议将状态设置函数
SetBackTop重命名为setBackTop。这样可以使代码更符合 React 的最佳实践，并提高可读性。src/packages/backtop/backtop.tsx (5)
53-53: 建议为scrollEl指定更具体的类型。目前
scrollEl被定义为useRef<any>(null)，使用any类型可能降低类型安全性。建议为其指定更精确的类型，例如useRef<HTMLElement | Window | null>(null)。
56-65: 使用useCallback优化scrollListener函数，建议添加测试覆盖。
scrollListener函数经过优化，但静态分析工具提示第57-64行未被测试覆盖。建议为此部分代码添加单元测试，确保功能的正确性。🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 57-64: src/packages/backtop/backtop.tsx#L57-L64
Added lines #L57 - L64 were not covered by tests
67-75: 使用useCallback优化init函数，建议添加测试覆盖。
init函数进行了优化，但静态分析工具指出第69行未被测试覆盖。建议添加相应的测试，确保在不同情况下函数的可靠性。🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 69-69: src/packages/backtop/backtop.tsx#L69
Added line #L69 was not covered by tests
101-101: 在scrollAnimation中使用cancelRaf，建议添加测试覆盖。你使用
cancelRaf来取消动画帧，确保了滚动动画的正确终止。静态分析工具提示该行未被测试覆盖，建议添加测试以验证动画中止的正确性。🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 101-101: src/packages/backtop/backtop.tsx#L101
Added line #L101 was not covered by tests
108-108: 建议使用Date.now()替代+new Date()获取时间戳。
Date.now()更直观，性能更优。应用以下修改：
- startTime = +new Date() + startTime = Date.now()🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 108-108: src/packages/backtop/backtop.tsx#L108
Added line #L108 was not covered by testssrc/packages/menu/menu.taro.tsx (1)
Line range hint
84-106: 避免直接修改状态，遵循 React 状态不可变原则在
toggleMenuItem、hideMenuItem和updateTitle函数中，直接修改了状态数组，例如：showMenuItem[index] = !showMenuItem[index];直接修改状态可能导致不可预测的行为，因为 React 依赖于状态的不可变性来确定何时重新渲染组件。
建议修改为：
// 对于 toggleMenuItem 函数 - showMenuItem[index] = !showMenuItem[index]; - const temp = showMenuItem.map((i: boolean, idx) => - idx === index ? i : false - ); - setShowMenuItem([...temp]); + const temp = showMenuItem.map((item, idx) => + idx === index ? !item : false + ); + setShowMenuItem(temp);// 对于 hideMenuItem 函数 - showMenuItem[index] = false; - setShowMenuItem([...showMenuItem]); + const temp = showMenuItem.map((item, idx) => + idx === index ? false : item + ); + setShowMenuItem(temp);// 对于 updateTitle 函数 - menuItemTitle[index] = text; - setMenuItemTitle([...menuItemTitle]); + const tempTitle = menuItemTitle.slice(); + tempTitle[index] = text; + setMenuItemTitle(tempTitle);通过创建状态的新副本，遵循了 React 的不可变性原则，避免了直接修改状态带来的潜在问题。
src/packages/menu/menu.tsx (1)
70-70:getScrollTop函数未被测试覆盖。检测到
getScrollTop函数的新实现未被测试覆盖，建议添加相应的单元测试以确保其在各种情况下的正确性。您是否需要我协助编写针对该函数的单元测试？
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 70-70: src/packages/menu/menu.tsx#L70
Added line #L70 was not covered by tests
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
- src/packages/backtop/backtop.taro.tsx (2 hunks)
- src/packages/backtop/backtop.tsx (4 hunks)
- src/packages/menu/menu.taro.tsx (3 hunks)
- src/packages/menu/menu.tsx (3 hunks)
- src/utils/raf.ts (2 hunks)
🧰 Additional context used
🪛 GitHub Check: codecov/patch
src/packages/backtop/backtop.tsx
[warning] 57-64: src/packages/backtop/backtop.tsx#L57-L64
Added lines #L57 - L64 were not covered by tests
[warning] 69-69: src/packages/backtop/backtop.tsx#L69
Added line #L69 was not covered by tests
[warning] 101-101: src/packages/backtop/backtop.tsx#L101
Added line #L101 was not covered by tests
[warning] 108-108: src/packages/backtop/backtop.tsx#L108
Added line #L108 was not covered by testssrc/packages/menu/menu.tsx
[warning] 70-70: src/packages/menu/menu.tsx#L70
Added line #L70 was not covered by testssrc/utils/raf.ts
[warning] 22-23: src/utils/raf.ts#L22-L23
Added lines #L22 - L23 were not covered by tests
🪛 Biome
src/packages/backtop/backtop.tsx
[error] 107-107: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (17)
src/utils/raf.ts (3)
Line range hint
1-1: 新增的inBrowser常量提高了代码质量这个新增的常量很好地封装了浏览器环境检查的逻辑。它提高了代码的可读性和可重用性，符合代码简化的目标。这个改动可以减少重复代码，使得后续的浏览器环境检查更加简洁明了。
Line range hint
1-30: 总体评价这些更改很好地实现了代码简化和性能优化的目标。引入
inBrowser常量提高了代码的可读性和可重用性，而对requestAniFrame和cancelRaf函数的修改增强了代码的一致性和可维护性。主要改进点：
- 代码结构更清晰，易于理解和维护。
- 通过使用
inBrowser常量，减少了重复的环境检查代码。- 增强了浏览器兼容性处理。
建议关注点：
- 确保为新增和修改的代码添加适当的单元测试，特别是
cancelRaf函数。- 考虑统一
requestAniFrame和cancelRaf函数的结构，以进一步提高一致性。总的来说，这些更改是积极的，提高了代码质量，符合项目的优化目标。
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 22-23: src/utils/raf.ts#L22-L23
Added lines #L22 - L23 were not covered by tests
22-23:cancelRaf函数的改进和测试覆盖建议
cancelRaf函数的修改提高了代码的可读性和可维护性。使用inBrowser常量和引入本地_window变量的做法很好，增强了代码的封装性。然而，静态分析工具指出这些新增的行没有被测试覆盖。建议添加相应的单元测试，以确保这些更改的正确性和稳定性。
为了验证测试覆盖情况，可以运行以下脚本：
如果没有找到相关的测试，建议添加针对
cancelRaf函数的单元测试。🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 22-23: src/utils/raf.ts#L22-L23
Added lines #L22 - L23 were not covered by testssrc/packages/backtop/backtop.taro.tsx (1)
33-33: 代码优化：使用 classNames 函数简化类名逻辑这个改动很好地优化了类名的处理逻辑。使用
classNames函数可以更清晰、简洁地组合多个类名，包括条件类名。这种方法提高了代码的可读性和可维护性。src/packages/backtop/backtop.tsx (2)
6-6: 引入了useCallback优化性能，良好的实践。通过使用
useCallback，优化了函数的性能，避免了不必要的重新渲染。
12-12: 引入cancelRaf替代cancelAnimationFrame，提高了兼容性。从
@/utils/raf中引入cancelRaf，确保了在不同浏览器环境下的动画取消功能。src/packages/menu/menu.taro.tsx (6)
1-7: 引入useCallback优化性能您在第 3 行引入了
useCallback钩子，用于优化函数的性能，这是一个良好的实践。
62-63: 新增状态变量管理菜单项添加了
showMenuItem和menuItemTitle两个状态变量，用于跟踪菜单项的可见性和标题，增强了组件的状态管理。
65-67: 优化类名构建，支持滚动固定功能使用
classNames库构建类名，并根据isScrollFixed动态添加'scroll-fixed'类，增强了代码的可读性。
72-77: 确保useCallback的依赖项完整在使用
useCallback定义onScroll函数时，依赖项数组为[scrollFixed]。请确认在onScroll函数内部是否还使用了其他状态或属性，以确保依赖项数组的完整性，防止潜在的状态同步问题。
84-84:useEffect依赖项更新，保证事件监听正确在
useEffect中，依赖项数组包含了[scrollFixed, onScroll]，确保了当scrollFixed或onScroll变化时，事件监听器能够正确更新。
Line range hint
188-193: 优化组件结构，提升可读性使用
{...rest}将剩余的属性传递给根节点，并使用cls作为根节点的className，使代码更简洁明了。src/packages/menu/menu.tsx (5)
1-7: 引入必要的React钩子，优化组件性能。新增了
useCallback等React钩子的引入，有助于优化组件性能和状态管理。
62-63: 新增状态变量以跟踪菜单项的可见性和标题。使用
useState新增了showMenuItem和menuItemTitle状态变量，便于管理菜单项的显示状态和标题信息。
65-67: 优化ClassName的处理，提高组件的可读性。使用
classNames库构建CSS类名，简化了条件类名的管理，代码更简洁清晰。
84-84: 正确设置useEffect的依赖数组，确保事件监听器更新。
useEffect的依赖数组包含了scrollFixed和onScroll，保证在依赖变化时更新事件监听器，避免内存泄漏或无效的事件监听。
190-190: 正确传递剩余属性和引用，增强组件的灵活性。使用
{...rest}传递剩余属性，赋予组件更大的灵活性，同时正确设置ref引用，确保对DOM元素的直接访问。
| scrollTop >= threshold ? SetBackTop(true) : SetBackTop(false) | ||
| }) | ||
|
|
||
| // 返回顶部点击事件 | ||
| const goTop = (e: MouseEvent<HTMLDivElement>) => { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
性能优化建议
建议使用 useCallback 钩子来优化滚动处理函数，特别是当 threshold 可能经常变化时。这可以防止不必要的重新渲染。例如：
const handleScroll = useCallback((res: { scrollTop: number }) => {
const { scrollTop } = res
setBackTop(scrollTop >= threshold)
}, [threshold])
usePageScroll(handleScroll)这样可以确保只有当 threshold 发生变化时，才会创建新的滚动处理函数。
| scrollEl.current?.removeEventListener('scroll', scrollListener, false) | ||
| scrollEl.current?.removeEventListener('resize', scrollListener, false) | ||
| } | ||
|
|
||
| const goTop = (e: MouseEvent<HTMLDivElement>) => { | ||
| onClick && onClick(e) |
There was a problem hiding this comment.
建议使用可选链调用onClick函数。
可以将onClick && onClick(e)替换为onClick?.(e)，使代码更简洁。
应用以下修改：
- onClick && onClick(e)
+ onClick?.(e)📝 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.
| onClick && onClick(e) | |
| onClick?.(e) |
🧰 Tools
🪛 Biome
[error] 107-107: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/packages/menu/menu.taro.tsx
Outdated
|
|
||
| const getScrollTop = (el: Element | Window) => { | ||
| return Math.max(0, 'scrollTop' in el ? el.scrollTop : el.pageYOffset) | ||
| return Math.max(0, 'scrollTop' in el ? el.scrollTop : el.scrollY) |
There was a problem hiding this comment.
修复 getScrollTop 函数的兼容性问题
在 getScrollTop 函数中：
return Math.max(0, 'scrollTop' in el ? el.scrollTop : el.scrollY)当 el 为元素时，el.scrollTop 存在，但 el.scrollY 不存在；当 el 为 window 时，el.scrollY 存在，但 'scrollTop' in el 判断会返回 false，导致获取 el.scrollY，这在某些浏览器会有兼容性问题。
建议修改为：
- return Math.max(0, 'scrollTop' in el ? el.scrollTop : el.scrollY)
+ return Math.max(0, el === window ? window.scrollY : (el as Element).scrollTop)这样可以准确区分 el 是否为 window，并获取正确的滚动位置，增强了函数的兼容性。
| const onScroll = useCallback(() => { | ||
| const scrollTop = getScrollTop(window) | ||
| const isFixed = | ||
| scrollTop > (typeof scrollFixed === 'boolean' ? 30 : Number(scrollFixed)) | ||
| setIsScrollFixed(isFixed) | ||
| } | ||
| }, [scrollFixed]) |
There was a problem hiding this comment.
确保scrollFixed属性的类型安全性。
在onScroll函数中，Number(scrollFixed)可能会返回NaN，如果scrollFixed不是一个可转换为数字的字符串。建议在使用Number(scrollFixed)之前，验证scrollFixed是否为有效数字，以避免潜在的问题。
建议在使用前添加类型检查和默认值，例如：
const isFixed =
- scrollTop > (typeof scrollFixed === 'boolean' ? 30 : Number(scrollFixed))
+ scrollTop > (typeof scrollFixed === 'boolean'
+ ? 30
+ : !isNaN(Number(scrollFixed))
+ ? Number(scrollFixed)
+ : 30)📝 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.
| const onScroll = useCallback(() => { | |
| const scrollTop = getScrollTop(window) | |
| const isFixed = | |
| scrollTop > (typeof scrollFixed === 'boolean' ? 30 : Number(scrollFixed)) | |
| setIsScrollFixed(isFixed) | |
| } | |
| }, [scrollFixed]) | |
| const onScroll = useCallback(() => { | |
| const scrollTop = getScrollTop(window) | |
| const isFixed = | |
| scrollTop > (typeof scrollFixed === 'boolean' | |
| ? 30 | |
| : !isNaN(Number(scrollFixed)) | |
| ? Number(scrollFixed) | |
| : 30) | |
| setIsScrollFixed(isFixed) | |
| }, [scrollFixed]) |
xiaoyatong
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
src/packages/menu/menu.taro.tsx (2)
62-63: 新增状态变量改进了状态管理添加
showMenuItem和menuItemTitle状态变量是一个很好的改进。这增强了组件管理菜单项可见性和标题的能力。建议：考虑使用对象而不是数组来存储这些状态，可能会使得按索引更新更加高效。例如：
const [showMenuItem, setShowMenuItem] = useState<Record<number, boolean>>({}) const [menuItemTitle, setMenuItemTitle] = useState<Record<number, string>>({})这样可以避免创建新数组，可能会在大型菜单中带来轻微的性能提升。
75-80: 使用 useCallback 优化了 onScroll 函数使用
useCallback包装onScroll函数是一个很好的性能优化。这可以防止不必要的重渲染，特别是当这个函数被传递给子组件时。建议：考虑将
scrollFixed添加到useCallback的依赖数组中，以确保当scrollFixed改变时，onScroll函数也会更新：const onScroll = useCallback(() => { // ... existing code ... }, [scrollFixed])这样可以确保
onScroll函数始终使用最新的scrollFixed值。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- src/packages/backtop/backtop.taro.tsx (3 hunks)
- src/packages/menu/menu.taro.tsx (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/packages/backtop/backtop.taro.tsx
🧰 Additional context used
🔇 Additional comments (5)
src/packages/menu/menu.taro.tsx (5)
1-7: 导入 useCallback 是一个很好的优化添加
useCallback到导入语句是一个很好的做法。这将有助于优化组件的性能，特别是在处理回调函数时。这个改变与PR的性能优化目标相符。
65-67: 简化的类名逻辑提高了可读性使用更简洁的语法来定义
cls变量是一个很好的改进。这种方式提高了代码的可读性和可维护性，符合代码简化的目标。
70-73: getScrollTop 函数的兼容性问题已解决非常好的修改。这个更新解决了之前评论中提到的兼容性问题。现在的实现正确区分了
window对象和普通元素，提高了函数的兼容性和可靠性。
82-87: useEffect 依赖项更新提高了代码的正确性将
onScroll添加到useEffect的依赖数组中是一个很好的改进。这确保了当onScroll函数发生变化时，事件监听器会被正确地更新。这种做法提高了代码的正确性和可靠性。
191-191: 简化的返回语句提高了一致性使用
cls变量作为className属性的值是一个很好的简化。这提高了代码的一致性，使其与earlier定义的cls保持一致。这种改变使代码更加清晰和易于维护。
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
src/packages/backtop/__test__/backtop.spec.tsx (2)
15-20: BackTop组件测试设置的优化这些更改很好地适应了新的容器结构，并通过添加特定的className使得在测试中更容易定位BackTop组件。
建议：考虑使用一个常量来存储"backtop-button"类名，以提高可维护性。例如：
const BACKTOP_CLASS = 'backtop-button'; // ... <BackTop target="target" className={BACKTOP_CLASS} onClick={handleClick} />这样可以在将来更容易地更新类名，并减少拼写错误的可能性。
22-23: 点击事件触发的改进这个更改与新的结构和BackTop组件的className添加保持一致，确保了点击事件在正确的元素上触发。
建议：为了提高代码的可读性和健壮性，可以考虑使用以下方式：
const backTopButton = container.querySelector('.backtop-button'); if (backTopButton) { fireEvent.click(backTopButton); } else { throw new Error('BackTop button not found'); }这样可以更明确地处理元素不存在的情况，并提供更有意义的错误信息。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- src/packages/backtop/test/backtop.spec.tsx (1 hunks)
🧰 Additional context used
🔇 Additional comments (1)
src/packages/backtop/__test__/backtop.spec.tsx (1)
11-14: 测试设置的改进这些更改通过创建可滚动的容器和更多的测试数据，显著改善了测试环境。这种方法为BackTop组件提供了更真实的使用场景，有助于更全面地测试其功能。
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
src/packages/menu/__test__/menu.spec.tsx (3)
37-46: 新测试用例有效验证多列布局这个新的测试用例很好地验证了 Menu.Item 支持多列布局的功能。这对于确保组件的灵活性很重要。
建议：考虑添加一个断言来验证实际渲染的列数，以确保
columns属性正确应用。
68-80: 测试用例全面性有所提高，但仍有改进空间这个测试用例的更新很好地覆盖了 Menu.Item 的新属性和功能。新增的 direction、icon 和样式类属性增强了组件的灵活性。
建议改进：
- 为每个新增的 prop 添加单独的断言，确保它们都被正确应用。
- 测试 disabled 属性的实际效果，例如尝试点击被禁用的选项并验证回调未被触发。
- 验证 icon 是否正确渲染。
Line range hint
1-100: 测试套件更新全面，反映了 Menu 组件的重大改进整体而言，这些更改很好地反映了 Menu 组件的重构。测试用例已经更新以涵盖新的嵌套结构（Menu.Item）和额外的属性。这些改动提高了组件的灵活性和功能性，同时保持了测试覆盖率。
建议：
- 考虑添加更多边缘情况的测试，例如空选项、极长文本等。
- 可以添加性能测试，特别是对于可能影响渲染性能的新功能（如多列布局）。
src/packages/menu/menu.tsx (1)
70-73: 建议为getScrollTop函数添加测试用例静态分析工具提示第70-73行未被测试覆盖。为了确保
getScrollTop函数在不同情况下的正确性，建议为其添加单元测试，特别是当el为window或其他元素时，验证返回的滚动值是否符合预期。您需要我协助编写这些测试用例吗？
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 70-73: src/packages/menu/menu.tsx#L70-L73
Added lines #L70 - L73 were not covered by tests
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
src/packages/menu/__test__/__snapshots__/menu.spec.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (2)
- src/packages/menu/test/menu.spec.tsx (2 hunks)
- src/packages/menu/menu.tsx (3 hunks)
🧰 Additional context used
🪛 GitHub Check: codecov/patch
src/packages/menu/menu.tsx
[warning] 70-73: src/packages/menu/menu.tsx#L70-L73
Added lines #L70 - L73 were not covered by tests
🔇 Additional comments (1)
src/packages/menu/__test__/menu.spec.tsx (1)
31-35: 更改符合组件重构这个更改从
MenuItem到Menu.Item符合组件的重构。这种嵌套结构可能会提高代码的可读性和组织性。
| })} | ||
| ref={menuRef} | ||
| > | ||
| <div {...rest} className={cls} ref={menuRef}> |
There was a problem hiding this comment.
合并rest中的className以避免覆盖
在第193行，<div>元素使用了{...rest}扩展属性并指定了className={cls}。如果rest中包含className属性，它将被cls覆盖。建议合并rest.className和cls，以确保所有传入的类名都被正确应用。
您可以按以下方式修改代码：
- <div {...rest} className={cls} ref={menuRef}>
+ <div {...rest} className={classNames(cls, rest.className)} ref={menuRef}>📝 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.
| <div {...rest} className={cls} ref={menuRef}> | |
| <div {...rest} className={classNames(cls, rest.className)} ref={menuRef}> |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (2)
src/packages/menuitem/menuitem.tsx (2)
92-103: 使用 useCallback 优化 getParentOffset 函数使用
useCallback来优化getParentOffset函数是一个很好的改进。这可以防止不必要的函数重新创建，从而提高性能。然而，建议考虑以下优化：
- 使用
requestAnimationFrame替代setTimeout，这可能会提供更平滑的视觉效果。- 考虑使用
ResizeObserver来监听父元素大小变化，这可能比在滚动事件中重复计算更有效。建议尝试以下优化：
const getParentOffset = useCallback(() => { requestAnimationFrame(() => { const p = parent.menuRef.current if (p) { const rect = p.getBoundingClientRect() setPosition({ height: rect.height, top: rect.top, }) } }) }, [parent.menuRef])
153-153: 优化滚动事件监听器将
getParentOffset添加到滚动事件监听器的useEffect依赖数组中是一个很好的改进。这确保了始终使用最新版本的getParentOffset函数。为了进一步优化性能，可以考虑以下建议：
- 使用节流（throttle）或防抖（debounce）技术来限制
getParentOffset的调用频率，特别是在快速滚动时。- 考虑使用
IntersectionObserverAPI 来检测可见性变化，这可能比持续监听滚动事件更高效。考虑使用节流函数来优化滚动事件处理：
import { throttle } from 'lodash-es'; // 在组件顶部定义节流函数 const throttledGetParentOffset = useMemo( () => throttle(getParentOffset, 100), [getParentOffset] ); // 在 useEffect 中使用节流函数 useEffect(() => { if (!parent.lockScroll) { scrollParent?.addEventListener('scroll', throttledGetParentOffset, false); return () => { scrollParent?.removeEventListener('scroll', throttledGetParentOffset, false); throttledGetParentOffset.cancel(); // 清理节流函数 }; } }, [parent.lockScroll, scrollParent, throttledGetParentOffset]);这将限制
getParentOffset的调用频率，提高滚动性能。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
src/packages/menu/__test__/__snapshots__/menu.spec.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (3)
- src/packages/menu/test/menu.spec.tsx (2 hunks)
- src/packages/menuitem/menuitem.taro.tsx (3 hunks)
- src/packages/menuitem/menuitem.tsx (3 hunks)
🧰 Additional context used
🔇 Additional comments (6)
src/packages/menuitem/menuitem.tsx (2)
4-4: 导入 useCallback 以优化性能引入
useCallback钩子是一个很好的做法，它可以帮助优化组件性能，特别是在处理复杂逻辑或频繁重渲染的情况下。这符合PR的性能优化目标。
106-107: 更新 useEffect 依赖数组将
getParentOffset添加到useEffect的依赖数组中是一个重要的改进。这确保了当getParentOffset函数引用发生变化时，效果会重新运行。这种做法符合 React 的 hooks 规则，有助于防止由于闭包陈旧而可能导致的 bug。src/packages/menu/__test__/menu.spec.tsx (1)
99-99:⚠️ Potential issue应使用正确的 Jest 匹配器 'toHaveBeenCalledWith'。
在第99行，使用了 Jest 的匹配器
toBeCalledWith。但是，正确的匹配器名称是toHaveBeenCalledWith。请将其修改以确保测试准确性。修改如下：
- expect(testClick).toBeCalledWith({ text: '新款商品', value: 1 }) + expect(testClick).toHaveBeenCalledWith({ text: '新款商品', value: 1 })Likely invalid or redundant comment.
src/packages/menuitem/menuitem.taro.tsx (3)
93-104: 确认getParentOffset函数的实现
getParentOffset函数使用useCallback定义，正确地获取父组件的尺寸和位置，并更新组件状态。实现合理，符合预期。
107-107: 验证useEffect钩子的依赖项
useEffect钩子依赖于showPopup和getParentOffset，确保在showPopup状态或getParentOffset函数变化时调用。依赖项设置正确。
152-152: 位置状态的初始化正确
position状态被初始化为{ top: 0, height: 0 }，为后续位置计算提供了正确的初始值。
| <Menu.Item /> | ||
| <Menu.Item title="筛选" ref={itemRef}> | ||
| <div | ||
| className="test-menu-item" | ||
| onClick={() => { | ||
| ;(itemRef.current as any)?.toggle(false) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
建议为 'Menu.Item' 的 ref 提供正确的类型，以避免使用 'any' 类型断言。
在第78行，代码中使用了 (itemRef.current as any)?.toggle(false)，通过将 itemRef.current 断言为 any 类型来调用 toggle 方法。这可能会隐藏类型检查中的潜在问题。建议在 Menu.Item 组件中正确定义 ref 的类型，以便在不使用 any 断言的情况下访问 toggle 方法，从而提高代码的类型安全性和可维护性。
| } | ||
| mockGetBoundingClientRect({ | ||
| width: 300, | ||
| height: 100, | ||
| left: 0, | ||
| top: 0, | ||
| right: 300, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
重复的 getBoundingClientRect 模拟可能不必要。
您在第87-93行再次调用了 mockGetBoundingClientRect 方法，但之前在 beforeAll 钩子中已经进行了全局模拟。如果模拟的参数未发生变化，可能无需再次调用。建议检查是否可以移除此处的重复模拟，或者仅在需要不同参数时在特定测试中重新模拟。
| </Menu.Item> | ||
| </Menu> | ||
| ) | ||
| } | ||
| mockGetBoundingClientRect({ | ||
| width: 300, | ||
| height: 100, | ||
| left: 0, | ||
| top: 0, | ||
| right: 300, | ||
| }) |
There was a problem hiding this comment.
将 'testClick' 的定义提前，以确保作用域正确。
在第71行的 onChange={testClick} 中使用了 testClick，但 testClick 的定义在第94行。这会导致作用域问题，因为在定义之前无法访问 testClick。建议将 testClick 的定义移动到 App 组件内部，或者在 App 定义之前。
您可以这样修改：
+ const testClick = vi.fn((val) => undefined)
const App = () => {
// 原有代码
}
- const testClick = vi.fn((val) => undefined)Committable suggestion was skipped due to low confidence.
| }, [direction, windowHeight, parent.lockScroll, parent.menuRef]) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
依赖项数组中包含未使用的变量
updateItemOffset 函数的依赖项数组包含了 direction 和 windowHeight，但在函数内部并未使用这些变量。建议移除未使用的依赖项，以避免不必要的重新渲染。
建议修改如下：
- }, [direction, windowHeight, parent.lockScroll, parent.menuRef])
+ }, [parent.lockScroll, parent.menuRef])📝 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.
| }, [direction, windowHeight, parent.lockScroll, parent.menuRef]) | |
| }, [parent.lockScroll, parent.menuRef]) |
🤔 这个变动的性质是？
🔗 相关 Issue
💡 需求背景和解决方案
☑️ 请求合并前的自查清单
Summary by CodeRabbit
新功能
BackTop和Menu组件的状态管理和性能。Menu组件中添加了新的状态变量以跟踪菜单项的可见性和标题。改进
useCallback钩子以提高性能。goTop函数和渲染逻辑。getScrollTop函数以使用scrollY。BackTop和Menu组件的测试用例以增强结构和全面性。