fix(handleResize)：修复响应式情况下 addEventListener 事件错误，导致菜单异常关闭#662
Conversation
📝 WalkthroughWalkthrough本次变更在 Changes
Estimated code review effort2 (~12 分钟) Suggested labels
Poem
✨ Finishing Touches
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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
Documentation and Community
|
dosubot
Bot
added
size:M
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
web/src/utils/handleResize.ts (2)
28-30: 考虑更可靠的移动设备检测方法当前的用户代理检测方法可以满足基本需求，但用户代理字符串可能被修改或伪造。考虑以下更可靠的替代方案：
function isMobileDevice() { - return /Android|iPhone|iPad|iPod|Mobile/i.test(navigator.userAgent) + // 结合多种检测方法提高准确性 + const isTouchDevice = 'ontouchstart' in window || navigator.maxTouchPoints > 0 + const isUserAgentMobile = /Android|iPhone|iPad|iPod|Mobile/i.test(navigator.userAgent) + return isTouchDevice && isUserAgentMobile }
35-53: 事件监听器管理逻辑正确，但需注意响应式引用的使用这个实现很好地解决了多重事件监听器绑定的问题：
- 正确地移除之前的监听器防止重复绑定
- 仅在移动设备且宽度小于1024px时添加监听器
- 使用WeakMap进行内存管理
不过需要注意一个潜在问题：
isOutside.value在监听器函数中的使用是正确的，因为它在运行时访问响应式引用的当前值。建议添加一些防御性检查：
if (isMobileDevice() && window.innerWidth < 1024) { const listenerEvent = () => { + if (!targetEl || !settingStore) return if (settingStore.getMobileSubmenuState() && isOutside.value) { settingStore.setMobileSubmenuState(false) } } document.addEventListener('mousemove', listenerEvent) listenerMap.set(targetEl, listenerEvent) }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
web/src/utils/handleResize.ts(2 hunks)
🔇 Additional comments (1)
web/src/utils/handleResize.ts (1)
13-13: 优秀的设计选择！使用 WeakMap 来管理事件监听器是一个很好的解决方案，它可以防止内存泄漏并确保每个元素只有一个监听器实例。WeakMap 会在元素被垃圾回收时自动清理相关的监听器引用。
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an issue with responsive event listener management in the mobile menu system. The problem was that addEventListener events were not properly managed, causing menus to close unexpectedly when the window was resized on non-mobile devices.
- Introduces a
listenerMapWeakMap to cache event listeners and prevent duplicate bindings - Adds proper listener cleanup logic to remove existing listeners before adding new ones
- Separates mobile device detection logic and improves the conditions for when listeners should be active
| import { useMouseInElement, useResizeObserver } from '@vueuse/core' | ||
| import type { Ref } from 'vue' | ||
|
|
||
| const listenerMap = new WeakMap<HTMLElement, (e: MouseEvent) => void>() |
There was a problem hiding this comment.
The global WeakMap could lead to memory leaks if not properly cleaned up. Consider moving this inside the function scope or ensuring proper cleanup when components are destroyed.
|
晚点我跑一下看看 |
| import { useMouseInElement, useResizeObserver } from '@vueuse/core' | ||
| import type { Ref } from 'vue' | ||
|
|
||
| const listenerMap = new WeakMap<HTMLElement, (e: MouseEvent) => void>() |
问题描述
handleResize.ts中未正确管理事件监听器，导致在窗口尺寸变化非移动端的情况下造成菜单无法正常显示。修改说明
listenerEvent缓存变量，防止多次绑定事件；影响范围
/web/src/utils/handleResize.tsredpandacompress_.2025-07-23.124122.mp4
Summary by CodeRabbit