Fix top of progress bar not behaving as such, #3911 #3915
Conversation
The commit in the pull request will: - Change MainWindowController methods playSliderChanges and updateTimeLabel to position the time preview relative to the progress bar - Add method canShowThumbnailAbove to MainWindowController - Change updateTimeLabel to use canShowThumbnailAbove - Change PlaySliderCell.awakeFromNib to set slider control size to small for macOS 11+ - Increase bottom and top constraints around progress bar from 4.5 to 6 These changes stop the time preview from intruding into the progress bar in the on screen controller.
|
See the issue for a detailed discussion of the issue and the proposed fix. |
| /// | ||
| /// Normally the OSC's thumbnail preview is shown above the time preview. This is the preferred location. However the | ||
| /// thumbnail preview extends beyond the frame of the OSC. If the OSC is near the top of the window this could result | ||
| /// in the thumbnail extending outside of the window resulting in clipping. This method checks if there is room for the |
There was a problem hiding this comment.
I feel like the long-term solution here would be to pull the thumbnail into its own window.
There was a problem hiding this comment.
On the text field and user interaction, in Xcode the behavior attribute, which can be selectable, editable or none, is set to none. I think user interaction is already disabled? So the mouse click goes up the responder chain and is handled by ControlBarView.
If the thumbnail was it its own window how would what work in full screen mode?
And of course I ignored the question of whether IINA should increase the size of its sliders in the spirt of Apple's Big Sur changes. For this fix I tried to make the minimum changes required to restore what I think was the intended behavior. Part of the reason for limiting the fix is due to my lack of UI experience.
There was a problem hiding this comment.
Hmm, my understanding is that if the view does not have user interaction enabled it should just remove itself from hit testing, causing whatever's under it to get the event. Is that not happening?
Also, there are various window flags that allow windows to show up in fullscreen mode.
There was a problem hiding this comment.
I found a UIView hitTest method. Not sure what the equivalent AppKit method is. That method does take into account the state of user interactions:
This method ignores view objects that are hidden, that have disabled user interactions, or have an alpha level less than 0.01.
However I have not found anything about the mouse event being delivered to the view underneath. The UIKit method refers to traversing the view higharchy. In the case at hand the superview is the OSC. This results in the mouseDown method of ControlBarView being called which initiates dragging of the OSC. That was as far as I traced the behavior as I realized the intention was that these OSC elements not overlap.
On converting the screenshot preview to use a NSWindow, although I am usually in favor of adding some cleanup/refactoring in this case I'd rather postpone that past the planned bug fix release. There are a lot of issues that it would be nice to fix in the bug fix release that still need PRs. For example issue #3863. I've tracked down what is going wrong. I was hoping to hear why the code is treating the OSC as a part of the title bar before I remove that. And of course I really need to get back to reviewing the plugin system as that is blocking the creation of PRs to avoid generating merge conflicts.
If you really want me to I'll see if I can switch screenshot previews to use a NSWindow.
There was a problem hiding this comment.
Found the AppKit hitTest method. Unlike the UIKit method the documentation for the AppKit method only mentions ignoring hidden views, nothing about user interaction.
There was a problem hiding this comment.
I'll ask around but I would be very surprised if views that have user interaction disabled do anything meaningful on clicks
There was a problem hiding this comment.
FYI, I used hitTest in the plugin overlay view:
iina/iina/PluginOverlayView.swift
Line 17 in c9dcd40
Overriding this method and returning nil should disable all interactions.
There was a problem hiding this comment.
Yes. But no need to do that for this issue?
The root cause of the problem is the miss-alignment of UI components caused by Apple increasing the size of sliders in Big Sur. Everything works as intended once the alignment problems are corrected.
There was a problem hiding this comment.
Be sure to look at the screenshots in the issue. They show the OSC before the changes in this commit and after.
|
Tested on macOS 10.15, seems fine. |
The commit in the pull request will:
updateTimeLabel to position the time preview relative to the progress
bar
for macOS 11+
These changes stop the time preview from intruding into the progress bar
in the on screen controller.
Description:
Correct regression introduced by Big Sur.