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.

Sign up for GitHub

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

Jump to bottom

Fixed natural scrolling bug #9

Merged
merged 4 commits into from
Apr 1, 2024
Merged

Fixed natural scrolling bug #9

merged 4 commits into from
Apr 1, 2024

Conversation

salihefee
Copy link

@salihefee salihefee commented Apr 1, 2024 •
edited
Loading

Used the Gio.Settings API to query the natural-scroll option and inverted scrolling accordingly.

francislavoie
francislavoie reviewed Apr 1, 2024
View reviewed changes
Copy link
Owner

@francislavoie francislavoie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a comment; also did you build and test this locally? Did you try without natural turned on as well?

[email protected]/extension.js Outdated
case Clutter.ScrollDirection.UP:
volume += this._get_step();
volume += naturalScroll ? -this._get_step() : this._get_step();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think maybe pull this ternary out to store a ? -1 : 1 multiplier before the switch?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@salihefee
Copy link
Author

Just a comment; also did you build and test this locally? Did you try without natural turned on as well?

Yes, it works with natural scrolling on and off.

francislavoie
francislavoie reviewed Apr 1, 2024
View reviewed changes
Copy link
Owner

@francislavoie francislavoie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more thing

[email protected]/extension.js Outdated Show resolved Hide resolved
francislavoie
francislavoie reviewed Apr 1, 2024
View reviewed changes
Copy link
Owner

@francislavoie francislavoie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh and...

[email protected]/extension.js Outdated Show resolved Hide resolved
@francislavoie francislavoie merged commit c9e28b0 into francislavoie:main Apr 1, 2024
@francislavoie francislavoie mentioned this pull request Apr 1, 2024
Closed
@francislavoie
Copy link
Owner

Oh - I guess this assumes all scroll events come from the trackpad? What if a user uses a mouse instead but has natural enabled? It might flip how the mouse behaves I think.

@salihefee
Copy link
Author

Oh - I guess this assumes all scroll events come from the trackpad? What if a user uses a mouse instead but has natural enabled? It might flip how the mouse behaves I think.

Oh my god... I don't know how I didn't realise that. I am discarding this and I will create another PR with an "Invert Direction" settings added. I am so sorry.

@francislavoie
Copy link
Owner

I think it might be possible to check the source of the scroll event though. You could experiment with that.

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

@francislavoie francislavoie francislavoie approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@salihefee @francislavoie