Skip to content

Add mouse scroll event#20

Open
MetaMmodern wants to merge 2 commits into
paulkr:masterfrom
MetaMmodern:master
Open

Add mouse scroll event#20
MetaMmodern wants to merge 2 commits into
paulkr:masterfrom
MetaMmodern:master

Conversation

@MetaMmodern

@MetaMmodern MetaMmodern commented Oct 17, 2020

Copy link
Copy Markdown

Hi, I made some improvements to the library:

  • remove unnecessary comment on the top of file.
  • add more magnification when user scrolls in lens.

These changes were needed in my personal project, so I made that 2 years ago and forgot to propose a PR.

Further improvements may be:

  • write a pure js implementation
  • write a react/vue/angular/svelte library

I didn't know which tools did you use to minify this code, so the minified version is not changed. Please, minify it before merging)

Please, add a "hacktoberfest-accepted" label to this PR, thank you!

@rfuehrer

rfuehrer commented Dec 5, 2021

Copy link
Copy Markdown

Hi @MetaMmodern ,

it's a pity that you removed the reference to the author in a PR at the upstream itself. I personally find that disrespectful to the original author @paulkr . Especially since we all have an advantage from this good preliminary work.

I think your idea and impulse are excellent, but found that scrolling with a logitech mouse activated screen scrolling. This is because the event mousewheel in Firefox is already deprecated. It is to use wheel instead (source: https://developer.mozilla.org/en-US/docs/Web/API/Element/mousewheel_event and https://developer.mozilla.org/en-US/docs/Web/API/Element/wheel_event).

I took the freedom to update your code locally, as the DOM events will be triggered via "wheel" under Firefox in 2021. Also, as a precaution, the default is disabled while the event is triggered. Tested with Safari and Firefox.

Maybe you are interested in an update of your mentioned project and this PR?

})
$element.bind('wheel', function (e) {
e.preventDefault();
if (e.originalEvent.wheelDelta / 120 > 0) {

@MetaMmodern

Copy link
Copy Markdown
Author

Hi @MetaMmodern ,

it's a pity that you removed the reference to the author in a PR at the upstream itself. I personally find that disrespectful to the original author @paulkr . Especially since we all have an advantage from this good preliminary work.

I think your idea and impulse are excellent, but found that scrolling with a logitech mouse activated screen scrolling. This is because the event mousewheel in Firefox is already deprecated. It is to use wheel instead (source: https://developer.mozilla.org/en-US/docs/Web/API/Element/mousewheel_event and https://developer.mozilla.org/en-US/docs/Web/API/Element/wheel_event).

I took the freedom to update your code locally, as the DOM events will be triggered via "wheel" under Firefox in 2021. Also, as a precaution, the default is disabled while the event is triggered. Tested with Safari and Firefox.

Maybe you are interested in an update of your mentioned project and this PR?

})
$element.bind('wheel', function (e) {
e.preventDefault();
if (e.originalEvent.wheelDelta / 120 > 0) {

Lol, dude, this repo is dead.

@rfuehrer

rfuehrer commented Dec 5, 2021

Copy link
Copy Markdown

...ok, I agree with you, this is not great handled by @paulkr. But for that we can set up forks ;)

At least @paulkr iis still active and that gives me hope that reasonable PRs are accepted or at least by activity @paulkr marks the repo as deprecated. Also @MetaMmodern your PR was worth a lot and therefore thank you both. No matter if the project looks dead or not. Maybe my comment will help others also :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants