-
Notifications
You must be signed in to change notification settings - Fork 3.5k
[video_player] Added fullscreen functionality (for web). #3577
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
2db6f5e
26dc525
11bdda0
03cb25b
bd1a0cf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -91,6 +91,32 @@ void main() { | |
}, | ||
); | ||
|
||
testWidgets( | ||
'can go fullscreen', | ||
(WidgetTester tester) async { | ||
await controller.initialize(); | ||
// Mute to allow playing without DOM interaction on Web. | ||
// See https://developers.google.com/web/updates/2017/09/autoplay-policy-changes | ||
await controller.setVolume(0); | ||
|
||
await controller.play(); | ||
await controller.toggleFullScreen(); | ||
await tester.pumpAndSettle(_playDuration); | ||
|
||
expect(controller.value.isFullScreen, true); | ||
expect(controller.value.position, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How is this expectation related to the functionality you are testing? |
||
(Duration position) => position > Duration.zero); | ||
|
||
await controller.toggleFullScreen(); | ||
await tester.pumpAndSettle(_playDuration); | ||
|
||
expect(controller.value.isFullScreen, false); | ||
expect(controller.value.position, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same question here. |
||
(Duration position) => position > Duration.zero); | ||
}, | ||
skip: !kIsWeb, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rather than making this platform-gated, there should be a |
||
); | ||
|
||
testWidgets( | ||
'can seek', | ||
(WidgetTester tester) async { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -47,6 +47,7 @@ class VideoPlayerValue { | |
this.isPlaying = false, | ||
this.isLooping = false, | ||
this.isBuffering = false, | ||
this.isFullScreen = false, | ||
this.volume = 1.0, | ||
this.playbackSpeed = 1.0, | ||
this.rotationCorrection = 0, | ||
|
@@ -93,6 +94,9 @@ class VideoPlayerValue { | |
/// True if the video is playing. False if it's paused. | ||
final bool isPlaying; | ||
|
||
/// True if the video is in fullscreen. False if it's not. | ||
final bool isFullScreen; | ||
|
||
/// True if the video is looping. | ||
final bool isLooping; | ||
|
||
|
@@ -151,6 +155,7 @@ class VideoPlayerValue { | |
List<DurationRange>? buffered, | ||
bool? isInitialized, | ||
bool? isPlaying, | ||
bool? isFullScreen, | ||
bool? isLooping, | ||
bool? isBuffering, | ||
double? volume, | ||
|
@@ -167,6 +172,7 @@ class VideoPlayerValue { | |
buffered: buffered ?? this.buffered, | ||
isInitialized: isInitialized ?? this.isInitialized, | ||
isPlaying: isPlaying ?? this.isPlaying, | ||
isFullScreen: isFullScreen ?? this.isFullScreen, | ||
isLooping: isLooping ?? this.isLooping, | ||
isBuffering: isBuffering ?? this.isBuffering, | ||
volume: volume ?? this.volume, | ||
|
@@ -475,6 +481,19 @@ class VideoPlayerController extends ValueNotifier<VideoPlayerValue> { | |
await _applyPlayPause(); | ||
} | ||
|
||
/// The video enters in fullscreen (only works for Web). | ||
Future<void> toggleFullScreen() async { | ||
if (kIsWeb) { | ||
if (!value.isFullScreen) { | ||
await _applyFullScreen(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since these are only used in one place, they don't need to be separated out into helpers. |
||
value = value.copyWith(isFullScreen: true); | ||
} else { | ||
await _applyExitFullScreen(); | ||
value = value.copyWith(isFullScreen: false); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can't the user exit full screen directly? It seems like we would need a state listener for this, rather than managing the state from Dart. |
||
} | ||
} | ||
} | ||
|
||
Future<void> _applyLooping() async { | ||
if (_isDisposedOrNotInitialized) { | ||
return; | ||
|
@@ -522,6 +541,20 @@ class VideoPlayerController extends ValueNotifier<VideoPlayerValue> { | |
await _videoPlayerPlatform.setVolume(_textureId, value.volume); | ||
} | ||
|
||
Future<void> _applyFullScreen() async { | ||
if (!kIsWeb || _isDisposedOrNotInitialized) { | ||
return; | ||
} | ||
await _videoPlayerPlatform.enterFullScreen(_textureId); | ||
} | ||
|
||
Future<void> _applyExitFullScreen() async { | ||
if (!kIsWeb || _isDisposedOrNotInitialized) { | ||
return; | ||
} | ||
await _videoPlayerPlatform.exitFullScreen(_textureId); | ||
} | ||
|
||
Future<void> _applyPlaybackSpeed() async { | ||
if (_isDisposedOrNotInitialized) { | ||
return; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -125,6 +125,16 @@ void main() { | |
expect(VideoPlayerPlatform.instance.pause(await textureId), completes); | ||
}); | ||
|
||
testWidgets('can enter fullscreen', (WidgetTester tester) async { | ||
expect(VideoPlayerPlatform.instance.enterFullScreen(await textureId), | ||
completes); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is only testing that the future completes, not that it does anything. Wouldn't at empty implementation of |
||
}); | ||
|
||
testWidgets('can exit fullscreen', (WidgetTester tester) async { | ||
expect(VideoPlayerPlatform.instance.exitFullScreen(await textureId), | ||
completes); | ||
}); | ||
|
||
testWidgets('can set volume', (WidgetTester tester) async { | ||
expect( | ||
VideoPlayerPlatform.instance.setVolume(await textureId, 0.8), | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you have to play a second of video for full screen changes to take effect?