Skip to content

[TODO] Fix the incorrect detached status display - #735

Closed
bo2themax wants to merge 0 commit into
insidegui:masterfrom
bo2themax:master
Closed

bo2themax wants to merge 0 commit into
insidegui:masterfrom
bo2themax:master

Conversation

@bo2themax

@bo2themax bo2themax commented Jul 31, 2025 •

Copy link
Copy Markdown
Contributor

While adopting Liquid Glass, I’ve noticed an issue with the detached status. I tested it with the release version, and it appears to be a historical problem.

When a video enters PiP mode and a new session is selected, the new session thumbnails are displayed as the status. This should be the corresponding thumbnails as the video is not currently playing. Switching back to the playing video displays the default status view provided by AVPlayer which should be the custom status view attached to the incorrect session.

Screen.Recording.2025-07-31.at.13.32.18.mov

Debugging revealed multiple $selectedSession listeners. One updates the view model of the detail view and the other checks if it is appropriate to enter PiP mode. However, the order of these two listeners is not deterministic so they can occur in any order. If the PiP listener occurs early it should function as expected. However, as far as I have tested on macOS Tahoe, after opening WWDC for the first time they almost appeared in the incorrect order in 90% of cases.

This fix comprises three stages, each addressing the previous one:

  1. Modify the update order to ensure the second listener is executed.
  2. Resolve the issue where a new video replaces the Picture-in-Picture content without altering the detached status of that session.
  3. Correct the positioning of the detached status.
  4. Add a status check we attaching detected status indicator

The result behaviour is looking like this now. I’ve tried to stick as closely to the codebase as possible, but there might still be a few spots where a change could make things even better.🙈

Screen.Recording.2025-07-31.at.15.40.04.mov

@allenhumphreys

Copy link
Copy Markdown
Collaborator

I noticed this problem, too!

@bo2themax
bo2themax marked this pull request as draft July 31, 2025 17:42
@bo2themax

Copy link
Copy Markdown
Contributor Author

I noticed this problem, too!

Found some issues for this fix when playing in fullscreen mode, made it WIP for now

@allenhumphreys

Copy link
Copy Markdown
Collaborator

Be careful, I have a relatively large refactor going on in #729 . I'm hoping to get back to it soon, but I don't want to end up with lots of conflicts!

@bo2themax
bo2themax marked this pull request as ready for review July 31, 2025 17:59
@bo2themax

Copy link
Copy Markdown
Contributor Author

Be careful, I have a relatively large refactor going on in #729 . I'm hoping to get back to it soon, but I don't want to end up with lots of conflicts!

Added a check when attaching the indicator view, please review the changes if they have any potential conflicts with your refactor

@bo2themax

bo2themax commented Jul 31, 2025 •

Copy link
Copy Markdown
Contributor Author

There are still issues when switching tabs while pip is playing. I will mark this as a work in progress and change the title here as it involves too many temporary fixes to synchronise the state between ShelfViewController and PUIPlayerView (VideoPlayerViewController). The latter is a singleton due to the pip limit. I think in a future version you should consider embedding the Shelf within the single-player container view to achieve a more stable experience. I will try this in my own branch.

Big thanks to you and everyone for their exceptional contributions to maintaining this project over the past years.

@bo2themax
bo2themax marked this pull request as draft July 31, 2025 19:34
@bo2themax bo2themax changed the title Fix the incorrect detached status display [TODO] Fix the incorrect detached status display Jul 31, 2025
@allenhumphreys

Copy link
Copy Markdown
Collaborator

I’m not surprised. I was looking through the code and the implementation of selection is a bit complex.

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