Button overlay color does not adapt when it changes shape #114301

Activity

1. exaby73

   added

   in triagePresently being triaged by the triage teamPresently being triaged by the triage team

   on Oct 31, 2022
2. exaby73 commented on Oct 31, 2022

   

   exaby73

   on Oct 31, 2022

   Member

   More actions

   Triage report

   I can reproduce this issue on Master (3.5.0-10.0.pre.168)

   Code sample

   import 'package:flutter/material.dart';
   
   void main() => runApp(const MyApp());
   
   class MyApp extends StatelessWidget {
     const MyApp({super.key});
   
     @override
     Widget build(BuildContext context) {
       return const MaterialApp(
         home: MyWidget(),
       );
     }
   }
   
   class MyWidget extends StatelessWidget {
     const MyWidget({super.key});
   
     @override
     Widget build(BuildContext context) {
       return const Scaffold(
         body: Center(child: AnimatedElevatedButton()),
       );
     }
   }
   
   class AnimatedElevatedButton extends StatefulWidget {
     const AnimatedElevatedButton({super.key});
   
     @override
     State<AnimatedElevatedButton> createState() => _AnimatedElevatedButtonState();
   }
   
   class _AnimatedElevatedButtonState extends State<AnimatedElevatedButton>
       with SingleTickerProviderStateMixin {
     late final AnimationController _controller = AnimationController(
       lowerBound: 0,
       upperBound: 300,
       value: 300,
       duration: const Duration(seconds: 2),
       vsync: this,
     );
   
     @override
     Widget build(BuildContext context) {
       return ElevatedButtonTransition(
         radius: _controller,
         onPressed: () {},
         onHover: (value) => setState(() => _controller.value = value ? 80 : 150),
       );
     }
   
     @override
     void dispose() {
       _controller.dispose();
       super.dispose();
     }
   }
   
   class ElevatedButtonTransition extends AnimatedWidget {
     final void Function()? onPressed;
     final void Function(bool)? onHover;
   
     const ElevatedButtonTransition({
       super.key,
       this.onPressed,
       this.onHover,
       required AnimationController radius,
     }) : super(listenable: radius);
   
     Animation<double> get radius => listenable as Animation<double>;
   
     @override
     Widget build(BuildContext context) {
       ButtonStyle style = ButtonStyle(
         elevation: MaterialStateProperty.resolveWith<double?>(
           (Set<MaterialState> states) => 0,
         ),
         foregroundColor: MaterialStateProperty.resolveWith<Color?>(
           (Set<MaterialState> states) =>
               Theme.of(context).colorScheme.onSecondaryContainer,
         ),
         backgroundColor: MaterialStateProperty.resolveWith<Color?>(
           (Set<MaterialState> states) =>
               Theme.of(context).colorScheme.secondaryContainer,
         ),
         overlayColor: MaterialStateProperty.resolveWith<Color?>(
           (Set<MaterialState> states) =>
               states.contains(MaterialState.hovered) ? Colors.black26 : null,
         ),
         shape: MaterialStateProperty.resolveWith<OutlinedBorder?>(
           (Set<MaterialState> states) => RoundedRectangleBorder(
             borderRadius: BorderRadius.circular(radius.value),
           ),
         ),
       );
   
       return ElevatedButton(
         onPressed: onPressed,
         onHover: onHover,
         style: style,
         child: const Icon(Icons.favorite, size: 300),
       );
     }
   }
3. exaby73

   added

   frameworkflutter/packages/flutter repository. See also f: labels.flutter/packages/flutter repository. See also f: labels.

   p: material_uimaterial_ui package in flutter/packagesmaterial_ui package in flutter/packages

   has reproducible stepsThe issue has been confirmed reproducible and is ready to work onThe issue has been confirmed reproducible and is ready to work on

   found in release: 3.3Found to occur in 3.3Found to occur in 3.3

   found in release: 3.4Found to occur in 3.4Found to occur in 3.4

   and removed

   in triagePresently being triaged by the triage teamPresently being triaged by the triage team

   on Oct 31, 2022
4. HansMuller

   self-assigned this

   on Nov 3, 2022
5. HansMuller commented on Nov 3, 2022

   

   HansMuller

   on Nov 3, 2022

   Contributor

   More actions

   Not that it really matters, but. The code that creates the ButtonStyle could be written more simply

       final ColorScheme colorScheme = Theme.of(context).colorScheme;
       ButtonStyle style = ElevatedButton.styleFrom(
         elevation: 0,
         foregroundColor: colorScheme.onSecondaryContainer,
         backgroundColor: colorScheme.secondaryContainer,
         shape: RoundedRectangleBorder(borderRadius: BorderRadius.circular(radius.value)),
       ).copyWith(
         overlayColor: MaterialStateProperty.resolveWith<Color?>((Set<MaterialState> states) {
           return states.contains(MaterialState.hovered) ? Colors.black26 : null;
         },
       ));
6. HansMuller commented on Nov 3, 2022

   

   HansMuller

   on Nov 3, 2022

   Contributor

   More actions

   This appears to be a problem with InkHighlight. When it's rebuilt with a new customBorder it doesn't update its _highlights[type] table with a new InkHighlight.
7. HansMuller

   assigned

   bleroux

   and unassigned

   HansMuller

   on Nov 4, 2022
8. bleroux

   added this to Nevercodeon Nov 7, 2022
9. bleroux

   moved this to To do in Nevercodeon Nov 7, 2022
10. bleroux

    moved this from To do to In progress in Nevercodeon Nov 7, 2022
11. bleroux commented on Nov 18, 2022

    

    bleroux

    on Nov 18, 2022

    Last edited by bleroux

    Contributor

    More actions

    @HansMuller Thank you for the investigation on this interesting issue.

    I filed two PRs:

    • A first one to add some missing tests and fix a documentation error - Add more InkWell tests #115634
    • Another one to make the highlights updatable - InkResponse highlights can be updated #115635

    I split this in two PRs because the first one is independent from this issue and will be useful even if the second one is not merged.
    The second one might not fully fix this issue and it will require some discussions.

    An ElevatedButton is a ButtonStyleButton that uses a Material parent and an InkWell child.
    When the button shape is updated:

    • Without the fix, the Material receives a new shape and updates its rendering accordingly but the InkWell highlights are not updated.
    • With the fix, both the Material and the InkWell highlights are updated.

    When allowing highlights to be updated, there is a new problem : Material animates its shape change, InkWell highlights don’t.

    Using the provided code sample, it results in the highlight growing instantly (the grey rounded rectangle in the following video) while the Material is still expanding:

    highlight_expand_before.mp4

    A workaround is to update the code sample to set ElevatedButton clipBehavior:

    with_clipBehavior.mp4

    Updated code sample (simplified + setting ElevatedButton.clipBehavior + timeDilation = 10)

    import 'package:flutter/material.dart';
    import 'package:flutter/scheduler.dart';
    
    void main() {
      timeDilation = 10;
      runApp(const MyApp());
    }
    
    class MyApp extends StatelessWidget {
      const MyApp({super.key});
    
      @override
      Widget build(BuildContext context) {
        return const MaterialApp(
          home: MyWidget(),
        );
      }
    }
    
    class MyWidget extends StatelessWidget {
      const MyWidget({super.key});
    
      @override
      Widget build(BuildContext context) {
        return const Scaffold(
          body: Center(child: AnimatedElevatedButton()),
        );
      }
    }
    
    class AnimatedElevatedButton extends StatefulWidget {
      const AnimatedElevatedButton({super.key});
    
      @override
      State<AnimatedElevatedButton> createState() => _AnimatedElevatedButtonState();
    }
    
    class _AnimatedElevatedButtonState extends State<AnimatedElevatedButton> {
      double radius = 150;
    
      @override
      Widget build(BuildContext context) {
        return ElevatedButtonTransition(
          radius: radius,
          onPressed: () {},
          onHover: (value) => setState(() => radius = value ? 30 : 150),
        );
      }
    
      @override
      void dispose() {
        super.dispose();
      }
    }
    
    class ElevatedButtonTransition extends StatelessWidget {
      final void Function()? onPressed;
      final void Function(bool)? onHover;
      final double radius;
    
      const ElevatedButtonTransition({
        super.key,
        this.onPressed,
        this.onHover,
        required this.radius,
      });
    
      @override
      Widget build(BuildContext context) {
        final ColorScheme colorScheme = Theme.of(context).colorScheme;
        ButtonStyle style = ElevatedButton.styleFrom(
          elevation: 0,
          foregroundColor: colorScheme.onSecondaryContainer,
          backgroundColor: colorScheme.secondaryContainer,
          shape: RoundedRectangleBorder(borderRadius: BorderRadius.circular(radius)),
        ).copyWith(overlayColor: MaterialStateProperty.resolveWith<Color?>(
          (Set<MaterialState> states) {
            if (states.contains(MaterialState.pressed)) return Colors.red;
            if (states.contains(MaterialState.focused)) return Colors.green;
            return states.contains(MaterialState.hovered) ? Colors.black26 : null;
          },
        ));
    
        return ElevatedButton(
          onPressed: onPressed,
          onHover: onHover,
          style: style,
          clipBehavior: Clip.hardEdge,
          child: const Icon(Icons.favorite, size: 300),
        );
      }
    }
    

    @HansMuller, considering the animation problem, I’m very interested to get your insights on how we can proceed on this issue:

    1. Validate this change and emphasize the clipBehavior requirement (it will be imperfect because the clipBehavior works well when the shape expands not when it shrinks. For instance, with the provided code sample, the fix will be ok for the hover highlight, not with a focus highlight if we add one).
    2. Try to animate InkWell highlights customBorder change (it might be difficult to synchronize it with the Material animation, I don't know if it is possible to do so in a reliable way).
    3. Other directions?
12. auto-submit

    closed this as completedin #115635on Nov 18, 2022
13. Repository owner moved this from In progress to Done (PR merged) in Nevercodeon Nov 18, 2022
14. HansMuller commented on Nov 18, 2022

    

    HansMuller

    on Nov 18, 2022

    Contributor

    More actions

    Maybe ButtonStyleButton should create its Material with animationDuration=0? The fact that Material implicitly animates some of its properties can be really frustrating. I think what we want to do here is to effectively create a Material widget with the animation feature turned off.

    CC @goderbauer
15. exaby73

    added

    r: fixedIssue is closed as already fixed in a newer versionIssue is closed as already fixed in a newer version

    on Nov 21, 2022
16. github-actions commented on Dec 5, 2022

    

    github-actionsbot

    on Dec 5, 2022

    More actions

    This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.
17. github-actions

    locked as resolved and limited conversation to collaborators on Dec 5, 2022