Closed insidegui closed 3 months ago
Looks great, noticed a few things:
-
to signal that it is the time remaining. Bonus points if we took the tvOS feature of showing what time of day the video will finish :-Pspeaker.wave.3.fill
-> speaker.wave.2.fill
-> speaker.wave.1.fill
Loving the more SwiftUI, this UI layout is insanely hard to follow.
I'm not discarding the possibility of redoing the whole player in SwiftUI tbh 😅
Surely the controls view could pretty easily be a swiftui view, it's basically just buttons. Maybe a binding or 2. It'd be like 500 fewer lines of code 😂
Something else we can remove (regardless of a SwiftUI migration) is the external playback stuff, since that's been disabled with a compile-time flag for a while.
Can we add subtitles to downloaded videos? The current situation is that even if a video has subtitles when played online, the subtitles will be lost as long as it is downloaded and played again. In fact, transcript can be used as subtitles. Is it difficult to implement this feature?
@sugarmo That's tracked in #657, feel free to continue this discussion there
The window width is now constrained to be unnecessarily wide
Fixed in https://github.com/insidegui/WWDC/pull/695/commits/c9379dbe01db8fc8dde9c8843f5928182fc4cbdd
For the PiP external status view, the status view takes the full space of the view which results in a bit of a jarring UI transition when moving in and our of PiP
Fixed as part of https://github.com/insidegui/WWDC/pull/695/commits/3780e5a11f2b99913b723e3705f69d19b9c0b98f Specifically: https://github.com/insidegui/WWDC/blob/3780e5a11f2b99913b723e3705f69d19b9c0b98f/PlayerUI/Views/PUIPlayerView.swift#L665-L668
The volume slider thumb is vertically misplaced
Probably for some weird legacy reason it was using custom drawing, just reverted to a regular NSSlider
with .small
control size: https://github.com/insidegui/WWDC/pull/695/commits/8c9bb0cfbc52d7e0bf3949c994de782a4a9a6a62
We should still the UX of the time remaining label from QuickTime. Clicking it changes between total time and time remaining. They also use a - to signal that it is the time remaining. Bonus points if we took the tvOS feature of showing what time of day the video will finish :-P
Implemented in 0c0fb0aff921d711b7d2b4e282af8baba647f308
I like the idea of having the time of day, will keep that in mind for later :)
The volume icon could change as the slider changes, e.g. speaker.wave.3.fill -> speaker.wave.2.fill -> speaker.wave.1.fill
Done in 7f3d5a5b4a17f71217e0f10cbaf00ce2124f7d40
The goforward/backward buttons look squished.
Not sure what happened (maybe different SDKs / macOS versions) but they look fine now, macOS 14.5 building with Xcode 15.3
We should still the UX of the time remaining label from QuickTime.
Steal* 🤪
Nothing like correcting the internet record 1 year later!
The book marks indicators a bit funny, could be existing issues
Redesigned in 5d304f46c34c5bef080875a3efd922737f74e7b1
I managed to create a bunch of book marks all at the same spot, then while removing them I managed to get the app to crash with a constraint not being finite! You can also crash it by just changing the playback speed a few times.
Not sure what the crash was about, but since we don't do clustering or some other solution for now, I've introduced a minimum distance of 30s between bookmarks, which should not be a problem for regular use (I usually have just a few bookmarks in any given session, and they're never really close together like that). Also done in 5d304f46c34c5bef080875a3efd922737f74e7b1
https://github.com/insidegui/WWDC/assets/67184/989fa16c-04fb-4afa-ae0b-fec6542cd0a0
I managed to create a bunch of book marks all at the same spot
Mitigated in 9bd28342da58a13df05d93a3ec045b41cf1efe9f
Before
After