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

Bump the mini player window on opening #4409

Merged
merged 1 commit into from
May 14, 2023

Conversation

uiryuu
Copy link
Member

@uiryuu uiryuu commented May 12, 2023


Description:
Manually bump up the mini player window position on opening when the video view is hidden by the height of the video view.

... when the video player is hidden. Fix #4399
@uiryuu uiryuu requested a review from low-batt May 12, 2023 12:33
@uiryuu uiryuu changed the title Bump the mini player window on opening miniplayer Bump the mini player window on opening May 12, 2023
Copy link
Contributor

@low-batt low-batt left a comment

Choose a reason for hiding this comment

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

I pulled the PR and tested a lot. This definitely greatly improves the behavior.

I'm approving it based on that testing. However…

This code seems like a workaround for bad code in MiniPlayerWindowController? I investigated a tiny bit when I was entering the issue. From what I recall the method normalWindowHeight was being called and isVideoVisible was still set to its default value of true. It seemed like the root cause of the problem had to do with that property not being set to its proper value soon enough.

The other issue here is proper separation of concerns. IINA needs to be slowly refactoring code into the appropriate classes. I think PlayerCore has to direct the transition between the main and mini windows. But I would expect a window's frame to be fully under the control of the associated NSWindowController.

@uiryuu
Copy link
Member Author

uiryuu commented May 14, 2023

I've tried to set normalWindowHeight according to Preference.bool(for: .musicModeShowAlbumArt) but that doesn't solve the problem. I agree that this seems like a workaround; if you think this is too workaround to be merged, we can close it and find a better solution.

@uiryuu uiryuu merged commit 10b0ff0 into develop May 14, 2023
2 checks passed
@uiryuu uiryuu deleted the fix/miniplayer-lower-windowframe branch May 14, 2023 04:28
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.

None yet

2 participants