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

Added scroll controlling to mini player #2191

Closed
wants to merge 7 commits into from
Closed

Added scroll controlling to mini player #2191

wants to merge 7 commits into from

Conversation

soya-daizu
Copy link
Contributor

@soya-daizu soya-daizu commented Jan 4, 2019


Description:

I did it because I personally just wanted that.
Implementation is somewhat lazy and its nearly just a copy-paste from MainWindowController.
However instead of having the dedicated OSD, this scroll controlling on mini player indicates the volume change by showing volume popover while moving trackpad/mouse.

@anohren
Copy link
Contributor

anohren commented Jan 4, 2019

You don't explicitly say what is being controlled by scrolling, but I assume it's exclusively the volume. Edit: Right, now I saw the issue reference.

That's exactly what I was missing as well. Tried scrolling while hovering over the window, the album art, the volume icon, the volume popover, the volume slider -- nothing :)

LEoREo2247 added 5 commits January 5, 2019 17:16
…me popover to appear

It also includes minor readability improvement
Giving some delay until popover gets disappeared for the trackpad
Making popover disappears AFTER user stops scrolling with the mouse
Copy link
Contributor

@anohren anohren left a comment

Choose a reason for hiding this comment

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

I tested this and it seems to work pretty well. The only problem I noticed was when using a macbook trackpad to scroll, and momentarily lifting my fingers to continue the scroll in a different position on the trackpad. In those cases the animation to hide the volume popover was not cancelled, but allowed to continue even though I had already initiated a new scrolling action.

Using an external logitech mouse with discrete scrolling steps did not show the same problem.

@anohren
Copy link
Contributor

anohren commented Jan 10, 2019

From testing now I think it works really well. Scrolling reacts everywhere it should.

@uiryuu uiryuu force-pushed the develop branch 2 times, most recently from 620dce1 to f4a83ec Compare February 9, 2019 23:08
@uiryuu
Copy link
Member

uiryuu commented Feb 9, 2019

Sorry for late reply. While I'm not oppose this idea, I see there are a lot of code share between MainWindowController and MiniPlayerWindowController. I'm considering abstract more from those two to reduce code sharing. So I'll probably close this one.

@uiryuu
Copy link
Member

uiryuu commented Feb 17, 2020

Will be fixed by #2772

@anohren
Copy link
Contributor

anohren commented Jun 17, 2020

@Alejx The feature has not been fully implemented as of release 1.0.7. See #2999.

uiryuu added a commit that referenced this pull request Jun 17, 2020
Ref: #2999, #2191

Co-authored-by: LEoREo2247 <info@dev.ringo-12.com>
uiryuu added a commit that referenced this pull request Jun 29, 2020
Ref: #2999, #2191

Co-authored-by: LEoREo2247 <info@dev.ringo-12.com>
lhc70000 pushed a commit that referenced this pull request Jul 10, 2020
Ref: #2999, #2191

Co-authored-by: LEoREo2247 <info@dev.ringo-12.com>
0111b pushed a commit to 0111b/iina that referenced this pull request Nov 7, 2020
Ref: iina#2999, iina#2191

Co-authored-by: LEoREo2247 <info@dev.ringo-12.com>
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.

3 participants