Skip to content
This repository was archived by the owner on Jun 24, 2025. It is now read-only.

Conversation

@rom1dep
Copy link
Contributor

@rom1dep rom1dep commented Jun 1, 2025

…ontal/vertical scrolling (esp. noteworthy with touchpads)

…ontal/vertical scrolling (esp. noteworthy with touchpads)
@eliandoran eliandoran added this to the v0.95.0 milestone Jun 1, 2025
@SiriusXT
Copy link
Member

SiriusXT commented Jun 2, 2025

  1. Hello! If the goal is to keep the scrolling speed consistent between horizontal and vertical scrolling on a touchpad, I would suggest removing && event.deltaX === 0 from:
if (!event.shiftKey && event.deltaX === 0)

and incorporating event.deltaX handling into:

deltaX += Math.sign(event.deltaY) * Math.max(Math.min(Math.abs(event.deltaY), TAB_CONTAINER_MIN_WIDTH * 3), TAB_CONTAINER_MIN_WIDTH);

The reason is that your implementation lacks a smooth scrolling transition. As a result, when users scroll, a tab can instantly jump to a different position, which feels quite abrupt. For example, if I keep my eyes on a specific tab and scroll, it's difficult to tell how far the tab has moved, and visually tracking it becomes nearly impossible.

  1. Moreover, without any boundary constraints, scrolling can become even more aggressive if the system’s scroll sensitivity (e.g., lines per scroll) is set to a high value.
    The lower bound should be removed to accommodate the smaller scroll distance of touchpads.

  2. I also noticed that VS Code adopts a similar "instant scroll" behavior. However, it limits the scroll distance per step to a very small, fixed value (which cannot be changed via system settings). As a result, it usually takes 3–5 scroll steps to fully move a tab out of view, making the overall experience feel much smoother. This is also a viable approach worth considering. And additionally, VS Code retains the scroll bar to allow users to scroll quickly when needed.

  3. Lastly, I noticed that the current implementation intercepts Shift + Wheel, making it behave exactly the same as a regular wheel scroll. In that case, would it be possible to preserve the functionality of Shift + Wheel for switching between tabs? This could be more user-friendly, especially for users accustomed to that shortcut.

@SiriusXT
Copy link
Member

SiriusXT commented Jun 2, 2025

I found a method to check whether smooth scrolling is enabled on the user's computer:

function quickSmoothScrollCheck() {
    const cssSmooth = getComputedStyle(document.documentElement).scrollBehavior === 'smooth' ||
                     getComputedStyle(document.body).scrollBehavior === 'smooth';
    
    const reducedMotion = window.matchMedia('(prefers-reduced-motion: reduce)').matches;
    
    const browserSupport = 'scrollBehavior' in document.documentElement.style;
    
    return cssSmooth && browserSupport && !reducedMotion;
}

Based on this, if smooth scrolling is enabled, we can apply the original decelerated scrolling behavior. Otherwise, we can complete the scroll instantly.

Additionally, I’ll make sure to include horizontal scrolling to maintain consistency.

Please let me know if you'd like me to implement this.

@FliegendeWurst
Copy link
Member

I found a method to check whether smooth scrolling is enabled on the user's computer:

Interesting, how does that work in Electron? Would we need to add a setting for that? (related: TriliumNext/Trilium#5413)

@SiriusXT
Copy link
Member

SiriusXT commented Jun 3, 2025

Interesting, how does that work in Electron? Would we need to add a setting for that? (related: TriliumNext/Trilium#5413)

This is supported in Electron. It automatically enables or disables animations based on the system preference.

On Windows, animations can be disabled in the advanced system settings, but Linux might not offer such an option. Therefore, if you want to support this feature, you could consider adding an option for it.

The option could have three settings: auto, off, and on.

@FliegendeWurst
Copy link
Member

but Linux might not offer such an option. Therefore, if you want to support this feature, you could consider adding an option for it.

Just had a look, there is a setting for it in Gnome Tweaks. So I don't think we need to add an option, but I guess we should check for prefers-reduced-motion in more places.

@dosubot dosubot bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Jun 4, 2025
@eliandoran
Copy link
Contributor

@rom1dep , @SiriusXT , what’s the state of this PR?
Are there changes to be made, according to the discussion?

@eliandoran eliandoran marked this pull request as draft June 4, 2025 19:56
@eliandoran eliandoran modified the milestones: v0.94.1, v1.0.0, v0.95.0 Jun 4, 2025
@eliandoran
Copy link
Contributor

Closing in favor of #2177.

@eliandoran eliandoran closed this Jun 7, 2025
@rom1dep
Copy link
Contributor Author

rom1dep commented Jun 7, 2025

Closing in favor of #2177.

please could we not? #2177 causes more problem than it solves for me. As a minimum:

@SiriusXT
Copy link
Member

SiriusXT commented Jun 7, 2025

doesn't incur unnecessary and inconsistent redraws during scrolling events (when the mouse pointer moves over a new tab, causing its border and close button to be focused), neither in the horizontal nor vertical directions.

event.stopImmediatePropagation(); does not block hover events. To prevent hover effects during scrolling, hover handling needs to be disabled during the scroll. This mechanism can be added.

doesn't intercept and override scroll deltas (which could cause unforeseen side effects).

On the computer, only the vertical scroll amount can be set. Setting a large vertical scroll delta may cause the tabs to move too far. Limiting the scroll movement to a maximum width equivalent to 2 tabs seems reasonable — for example, VSCode uses even less, about one-third of a tab’s width, and this cannot be changed. Of course, removing this limit is also feasible; the choice depends on the trade-offs to be made.

@SiriusXT SiriusXT mentioned this pull request Jun 9, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

size:S This PR changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants