Skip to content
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

Added seek forward and rewind buttons #717

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 31 additions & 10 deletions lib/src/cupertino/cupertino_controls.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@ import 'package:chewie/src/chewie_progress_colors.dart';
import 'package:chewie/src/cupertino/cupertino_progress_bar.dart';
import 'package:chewie/src/cupertino/widgets/cupertino_options_dialog.dart';
import 'package:chewie/src/helpers/utils.dart';
import 'package:chewie/src/hit_area_controls.dart';
import 'package:chewie/src/models/option_item.dart';
import 'package:chewie/src/models/subtitle_model.dart';
import 'package:chewie/src/notifiers/index.dart';
import 'package:chewie/src/seek_rewind_button.dart';
import 'package:flutter/cupertino.dart';
import 'package:flutter/material.dart';
import 'package:provider/provider.dart';
Expand All @@ -22,12 +24,14 @@ class CupertinoControls extends StatefulWidget {
required this.backgroundColor,
required this.iconColor,
this.showPlayButton = true,
this.showSeekButton = true,
Key? key,
}) : super(key: key);

final Color backgroundColor;
final Color iconColor;
final bool showPlayButton;
final bool showSeekButton;

@override
State<StatefulWidget> createState() {
Expand Down Expand Up @@ -349,9 +353,11 @@ class _CupertinoControlsState extends State<CupertinoControls>
final bool isFinished = _latestValue.position >= _latestValue.duration;
final bool showPlayButton =
widget.showPlayButton && !_latestValue.isPlaying && !_dragging;
final bool showSeekButton =
widget.showSeekButton && !_latestValue.isPlaying && !_dragging;

return GestureDetector(
onTap: _latestValue.isPlaying
return HitAreaControls(
onTapPlay: _latestValue.isPlaying
? _cancelAndRestartTimer
: () {
_hideTimer?.cancel();
Expand All @@ -360,14 +366,15 @@ class _CupertinoControlsState extends State<CupertinoControls>
notifier.hideStuff = false;
});
},
child: CenterPlayButton(
backgroundColor: widget.backgroundColor,
iconColor: widget.iconColor,
isFinished: isFinished,
isPlaying: controller.value.isPlaying,
show: showPlayButton,
onPressed: _playPause,
),
Comment on lines -363 to -370
Copy link
Collaborator

Choose a reason for hiding this comment

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

Cupertino controls already have the seek-forward and rewind buttons in the seek bar (i.e. the ones that rewind and fast-forward by 15 seconds).

As a result, this change should only apply to Material based widgets.

Do this for the other new widgets added in this PR, since they should only apply to Material.

Copy link
Author

Choose a reason for hiding this comment

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

Hello @diegotori

requested changes done please review

Thank you

backgroundColor: widget.backgroundColor,
iconColor: widget.iconColor,
isFinished: isFinished,
isPlaying: controller.value.isPlaying,
showPlayButton: showPlayButton,
showSeekButton: showSeekButton,
onPressedPlay: _playPause,
seekRewind: _seekRewind,
seekForward: _seekForward,
);
}

Expand Down Expand Up @@ -806,6 +813,20 @@ class _CupertinoControlsState extends State<CupertinoControls>
_subtitlesPosition = controller.value.position;
});
}

void _seekTo({int seconds = 10}) {
setState(() {
controller.seekTo(
Duration(
seconds: _latestValue.position.inSeconds + seconds,
),
);
});
}

void _seekForward() => _seekTo();

void _seekRewind() => _seekTo(seconds: -10);
}

class _PlaybackSpeedDialog extends StatelessWidget {
Expand Down
64 changes: 64 additions & 0 deletions lib/src/hit_area_controls.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
import 'package:flutter/material.dart';

import 'center_play_button.dart';
import 'seek_rewind_button.dart';

class HitAreaControls extends StatelessWidget {
const HitAreaControls({
Key? key,
this.onTapPlay,
this.onPressedPlay,
this.seekRewind,
this.seekForward,
required this.backgroundColor,
required this.iconColor,
required this.isFinished,
required this.isPlaying,
required this.showPlayButton,
required this.showSeekButton,
}) : super(key: key);

final Function()? onTapPlay;
final Function()? onPressedPlay;
final Function()? seekRewind;
final Function()? seekForward;
final Color backgroundColor;
final Color iconColor;
final bool isFinished;
final bool isPlaying;
final bool showPlayButton;
final bool showSeekButton;

@override
Widget build(BuildContext context) {
return Row(
mainAxisAlignment: MainAxisAlignment.center,
children: [
_buildSeekRewindButton(isSeekForward: false),
GestureDetector(
onTap: onTapPlay,
child: CenterPlayButton(
backgroundColor: backgroundColor,
iconColor: iconColor,
isFinished: isFinished,
isPlaying: isPlaying,
show: showPlayButton,
onPressed: onPressedPlay,
),
),
_buildSeekRewindButton(isSeekForward: true),
],
);
}

SeekRewindButton _buildSeekRewindButton({bool isSeekForward = true}) {
return SeekRewindButton(
backgroundColor: backgroundColor,
iconColor: iconColor,
show: showSeekButton,
onPressed: isSeekForward ? seekForward : seekRewind,
onDoublePressed: isSeekForward ? seekForward : seekRewind,
icon: isSeekForward ? Icons.fast_forward : Icons.fast_rewind,
);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do not use methods to return widgets. Instead you can simply add SeekRewindButton as part of your Row children.

Copy link
Author

Choose a reason for hiding this comment

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

earlier it was inside a Row children. it's extracted into method as changes requested #717 (comment)

Copy link
Collaborator

@diegotori diegotori Aug 2, 2023

Choose a reason for hiding this comment

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

@maherjaafar just an FYI, according to this issue and the official documentation regarding this subject, large Widgets should be spun off into new ones instead of methods that return them.

That way, you'll be able to create them as consts, thus increasing performance since they won't have to be re-built when build is called again.

@punit1111 sorry for the extra work, but these widgets should ultimately be extracted as new StatelessWidgets instead of being computed from a method.

Please let me know if this clears up things.

Copy link
Author

Choose a reason for hiding this comment

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

@diegotori SeekRewindButton is already extracted as Stateless Widget lib/src/seek_rewind_button.dart

Copy link
Collaborator

@diegotori diegotori Aug 4, 2023

Choose a reason for hiding this comment

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

@punit1111 that is true, but instead of creating it from _buildSeekRewindButton, you should just replace those calls with the actual widget:

Change this:

_buildSeekRewindButton(isSeekForward: true),

to this:

SeekRewindButton(
      backgroundColor: backgroundColor,
      iconColor: iconColor,
      show: showSeekButton,
      onPressed: seekForward,
      onDoublePressed: seekForward,
      icon: Icons.fast_forward,
    )

Copy link
Collaborator

Choose a reason for hiding this comment

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

Do likewise for the other places where that method is being called from.

}
41 changes: 31 additions & 10 deletions lib/src/material/material_controls.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,23 +4,27 @@ import 'package:chewie/src/center_play_button.dart';
import 'package:chewie/src/chewie_player.dart';
import 'package:chewie/src/chewie_progress_colors.dart';
import 'package:chewie/src/helpers/utils.dart';
import 'package:chewie/src/hit_area_controls.dart';
import 'package:chewie/src/material/material_progress_bar.dart';
import 'package:chewie/src/material/widgets/options_dialog.dart';
import 'package:chewie/src/material/widgets/playback_speed_dialog.dart';
import 'package:chewie/src/models/option_item.dart';
import 'package:chewie/src/models/subtitle_model.dart';
import 'package:chewie/src/notifiers/index.dart';
import 'package:chewie/src/seek_rewind_button.dart';
import 'package:flutter/material.dart';
import 'package:provider/provider.dart';
import 'package:video_player/video_player.dart';

class MaterialControls extends StatefulWidget {
const MaterialControls({
this.showPlayButton = true,
this.showSeekButton = true,
Key? key,
}) : super(key: key);

final bool showPlayButton;
final bool showSeekButton;

@override
State<StatefulWidget> createState() {
Expand Down Expand Up @@ -366,9 +370,11 @@ class _MaterialControlsState extends State<MaterialControls>
final bool isFinished = _latestValue.position >= _latestValue.duration;
final bool showPlayButton =
widget.showPlayButton && !_dragging && !notifier.hideStuff;
final bool showSeekButton =
widget.showSeekButton && !_dragging && !notifier.hideStuff;

return GestureDetector(
onTap: () {
return HitAreaControls(
onTapPlay: () {
if (_latestValue.isPlaying) {
if (_displayTapped) {
setState(() {
Expand All @@ -385,14 +391,15 @@ class _MaterialControlsState extends State<MaterialControls>
});
}
},
child: CenterPlayButton(
backgroundColor: Colors.black54,
iconColor: Colors.white,
isFinished: isFinished,
isPlaying: controller.value.isPlaying,
show: showPlayButton,
onPressed: _playPause,
),
backgroundColor: Colors.black54,
iconColor: Colors.white,
isFinished: isFinished,
isPlaying: controller.value.isPlaying,
showPlayButton: showPlayButton,
showSeekButton: showSeekButton,
onPressedPlay: _playPause,
seekRewind: _seekRewind,
seekForward: _seekForward,
);
}

Expand Down Expand Up @@ -542,6 +549,20 @@ class _MaterialControlsState extends State<MaterialControls>
});
}

void _seekTo({int seconds = 10}) {
setState(() {
controller.seekTo(
Duration(
seconds: _latestValue.position.inSeconds + seconds,
),
);
});
}

void _seekForward() => _seekTo();

void _seekRewind() => _seekTo(seconds: -10);

void _startHideTimer() {
final hideControlsTimer = chewieController.hideControlsTimer.isNegative
? ChewieController.defaultHideControlsTimer
Expand Down
41 changes: 31 additions & 10 deletions lib/src/material/material_desktop_controls.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,23 +5,27 @@ import 'package:chewie/src/center_play_button.dart';
import 'package:chewie/src/chewie_player.dart';
import 'package:chewie/src/chewie_progress_colors.dart';
import 'package:chewie/src/helpers/utils.dart';
import 'package:chewie/src/hit_area_controls.dart';
import 'package:chewie/src/material/material_progress_bar.dart';
import 'package:chewie/src/material/widgets/options_dialog.dart';
import 'package:chewie/src/material/widgets/playback_speed_dialog.dart';
import 'package:chewie/src/models/option_item.dart';
import 'package:chewie/src/models/subtitle_model.dart';
import 'package:chewie/src/notifiers/index.dart';
import 'package:chewie/src/seek_rewind_button.dart';
import 'package:flutter/material.dart';
import 'package:provider/provider.dart';
import 'package:video_player/video_player.dart';

class MaterialDesktopControls extends StatefulWidget {
const MaterialDesktopControls({
this.showPlayButton = true,
this.showSeekButton = true,
Key? key,
}) : super(key: key);

final bool showPlayButton;
final bool showSeekButton;

@override
State<StatefulWidget> createState() {
Expand Down Expand Up @@ -330,9 +334,11 @@ class _MaterialDesktopControlsState extends State<MaterialDesktopControls>
final bool isFinished = _latestValue.position >= _latestValue.duration;
final bool showPlayButton =
widget.showPlayButton && !_dragging && !notifier.hideStuff;
final bool showSeekButton =
widget.showSeekButton && !_dragging && !notifier.hideStuff;

return GestureDetector(
onTap: () {
return HitAreaControls(
onTapPlay: () {
if (_latestValue.isPlaying) {
if (_displayTapped) {
setState(() {
Expand All @@ -349,14 +355,15 @@ class _MaterialDesktopControlsState extends State<MaterialDesktopControls>
});
}
},
child: CenterPlayButton(
backgroundColor: Colors.black54,
iconColor: Colors.white,
isFinished: isFinished,
isPlaying: controller.value.isPlaying,
show: showPlayButton,
onPressed: _playPause,
),
backgroundColor: Colors.black54,
iconColor: Colors.white,
isFinished: isFinished,
isPlaying: controller.value.isPlaying,
showPlayButton: showPlayButton,
showSeekButton: showSeekButton,
onPressedPlay: _playPause,
seekRewind: _seekRewind,
seekForward: _seekForward,
);
}

Expand Down Expand Up @@ -596,4 +603,18 @@ class _MaterialDesktopControlsState extends State<MaterialDesktopControls>
),
);
}

void _seekTo({int seconds = 10}) {
setState(() {
controller.seekTo(
Duration(
seconds: _latestValue.position.inSeconds + seconds,
),
);
});
}

void _seekForward() => _seekTo();

void _seekRewind() => _seekTo(seconds: -10);
}
61 changes: 61 additions & 0 deletions lib/src/seek_rewind_button.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import 'package:flutter/material.dart';

class SeekRewindButton extends StatefulWidget {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be renamed to SeekControlButton, so that it better reflects its stated purpose.

That way, you can create buttons that just configure that widget for its specific purpose:

class SeekRewindButton extends StatefulWidget {
   const SeekRewindButton({
     super.key,
    required this.backgroundColor,
    this.iconColor,
    required this.show,
    this.onPressed,
    this.onDoublePressed,
    this.icon = const Icons.fast_rewind,
   });
   
  final Color backgroundColor;
  final Color? iconColor;
  final bool show;
  final VoidCallback? onPressed;
  final VoidCallback? onDoublePressed;
  final IconData icon;
   
   Widget build(BuildContext context) {
     return SeekControlButton(
       backgroundColor: backgroundColor,
       iconColor: iconColor,
       show: show,
       icon: icon,
       onPressed: onPressed,
       onDoublePressed: onDoublePressed,
     );
   }
}

Do likewise for the fast-forward button: (i.e. SeekFastForwardButton).

const SeekRewindButton({
Key? key,
required this.backgroundColor,
this.iconColor,
required this.show,
required this.icon,
this.onPressed,
this.onDoublePressed,
}) : super(key: key);

final Color backgroundColor;
final Color? iconColor;
final bool show;
final VoidCallback? onPressed;
final VoidCallback? onDoublePressed;
final IconData? icon;
Copy link
Collaborator

Choose a reason for hiding this comment

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

If required, make it non-nullable.


@override
State<SeekRewindButton> createState() => _SeekRewindButtonState();
}

class _SeekRewindButtonState extends State<SeekRewindButton> {

@override
void initState() {
super.initState();
}

@override
Widget build(BuildContext context) {
return widget.show
? Expanded(
child: GestureDetector(
onDoubleTap: widget.onDoublePressed,
child: ColoredBox(
color: Colors.transparent,
child: Center(
child: DecoratedBox(
decoration: BoxDecoration(
color: widget.backgroundColor,
shape: BoxShape.circle,
),
// Always set the iconSize on the IconButton, not on the Icon itself:
// https://github.com/flutter/flutter/issues/52980
child: IconButton(
iconSize: 32,
padding: const EdgeInsets.all(12.0),
icon: Icon(widget.icon, color: widget.iconColor),
onPressed: widget.onPressed,
),
),
),
),
),
)
: const SizedBox.shrink();
}
}