Skip to content

feat: Use custom controls for video previews - #2073

Open
BaptGrv wants to merge 13 commits into
masterfrom
fix/autoplay-video
Open

BaptGrv wants to merge 13 commits into
masterfrom
fix/autoplay-video

Conversation

@BaptGrv

@BaptGrv BaptGrv commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

No description provided.

Copilot AI balanced review requested due to automatic review settings November 6, 2026 10:12
@BaptGrv
BaptGrv force-pushed the fix/autoplay-video branch from ea7f43a to 471fc63 Compare November 6, 2026 10:17

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Compilation is blocked by an implicit-self access in an escaping closure and a nonconforming test delegate, with additional playback and control-state defects unresolved.

7 open issues
What changed in this PR

This PR replaces native video preview controls with custom UIKit controls, integrating playback controls into kDrive’s preview layout and sharing visual styling with audio previews.

Changes:

  • Adds layer-based video rendering, custom playback and seeking controls, AirPlay, and picture-in-picture setup.
  • Connects control visibility to fullscreen preview state.
  • Shares media control styling and remaining-time formatting between audio and video.
File Description
kDriveCore/​VideoPlayer/​VideoPlayer.swift Exposes the underlying player for custom rendering.
kDriveCore/​AudioPlayer/​SingleTrackPlayer.swift Extracts remaining-time formatting with duration handling.
kDrive/​UI/​View/​Files/​Preview/​VideoControlsView.swift Adds custom playback, seeking, time, and AirPlay controls.
kDrive/​UI/​View/​Files/​Preview/​VideoCollectionViewCell.swift Integrates custom rendering, controls, and playback observation.
kDrive/​UI/​View/​Files/​Preview/​TouchTrackingGestureRecognizer.swift Removes the native-control touch tracker.
kDrive/​UI/​View/​Files/​Preview/​PreviewCollectionViewCell.swift Adds a fullscreen-change callback.
kDrive/​UI/​View/​Files/​Preview/​PlayerLayerView.swift Adds a UIView backed by AVPlayerLayer.
kDrive/​UI/​View/​Files/​Preview/​MediaPlaybackStyle.swift Centralizes time-label fonts and slider-thumb rendering.
kDrive/​UI/​View/​Files/​Preview/​AudioCollectionViewCell.swift Adopts shared media styling.
kDrive/​UI/​Controller/​Files/​Preview/​PreviewViewController.swift Exposes fullscreen state and notifies visible preview cells.

Review effort: Balanced

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Unresolved issues cause incorrect remaining-time labels at playback completion, stale playback controls after reversed swipes, and VoiceOver slider adjustments that fail to seek while paused.

1 open issue
1 previously missed issue

In code that hasn't changed since last review

Medium severity Update pause controls after the video leaves the current page

kDrive/​UI/​View/​Files/​Preview/​VideoCollectionViewCell.swift:255

didEndDisplaying clears isCurrentPage before pausing, so this guard drops the pause-state update. If the user reverses a swipe and returns to the video before its cell leaves the collection view, the controls can retain the pause icon even though startFromBeginning leaves playback paused. Update the controls for the attached player even when its cell is not the current page.

7 resolved since last review

Review effort: Balanced

@BaptGrv
BaptGrv force-pushed the fix/autoplay-video branch from 81983bb to 9800900 Compare November 6, 2026 14:11
@sonarqubecloud

sonarqubecloud Bot commented Nov 9, 2026

Copy link
Copy Markdown

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