Conversation
|
@punit1111 Thank you for your PR. Please re-sync your changes with the latest changes in |
@diegotori re-sync done please review the PR Thank you |
|
@maherjaafar requested changes are done. please review the PR Thank you |
| 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, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Do not use methods to return widgets. Instead you can simply add SeekRewindButton as part of your Row children.
There was a problem hiding this comment.
earlier it was inside a Row children. it's extracted into method as changes requested #717 (comment)
There was a problem hiding this comment.
@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.
There was a problem hiding this comment.
@diegotori SeekRewindButton is already extracted as Stateless Widget lib/src/seek_rewind_button.dart
There was a problem hiding this comment.
@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,
)There was a problem hiding this comment.
Do likewise for the other places where that method is being called from.
| @@ -0,0 +1,61 @@ | |||
| import 'package:flutter/material.dart'; | |||
|
|
|||
| class SeekRewindButton extends StatefulWidget { | |||
There was a problem hiding this comment.
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).
| final bool show; | ||
| final VoidCallback? onPressed; | ||
| final VoidCallback? onDoublePressed; | ||
| final IconData? icon; |
There was a problem hiding this comment.
If required, make it non-nullable.
|
Hi @diegotori please review the Changes and let me know if anything needs to be fixed Thank you |
| child: CenterPlayButton( | ||
| backgroundColor: widget.backgroundColor, | ||
| iconColor: widget.iconColor, | ||
| isFinished: isFinished, | ||
| isPlaying: controller.value.isPlaying, | ||
| show: showPlayButton, | ||
| onPressed: _playPause, | ||
| ), |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
|
@maherjaafar please give this PR one last look before I can finalize this change. Thanks. |
|
Hi, |
|
Thanks @punit1111 — this PR pioneered double-tap-to-seek for chewie. For context on why it stalled: #703 shipped the center seek buttons in Sep 2024 (this PR was approved again two days after that, which is how the overlap got missed), and master has since diverged past the The parts master still lacks — double-tap left/right zones with an accumulating indicator — are now implemented in #964 (stacked on #953), with credit to this PR. Suggest the maintainers close this one as superseded by #703 + #964. |
Mobile (MaterialControls): - Double-tapping the left/right side of the video seeks backward/forward, flashing the accumulating seek indicator. The middle band is a dead zone. - While the indicator is visible, further single taps in the same zone keep seeking (10s -> 20s -> 30s), like YouTube. - Gated by ChewieController.allowDoubleTapSeek (default true) with the step set by ChewieController.doubleTapSeekDuration (default 10s). Disabled on live streams and when playback is finished. Desktop (MaterialDesktopControls): - Double-clicking the video toggles fullscreen, like the YouTube desktop player. Gated by ChewieController.allowDoubleTapToggleFullScreen (default true) and allowFullScreen. The keyboard seek indicator state introduced by fluttercommunity#953 moves from _MaterialDesktopControlsState into a shared SeekIndicatorStateMixin so both control sets drive the same SeekIndicator. Double-tap-to-seek was first explored by punit1111 in fluttercommunity#717.
Mobile (MaterialControls): - Double-tapping the left/right side of the video seeks backward/forward, flashing the accumulating seek indicator. The middle band is a dead zone. - While the indicator is visible, further single taps in the same zone keep seeking (10s -> 20s -> 30s), like YouTube. - Gated by ChewieController.allowDoubleTapSeek (default true) with the step set by ChewieController.doubleTapSeekDuration (default 10s). Disabled on live streams and when playback is finished. Desktop (MaterialDesktopControls): - Double-clicking the video toggles fullscreen, like the YouTube desktop player. Gated by ChewieController.allowDoubleTapToggleFullScreen (default true) and allowFullScreen. The keyboard seek indicator state introduced by fluttercommunity#953 moves from _MaterialDesktopControlsState into a shared SeekIndicatorStateMixin so both control sets drive the same SeekIndicator. Double-tap-to-seek was first explored by punit1111 in fluttercommunity#717.
Mobile (MaterialControls): - Double-tapping the left/right side of the video seeks backward/forward, flashing the accumulating seek indicator. The middle band is a dead zone. - While the indicator is visible, further single taps in the same zone keep seeking (10s -> 20s -> 30s), like YouTube. - Gated by ChewieController.allowDoubleTapSeek (default true) with the step set by ChewieController.doubleTapSeekDuration (default 10s). Disabled on live streams and when playback is finished. Desktop (MaterialDesktopControls): - Double-clicking the video toggles fullscreen, like the YouTube desktop player. Gated by ChewieController.allowDoubleTapToggleFullScreen (default true) and allowFullScreen. The keyboard seek indicator state introduced by fluttercommunity#953 moves from _MaterialDesktopControlsState into a shared SeekIndicatorStateMixin so both control sets drive the same SeekIndicator. Double-tap-to-seek was first explored by punit1111 in fluttercommunity#717.
Mobile (MaterialControls): - Double-tapping the left/right side of the video seeks backward/forward, flashing the accumulating seek indicator. The middle band is a dead zone. - While the indicator is visible, further single taps in the same zone keep seeking (10s -> 20s -> 30s), like YouTube. - Gated by ChewieController.allowDoubleTapSeek (default true) with the step set by ChewieController.doubleTapSeekDuration (default 10s). Disabled on live streams and when playback is finished. Desktop (MaterialDesktopControls): - Double-clicking the video toggles fullscreen, like the YouTube desktop player. Gated by ChewieController.allowDoubleTapToggleFullScreen (default true) and allowFullScreen. The keyboard seek indicator state introduced by fluttercommunity#953 moves from _MaterialDesktopControlsState into a shared SeekIndicatorStateMixin so both control sets drive the same SeekIndicator. Double-tap-to-seek was first explored by punit1111 in fluttercommunity#717.
Mobile (MaterialControls): - Double-tapping the left/right side of the video seeks backward/forward, flashing the accumulating seek indicator. The middle band is a dead zone. - While the indicator is visible, further single taps in the same zone keep seeking (10s -> 20s -> 30s), like YouTube. - Gated by ChewieController.allowDoubleTapSeek (default true) with the step set by ChewieController.doubleTapSeekDuration (default 10s). Disabled on live streams and when playback is finished. Desktop (MaterialDesktopControls): - Double-clicking the video toggles fullscreen, like the YouTube desktop player. Gated by ChewieController.allowDoubleTapToggleFullScreen (default true) and allowFullScreen. The keyboard seek indicator state introduced by fluttercommunity#953 moves from _MaterialDesktopControlsState into a shared SeekIndicatorStateMixin so both control sets drive the same SeekIndicator. Double-tap-to-seek was first explored by punit1111 in fluttercommunity#717.
Mobile (MaterialControls): - Double-tapping the left/right side of the video seeks backward/forward, flashing the accumulating seek indicator. The middle band is a dead zone. - While the indicator is visible, further single taps in the same zone keep seeking (10s -> 20s -> 30s), like YouTube. - Gated by ChewieController.allowDoubleTapSeek (default true) with the step set by ChewieController.doubleTapSeekDuration (default 10s). Disabled on live streams and when playback is finished. Desktop (MaterialDesktopControls): - Double-clicking the video toggles fullscreen, like the YouTube desktop player. Gated by ChewieController.allowDoubleTapToggleFullScreen (default true) and allowFullScreen. The keyboard seek indicator state introduced by fluttercommunity#953 moves from _MaterialDesktopControlsState into a shared SeekIndicatorStateMixin so both control sets drive the same SeekIndicator. Double-tap-to-seek was first explored by punit1111 in fluttercommunity#717.
Mobile (MaterialControls): - Double-tapping the left/right side of the video seeks backward/forward, flashing the accumulating seek indicator. The middle band is a dead zone. - While the indicator is visible, further single taps in the same zone keep seeking (10s -> 20s -> 30s), like YouTube. - Gated by ChewieController.allowDoubleTapSeek (default true) with the step set by ChewieController.doubleTapSeekDuration (default 10s). Disabled on live streams and when playback is finished. Desktop (MaterialDesktopControls): - Double-clicking the video toggles fullscreen, like the YouTube desktop player. Gated by ChewieController.allowDoubleTapToggleFullScreen (default true) and allowFullScreen. The keyboard seek indicator state introduced by fluttercommunity#953 moves from _MaterialDesktopControlsState into a shared SeekIndicatorStateMixin so both control sets drive the same SeekIndicator. Double-tap-to-seek was first explored by punit1111 in fluttercommunity#717.
are there other controls? #715