-
Notifications
You must be signed in to change notification settings - Fork 1k
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
base: master
Are you sure you want to change the base?
Changes from 4 commits
ac80825
e34033c
a847292
70a5875
ab57910
ca7a04f
e9daf6b
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 |
---|---|---|
@@ -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, | ||
); | ||
} | ||
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. Do not use methods to return widgets. Instead you can simply add 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. earlier it was inside a Row children. it's extracted into method as changes requested #717 (comment) 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. @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 @punit1111 sorry for the extra work, but these widgets should ultimately be extracted as new Please let me know if this clears up things. 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. @diegotori 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. @punit1111 that is true, but instead of creating it from 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. Choose a reason for hiding this commentThe 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. |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
import 'package:flutter/material.dart'; | ||
|
||
class SeekRewindButton extends StatefulWidget { | ||
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 should be renamed to 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. |
||
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; | ||
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. If |
||
|
||
@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(); | ||
} | ||
} |
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.
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.
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