feat: tabbar support mouse wheel vertical #5129
Conversation
|
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughThe pull request introduces a new 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: 0
🧹 Outside diff range and nitpick comments (8)
packages/@core/ui-kit/tabs-ui/src/types.ts (2)
46-54: Consider adding English documentationThe
styleTypeproperty documentation only includes Chinese comments. Consider adding English documentation to maintain consistency with international developers./** + * @en_US Tab style type * @zh_CN 标签页风格 */ styleType?: TabsStyleType;
56-59: Consider adding English documentationThe
wheelableproperty documentation only includes Chinese comments. Consider adding English documentation to improve accessibility for international developers./** + * @en_US Whether to respond to mouse wheel events * @zh_CN 是否响应滚轮事件 */ wheelable?: boolean;packages/effects/layouts/src/widgets/preferences/blocks/switch-item.vue (1)
36-46: Consider enhancing type safety for tooltip contentThe tooltip implementation handles multi-line content well, but could benefit from additional type safety.
Consider creating a computed property to handle the tip splitting:
<script setup lang="ts"> + const tooltipLines = computed(() => tip.split('\n')); </script> <template> ... <slot name="tip"> <template v-if="tip"> - <p v-for="(line, index) in tip.split('\n')" :key="index"> + <p v-for="(line, index) in tooltipLines" :key="index"> {{ line }} </p> </template> </slot> ... </template>packages/effects/layouts/src/widgets/preferences/blocks/layout/tabbar.vue (1)
21-21: LGTM! Consider grouping related preferences together.The implementation of the wheelable preference is well-structured and follows the established patterns. The switch item is properly integrated with localization and conditional disabling.
Consider grouping this preference with other scrolling/navigation-related preferences for better organization.
Consider moving this switch item next to the
tabbarDraggableswitch since both relate to tab navigation/interaction.Also applies to: 57-63
packages/@core/ui-kit/tabs-ui/src/tabs-view.vue (1)
39-45: Consider improving wheel event handling.A few suggestions to enhance the wheel event handler:
- Consider debouncing the handler for smoother scrolling
- Add browser compatibility checks
- Consider adding ARIA attributes to indicate scrollable region
Here's a suggested implementation:
+import { debounce } from 'lodash-es'; + +const debouncedHandleWheel = debounce((e: WheelEvent) => { + handleWheel(e); +}, 16); // ~60fps + function onWheel(e: WheelEvent) { if (props.wheelable) { - handleWheel(e); + debouncedHandleWheel(e); e.stopPropagation(); e.preventDefault(); } }Also, add ARIA attributes to the scrollable region:
<VbenScrollbar ref="scrollbarRef" :shadow-bottom="false" :shadow-top="false" class="h-full" horizontal scroll-bar-class="z-10 hidden " shadow shadow-left shadow-right + role="region" + aria-label="Scrollable tabs" @scroll-at="handleScrollAt" @wheel="onWheel" >packages/@core/ui-kit/tabs-ui/src/use-tabs-view-scroll.ts (1)
145-150: Consider enhancing the wheel scroll implementation.While the basic implementation works, there are several improvements that could make it more robust:
- The scroll multiplier (3) should be configurable to allow customization of scroll sensitivity
- Consider handling horizontal scroll events (deltaX) for trackpad users
- Add boundary checks to prevent unnecessary scroll attempts
- Consider preventing default scroll behavior to avoid conflicts
Here's a suggested enhancement:
- function handleWheel({ deltaY }: WheelEvent) { + function handleWheel(event: WheelEvent) { + const { deltaY, deltaX } = event; + const scrollMultiplier = 3; // Consider making this configurable + + if (!scrollViewportEl.value) return; + + // Prevent default only if scroll would be effective + const newScrollLeft = scrollViewportEl.value.scrollLeft + (deltaY + deltaX) * scrollMultiplier; + if (newScrollLeft >= 0 && + newScrollLeft <= scrollViewportEl.value.scrollWidth - scrollViewportEl.value.clientWidth) { + event.preventDefault(); + + scrollViewportEl.value.scrollBy({ + behavior: 'smooth', + left: (deltaY + deltaX) * scrollMultiplier, + }); + } }packages/locales/src/langs/en-US/preferences.json (1)
58-59: Enhance English translations for consistency with Chinese version.The English translations could be more detailed to match the information provided in the Chinese version:
- "wheelable": "Support Mouse Wheel", - "wheelableTip": "When enabled, the Tabbar area responds to vertical scrolling events of the scroll wheel.", + "wheelable": "Enable Vertical Wheel Scrolling", + "wheelableTip": "When enabled, the tabbar area responds to vertical scroll wheel events. When disabled, only system horizontal scroll events (requiring Shift + wheel) are supported."packages/@core/preferences/src/types.ts (1)
176-177: Maintain documentation consistency by translating the comment to English.The JSDoc comment should be in English to maintain consistency with other comments in the file.
- /** 是否开启鼠标滚轮响应 */ + /** Whether to enable mouse wheel scrolling */ wheelable: boolean;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
packages/@core/preferences/src/config.ts(1 hunks)packages/@core/preferences/src/types.ts(1 hunks)packages/@core/ui-kit/tabs-ui/src/tabs-view.vue(3 hunks)packages/@core/ui-kit/tabs-ui/src/types.ts(1 hunks)packages/@core/ui-kit/tabs-ui/src/use-tabs-view-scroll.ts(2 hunks)packages/effects/layouts/src/basic/tabbar/tabbar.vue(1 hunks)packages/effects/layouts/src/widgets/preferences/blocks/layout/tabbar.vue(2 hunks)packages/effects/layouts/src/widgets/preferences/blocks/switch-item.vue(2 hunks)packages/effects/layouts/src/widgets/preferences/preferences-drawer.vue(2 hunks)packages/locales/src/langs/en-US/preferences.json(1 hunks)packages/locales/src/langs/zh-CN/preferences.json(1 hunks)
🔇 Additional comments (9)
packages/@core/ui-kit/tabs-ui/src/types.ts (1)
46-59: LGTM! Type definitions are well-structured
The new properties are properly typed and follow the existing interface pattern.
packages/effects/layouts/src/widgets/preferences/blocks/switch-item.vue (1)
11-13: LGTM! Props are well-defined
The new tip prop is properly typed with a sensible default value.
packages/effects/layouts/src/basic/tabbar/tabbar.vue (1)
58-58: LGTM! Clean implementation of the wheelable property
The wheelable property is properly bound to the preferences system and follows the component's existing pattern.
packages/@core/preferences/src/config.ts (1)
86-86: LGTM! Default configuration looks good.
The wheelable property is correctly placed in the tabbar section with a sensible default value of true.
packages/@core/ui-kit/tabs-ui/src/tabs-view.vue (2)
22-22: LGTM! Default prop value aligns with preferences.
The default value of true for the wheelable prop matches the default preferences configuration.
82-82: Verify wheel event behavior across browsers.
The wheel event behavior can vary across different browsers. We should verify that the scrolling works consistently.
✅ Verification successful
Let me analyze the wheel event implementation further by checking for any browser-specific handling or test cases.
Let me check the wheel event implementation more thoroughly with a corrected command.
Wheel event implementation is safe and well-implemented
The wheel event implementation is properly handled with several safety measures in place:
- Uses standard
WheelEventinterface which is well-supported across modern browsers - Implements smooth scrolling behavior using native
scrollByAPI - Has proper cleanup with ResizeObserver and MutationObserver
- The scrolling is controlled by a configurable
wheelableprop - Includes event prevention to avoid conflicting scroll behaviors
- Uses a reasonable scroll multiplier (3x) for smooth horizontal scrolling
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for browser compatibility checks or normalizations
rg -g '!{dist,node_modules}/*' -A 5 'WheelEvent|wheel.*event'
Length of output: 1519
Script:
#!/bin/bash
# Search for any browser-specific handling in the tabs implementation
rg -g '!{dist,node_modules}/*' -A 5 'browser|navigator|userAgent' packages/@core/ui-kit/tabs-ui/
# Look for test files related to wheel events
fd -g '*test*' -g '*spec*' packages/@core/ui-kit/tabs-ui/ --exec rg -l 'wheel|WheelEvent'
Length of output: 243
Script:
#!/bin/bash
# Look at the complete implementation of use-tabs-view-scroll.ts
cat packages/@core/ui-kit/tabs-ui/src/use-tabs-view-scroll.ts
# Search for test files with corrected syntax
fd -e spec.ts -e test.ts . packages/@core/ui-kit/tabs-ui/ --exec rg -l 'wheel|WheelEvent' {}
Length of output: 4873
packages/locales/src/langs/zh-CN/preferences.json (1)
58-59: LGTM! Clear and informative localization entries.
The Chinese translations for the new wheelable feature are clear, accurate, and provide good context about the functionality.
packages/@core/ui-kit/tabs-ui/src/use-tabs-view-scroll.ts (1)
194-194: Verify the handleWheel export.
The handleWheel method is now exposed through the hook's return object. Ensure that components using this hook properly handle wheel events and respect the wheelable preference.
✅ Verification successful
The handleWheel implementation is properly used and controlled by wheelable prop
The verification shows that:
handleWheelis correctly implemented inuse-tabs-view-scroll.tsand exposed through the hook's return object- It's properly consumed in
tabs-view.vuewhere it's only called when thewheelableprop is true - The wheel event is properly handled with stopPropagation and preventDefault
- The implementation includes smooth scrolling behavior with appropriate deltaY multiplication
The code demonstrates proper handling of wheel events and respect for the wheelable preference as originally intended.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for components using handleWheel
ast-grep --pattern 'handleWheel($$$)'
Length of output: 39
Script:
#!/bin/bash
# Search for usages of handleWheel with ripgrep for better context
rg "handleWheel" -A 3
# Also search for any imports or destructuring of handleWheel
rg "handleWheel.*}" -A 3
# Search for use-tabs-view-scroll hook usage
rg "useTabsViewScroll" -A 3
Length of output: 2654
packages/effects/layouts/src/widgets/preferences/preferences-drawer.vue (1)
108-108: LGTM! Well-integrated changes.
The new tabbarWheelable model is correctly defined and integrated, maintaining consistency with the existing codebase patterns for both the model declaration and its binding in the template.
Also applies to: 349-349
Description
默认情况下，标签栏是横向滚动的，只能支持系统的横向滚动事件，因此只能按下Shift键才能滚动标签栏。
现在添加了一个开关，可以响应系统的纵向滚动事件。开启后，无需按下Shift键直接可以使用滚轮滚动标签栏。
close #4474
Type of change
Please delete options that are not relevant.
pnpm-lock.yamlunless you introduce a new test example.Checklist
pnpm run docs:devcommand.pnpm test.feat:,fix:,perf:,docs:, orchore:.Summary by CodeRabbit
New Features
wheelableproperty for the tab bar, enabling mouse wheel interactions for navigation.wheelablefeature.Documentation
wheelablefeature in English and Chinese.Bug Fixes