Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for SHIFT + Scroll #366

Closed
wants to merge 1 commit into from
Closed

Conversation

DjSt3rios
Copy link

@DjSt3rios DjSt3rios commented Jul 10, 2021

Description

Many people use Shift + scroll but it doesn't work with this custom scrolling bar. These changes will allow to listen for keys (keydown and keyup events) and scroll horizontally while the shift button is held down.
This however comes with a downside: If a use is holding shift and click on another window, the browser will still think shift is held down (because keyup is never triggered), and it will horizontally scroll even when shift is not held down. I don't think this will often happen, and all the user will have to do is hold shift again and release it, to reset it.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

@idiotWu
Copy link
Owner

idiotWu commented Jul 11, 2021

Does shift + wheel always swap the scrolling direction? I don't have a mouse to test this, but the direction doesn't seem to change when scrolling with the trackpad.

This however comes with a downside: If a use is holding shift and click on another window, the browser will still think shift is held down (because keyup is never triggered), and it will horizontally scroll even when shift is not held down.

This can be fixed by attaching blur event listener to the window object:

// better to use the scoped event handlers here 
// as they can be removed automatically when the scrollbar is destroyed
addEvent(window, 'blur keyup', () => {
  // do something...
})

@idiotWu
Copy link
Owner

idiotWu commented Jul 11, 2021

It appears we already had some discussion here: #63

If shift + wheel does not always trigger horizontal scrolling (when it does, we can receive a deltaX instead of a deltaY, see: #63 (comment)), I think this feature should not be added to the core codebase which tries to follow the native behaviors. Instead, I'd suggest implementing this feature as a third-party plugin.

@DjSt3rios
Copy link
Author

I see! I personally did it for a personal project so I decided to share, since I saw some people asking for it. It's not perfect by any means, but might do for some people. The blur event is indeed a good idea, but I suspect if a user starts holding shift before clicking somewhere in the page, it won't get picked up, so it's kinda one way or another, but it does sound better with the blur event. Well I guess you can think about it, if not just close it, no worries at all , thank you for your work by the way 😊

@idiotWu
Copy link
Owner

idiotWu commented Jul 11, 2021

Actually, I'm thinking about maintaining a list of widely-used plugins like the ModalPlugin, AnchorPlugin, HorizontalScrollPlugin, etc. It would be great if you could create a plugin for this feature and share it in the discussion 🤗 !

@DjSt3rios
Copy link
Author

Actually, I'm thinking about maintaining a list of widely-used plugins like the ModalPlugin, AnchorPlugin, HorizontalScrollPlugin, etc. It would be great if you could create a plugin for this feature and share it in the discussion 🤗 !

I'd love to but I never made plugins in TypeScript/Node. Do you happen to have a tutorial/guide? I am relatively new to Typescript and Node libraries etc 😇

@idiotWu
Copy link
Owner

idiotWu commented Jul 11, 2021

You can find the documentation here. TypeScript/Node is not required, you can use the vanilla JavaScript and — if I'm right — implement something like the following:

class YourPlugin extends ScrollbarPlugin {
  static pluginName = 'yourPlugin'; // <= don't actually like this but you have to do it now 😞 

  transformDelta(delta, fromEvent) {
    // we can use the `evt.shiftKey` property to tell if the shift key is pressed
    if (/wheel/.test(fromEvent.type) && fromEvent.shiftKey) { 
        return {
          x: delta.y,
          y: delta.x,
        };
    }

    return delta;
  }
}

@DjSt3rios
Copy link
Author

Hello my friend, sorry for the delay, just finished it up! If you want take look at Discussion and see if it needs any editing or something 😊

@idiotWu
Copy link
Owner

idiotWu commented Jul 18, 2021

I think we can close this PR now. Many thanks!

@idiotWu idiotWu closed this Jul 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Widely-used Plugins
Development

Successfully merging this pull request may close these issues.

2 participants