MiniPlayer: Audio #221
MiniPlayer: Audio #221
Conversation
Mikanbu
force-pushed
the
newui/miniplayer/audio/init
branch
from
2e3ebdd
to
f054ccb
Compare
|
Rebased to style fix on master. |
Mikanbu
force-pushed
the
newui/miniplayer/audio/init
branch
from
77afcac
to
bef7906
Compare
|
Rebased and add handling of video output. |
| var visible: Bool = false | ||
| var contentHeight: Float { | ||
| return 72.0 | ||
| } |
There was a problem hiding this comment.
Ideally this would resize itself based on it's constraints and content, thinking of longer titles or people with bigger font. That is something the old UI lagged
There was a problem hiding this comment.
I would say in a follow-up ticket. For me, this isn't a priority.
| currentMediaHasTrackToChooseFrom: Bool, | ||
| currentMediaHasChapters: Bool, | ||
| for controller: VLCPlaybackController) { | ||
| updatePlayPauseButton() |
There was a problem hiding this comment.
I believe there is a bug where the isPlaying event doesn't always get triggered that we worked around in the old UI. Can you double check if it was handled in the UI or somewhere else?
There was a problem hiding this comment.
I'm not sure to see which bug you are referring to? Looking at the old mini player it doesn't look like there was anything special.
Please let me know if I missed something.
| switch sender.state { | ||
| // case .began: | ||
| // In the case of .began we could a an icon like the old miniplayer | ||
| case .ended: |
There was a problem hiding this comment.
make it a ticket and remove the suggestion and commented out code. Instead of leaving that in code, commenting here on the PR is better than checking suggestions in into the codebase :)
There was a problem hiding this comment.
Yes, I didn't want to create a ticket before the PR was merged.
I still would want to keep the comment with a ref to the ticket but I can also remove it.
| private extension AudioMiniPlayer { | ||
| private func initView() { | ||
| Bundle.main.loadNibNamed("AudioMiniPlayer", owner: self, options: nil) | ||
| addSubview(audioMiniPlayer) |
There was a problem hiding this comment.
This looks odd to me that the AudioMiniplayer loads a Nib that is also called AudioMiniPlayer and adds that as a subview.
I would've expected this call to be wherever the AudioMiniplayer is instantiated and to have the rest of the setup code in initWithDecoder or awakeFromNib.
This add a new audio mini player according to the new design. For now, we will use the same player for audio and video. (closes videolan#164)
Mikanbu
force-pushed
the
newui/miniplayer/audio/init
branch
from
9b98a32
to
c797a1d
Compare
|
Addressed review, rebased & fixupped. |
Checklist
bundle exec fastlane testfrom the root directory to see all new and existing tests passDescription
This adds a new mini player for audio according to the new design.
With this PR, I didn't want to break the whole
VLCPlayerDisplayControllerarchitecture we currently have. So in order to useVLCPlayerDisplayControllerI created aMiniPlayerinterface.Therefore, this removes the usage of
VLCMiniPlaybackViewfrom the project.For now, in
VLCPlayerDisplayController, only theAudioMiniPlayeris used.This should be fixed with the integration of a
VideoMiniPlayerlater on.