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.

Sign up for GitHub

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

Jump to bottom

fix: RankingFloatingActionButton partially off-screen for some translations #5117

Merged
merged 10 commits into from
May 14, 2024
Merged

fix: RankingFloatingActionButton partially off-screen for some translations #5117

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
efeef68
Bug: wrapping RankingFloatingActionButton inside a Expanded widget an…
Mar 13, 2024
53f2d7b
Merge branch 'develop' into bug/ranking_float_action_button_overflow
Mar 13, 2024
d854a60
fix: Adding AutoSizeText to FloatingActionButton and adding it to a F…
Mar 28, 2024
fca916e
fix: returning RankingFloatingActionButton class comments
Mar 28, 2024
7e97946
refactor: Replacing FloatingActionButton for ElevatedButton inside Ra…
May 7, 2024
f775dbd
fix: Limiting BackToTopButton height to MINIMUM_TOUCH_SIZE and adjust…
May 7, 2024
78a5eed
Merge branch 'openfoodfacts:develop' into bug/ranking_float_action_bu…
gmmoraes May 7, 2024
263a015
fix: Changed the property of a widget from conditional to fixed valu…
May 13, 2024
7a44eb8
fix: Refactor widget structure from Row to Container
May 13, 2024
72eef5d
Merge branch 'openfoodfacts:develop' into bug/ranking_float_action_bu…
gmmoraes May 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
34 changes: 20 additions & 14 deletions packages/smooth_app/lib/pages/product/common/product_query_page.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,12 +156,9 @@ class _ProductQueryPageState extends State<ProductQueryPage>
SmoothSharedAnimationController(
child: SmoothScaffold(
floatingActionButton: Row(
mainAxisAlignment: _showBackToTopButton
? MainAxisAlignment.spaceBetween
: MainAxisAlignment.center,
mainAxisAlignment: MainAxisAlignment.end,
children: <Widget>[
Padding(
padding: const EdgeInsets.symmetric(vertical: 5.0),
Expanded(
child: RankingFloatingActionButton(
onPressed: () => Navigator.push<Widget>(
context,
Expand All @@ -187,15 +184,24 @@ class _ProductQueryPageState extends State<ProductQueryPage>
padding: const EdgeInsetsDirectional.only(
start: SMALL_SPACE,
),
child: FloatingActionButton(
backgroundColor: themeData.colorScheme.secondary,
onPressed: () {
_scrollToTop();
},
tooltip: appLocalizations.go_back_to_top,
child: Icon(
Icons.arrow_upward,
color: themeData.colorScheme.onSecondary,
child: SizedBox(
height: MINIMUM_TOUCH_SIZE,
child: ElevatedButton(
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess IconButton would make more sense and the code would be less verbose.
Would you please try it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess IconButton would make more sense and the code would be less verbose. Would you please try it?

If we do use the IconButton, I believe it will need to be embedded inside an InkResponse widget for a circular ripple effect and inside a Material widget for background and elevation.

Perhaps something like this:

    ClipOval(
       child: Material(
         elevation: 2.0,
         color: themeData.colorScheme.secondary,
         child: InkResponse(
           containedInkWell: true,
           child: SizedBox(
             height: MINIMUM_TOUCH_SIZE,
             child: IconButton(
               icon: const Icon(
                 Icons.arrow_upward,
               ),
               color: themeData.colorScheme.onSecondary,
               onPressed: onPressed,
             ),
           ),
         ),
       ),
     );

Perhaps it could be added inside a private method that returns a widget:

Widget _getBackToTopButton(ThemeData themeData, void Function()? onPressed) =>
      ClipOval(
        child: Material(
          elevation: 2.0,
          color: themeData.colorScheme.secondary,
          child: InkResponse(
            containedInkWell: true,
            child: SizedBox(
              height: MINIMUM_TOUCH_SIZE,
              child: IconButton(
                icon: const Icon(
                  Icons.arrow_upward,
                ),
                color: themeData.colorScheme.onSecondary,
                onPressed: onPressed,
              ),
            ),
          ),
        ),
      );

What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

@gmmoraes My assumption was that it was easy to use (and to maintain) an IconButton in order to display a clickable disk with an icon inside. Easier than an ElevatedButton.
If it's not the case, don't waste time with the hard to read/maintain solution you've just suggested, and ignore my suggestion about using an IconButton.

onPressed: () {
_scrollToTop();
},
style: ElevatedButton.styleFrom(
backgroundColor: themeData.colorScheme.secondary,
foregroundColor: themeData.colorScheme.onSecondary,
shape: const CircleBorder(),
padding: EdgeInsets.zero,
),
child: Center(
child: Icon(
Icons.arrow_upward,
color: themeData.colorScheme.onSecondary,
),
),
),
),
),
Expand Down
38 changes: 25 additions & 13 deletions packages/smooth_app/lib/widgets/ranking_floating_action_button.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import 'dart:math';

import 'package:auto_size_text/auto_size_text.dart';
import 'package:flutter/material.dart';
import 'package:flutter_gen/gen_l10n/app_localizations.dart';
import 'package:smooth_app/generic_lib/animations/smooth_reveal_animation.dart';
import 'package:smooth_app/generic_lib/design_constants.dart';

// TODO(monsieurtanuki): we should probably remove that class to avoid confusion with the "compare" button
/// Floating Action Button dedicated to Personal Ranking
Expand All @@ -19,19 +19,31 @@ class RankingFloatingActionButton extends StatelessWidget {
Widget build(BuildContext context) => SmoothRevealAnimation(
animationCurve: Curves.easeInOutBack,
startOffset: const Offset(0.0, 1.0),
child: Row(
mainAxisSize: MainAxisSize.max,
mainAxisAlignment: MainAxisAlignment.center,
children: <Widget>[
SizedBox(width: MediaQuery.of(context).size.width * 0.09),
FloatingActionButton.extended(
heroTag: 'ranking_fab_${Random(100)}',
elevation: 12.0,
icon: const Icon(rankingIconData),
label: Text(AppLocalizations.of(context).myPersonalizedRanking),
child: Container(
height: MINIMUM_TOUCH_SIZE,
margin:
EdgeInsets.only(left: MediaQuery.of(context).size.width * 0.09),
alignment: Alignment.center,
child: SizedBox(
height: MINIMUM_TOUCH_SIZE,
child: ElevatedButton.icon(
onPressed: onPressed,
style: ButtonStyle(
shape: MaterialStateProperty.all<RoundedRectangleBorder>(
const RoundedRectangleBorder(
borderRadius: CIRCULAR_BORDER_RADIUS,
),
),
),
icon: const Icon(rankingIconData),
label: AutoSizeText(
AppLocalizations.of(context).myPersonalizedRanking,
textAlign: TextAlign.center,
maxLines: 2,
overflow: TextOverflow.ellipsis,
),
),
],
),
),
);
}