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

Buttons don't splash when hovered, turning black for one frame if 'unhovered'. #110266

Closed
Wissperwind opened this issue Aug 25, 2022 · 15 comments · Fixed by #111112
Closed

Buttons don't splash when hovered, turning black for one frame if 'unhovered'. #110266

Wissperwind opened this issue Aug 25, 2022 · 15 comments · Fixed by #111112
Assignees
Labels
a: desktop Running on desktop c: regression It was better in the past than it is now f: material design flutter/packages/flutter/material repository. found in release: 3.1 Found to occur in 3.1 framework flutter/packages/flutter repository. See also f: labels. has reproducible steps The issue has been confirmed reproducible and is ready to work on P2 Important issues not at the top of the work list r: fixed Issue is closed as already fixed in a newer version

Comments

@Wissperwind
Copy link

Wissperwind commented Aug 25, 2022

Hi,

I just updated flutter to the curret version (3.1.0-0.0.pre.2481) and checked my flutter web application if everything is still fine.
Unfortunately it is not. In the real application I experienced that the splash on a button click has some black glitches. So I hover, I click, it splashes, but the "outside area of the splash" is black. Or lets say a darker color than bevore the update. Can't tell for sure it is back.

I tried to produce a minimal working example but was not able to reproduce the exact behaviour.
But there are till some strange things. Here is my code:

import 'package:flutter/material.dart';


void main() => runApp(const MyApp());

class MyApp extends StatelessWidget {
  const MyApp({Key key}) : super(key: key);

  static const String _title = 'Flutter Code Sample';

  @override
  Widget build(BuildContext context) {

    return MaterialApp(
      title: _title,
      home: Scaffold(
        appBar: AppBar(title: const Text(_title)),
        body: const MyStatefulWidget(),
      ),
      //theme: themeSettings,
    );
  }
}

class MyStatefulWidget extends StatefulWidget {
  const MyStatefulWidget({Key key}) : super(key: key);

  @override
  State<MyStatefulWidget> createState() => _MyStatefulWidgetState();
}

class _MyStatefulWidgetState extends State<MyStatefulWidget> {

  bool inFocus = false;


  @override
  Widget build(BuildContext context) {
    ButtonStyle buttonStyle = ButtonStyle(
        overlayColor: MaterialStateProperty.resolveWith<Color>(
                (Set<MaterialState> states) {
              if (states.contains(MaterialState.focused)) {
                return Colors.white;
              }
              if (states.contains(MaterialState.hovered)) {
                return Colors.orange;
              }
              if (states.contains(MaterialState.pressed)) {
                return Colors.yellowAccent;
              }
              return null; // Defer to the widget's default.
            }),
    );


    return Center(
      child: Column(
        mainAxisSize: MainAxisSize.min,
        children: <Widget>[

          Container(
            color: Colors.blue,
              child: SizedBox(
                height: 100,
                width: 100,
                child: TextButton(
                  child: Text("Hi", style: TextStyle(color: Colors.black)),
                  onPressed: () {  },
                  style: buttonStyle,
                ),
              ),
          ),
        ],
      ),
    );
  }
}

So if I move the mouse on to the button the collor changes. Fine BUT:

  • If I move the mouse away from the button, the button color is black for one frame. This should not happen.
  • If I click the button, there is no splash. If I click the button and quickly move the mouse away from the button, I can see the rest of the splash animation. So somewhere there is a splash animation. But why there is no one if I leave the mouse on the button?

Maybe solving these problems solves also the problem in my main application.

@Wissperwind Wissperwind changed the title Buttons don' splash when hovered, or splash in black, also strange color changes. Buttons don't splash when hovered, turning black for one frame if 'unhovered'. Aug 25, 2022
@exaby73 exaby73 added the in triage Presently being triaged by the triage team label Aug 25, 2022
@exaby73
Copy link
Member

exaby73 commented Aug 25, 2022

I have migrated the code to null safety and I can reproduce this issue.

Reproducible on

  • Master (3.1.0-0.0.pre.2481)

Info

Screenshot
Code Sample
import 'package:flutter/material.dart';

void main() => runApp(const MyApp());

class MyApp extends StatelessWidget {
  const MyApp({Key? key}) : super(key: key);

  static const String _title = 'Flutter Code Sample';

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      title: _title,
      home: Scaffold(
        appBar: AppBar(title: const Text(_title)),
        body: const MyStatefulWidget(),
      ),
    );
  }
}

class MyStatefulWidget extends StatefulWidget {
  const MyStatefulWidget({Key? key}) : super(key: key);

  @override
  State<MyStatefulWidget> createState() => _MyStatefulWidgetState();
}

class _MyStatefulWidgetState extends State<MyStatefulWidget> {
  bool inFocus = false;

  @override
  Widget build(BuildContext context) {
    ButtonStyle buttonStyle = ButtonStyle(
      overlayColor: MaterialStateProperty.resolveWith<Color?>(
        (Set<MaterialState> states) {
          if (states.contains(MaterialState.focused)) {
            return Colors.white;
          }
          if (states.contains(MaterialState.hovered)) {
            return Colors.orange;
          }
          if (states.contains(MaterialState.pressed)) {
            return Colors.yellowAccent;
          }
          return null; // Defer to the widget's default.
        },
      ),
    );

    return Center(
      child: Column(
        mainAxisSize: MainAxisSize.min,
        children: <Widget>[
          Container(
            color: Colors.blue,
            child: SizedBox(
              height: 100,
              width: 100,
              child: TextButton(
                onPressed: () {},
                style: buttonStyle,
                child: const Text(
                  "Hi",
                  style: TextStyle(color: Colors.black),
                ),
              ),
            ),
          ),
        ],
      ),
    );
  }
}

Flutter Doctor

flutter doctor -v (Stable)
[✓] Flutter (Channel stable, 3.0.5, on Microsoft Windows [Version 10.0.22000.856], locale en-IN)
    • Flutter version 3.0.5 at C:\Users\Nabeel\fvm\versions\stable
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision f1875d570e (6 weeks ago), 2022-07-13 11:24:16 -0700
    • Engine revision e85ea0e79c
    • Dart version 2.17.6
    • DevTools version 2.12.2

[✗] Android toolchain - develop for Android devices
    ✗ Unable to locate Android SDK.
      Install Android Studio from: https://developer.android.com/studio/index.html
      On first launch it will assist you in installing the Android SDK components.
      (or visit https://flutter.dev/docs/get-started/install/windows#android-setup for detailed instructions).
      If the Android SDK has been installed to a custom location, please use
      `flutter config --android-sdk` to update to that location.


[✓] Chrome - develop for the web
    • CHROME_EXECUTABLE = C:\Program Files\BraveSoftware\Brave-Browser\Application\brave.exe

[✓] Visual Studio - develop for Windows (Visual Studio Community 2022 17.2.6)
    • Visual Studio at C:\Program Files\Microsoft Visual Studio\2022\Community
    • Visual Studio Community 2022 version 17.2.32630.192
    • Windows 10 SDK version 10.0.19041.0

[!] Android Studio (not installed)
    • Android Studio not found; download from https://developer.android.com/studio/index.html
      (or visit https://flutter.dev/docs/get-started/install/windows#android-setup for detailed instructions).

[✓] VS Code (version 1.70.2)
    • VS Code at C:\Users\Nabeel\AppData\Local\Programs\Microsoft VS Code
    • Flutter extension can be installed from:
      🔨 https://marketplace.visualstudio.com/items?itemName=Dart-Code.flutter

[✓] Connected device (3 available)
    • Windows (desktop) • windows • windows-x64    • Microsoft Windows [Version 10.0.22000.856]
    • Chrome (web)      • chrome  • web-javascript • unknown
    • Edge (web)        • edge    • web-javascript • Microsoft Edge 104.0.1293.63

[✓] HTTP Host Availability
    • All required HTTP hosts are available

! Doctor found issues in 2 categories.
flutter doctor -v (Master)
[✓] Flutter (Channel master, 3.1.0-0.0.pre.2481, on Microsoft Windows [Version 10.0.22000.856], locale en-IN)
    • Flutter version 3.1.0-0.0.pre.2481 on channel master at C:\Users\Nabeel\fvm\versions\master
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision 91fdbfa7c8 (4 hours ago), 2022-08-25 03:07:06 -0400
    • Engine revision 9600c6c39b
    • Dart version 2.19.0 (build 2.19.0-137.0.dev)
    • DevTools version 2.16.0

[✓] Android toolchain - develop for Android devices (Android SDK version 33.0.0)
    • Android SDK at C:\android-sdk
    • Platform android-33, build-tools 33.0.0
    • Java binary at: C:\Program Files\Java\jdk-17.0.4\bin\java
    • Java version Java(TM) SE Runtime Environment (build 17.0.4+11-LTS-179)
    • All Android licenses accepted.

[✓] Chrome - develop for the web
    • CHROME_EXECUTABLE = C:\Program Files\BraveSoftware\Brave-Browser\Application\brave.exe

[✓] Visual Studio - develop for Windows (Visual Studio Community 2022 17.2.6)
    • Visual Studio at C:\Program Files\Microsoft Visual Studio\2022\Community
    • Visual Studio Community 2022 version 17.2.32630.192
    • Windows 10 SDK version 10.0.19041.0

[!] Android Studio (not installed)
    • Android Studio not found; download from https://developer.android.com/studio/index.html
      (or visit https://flutter.dev/docs/get-started/install/windows#android-setup for detailed instructions).

[✓] VS Code (version 1.70.2)
    • VS Code at C:\Users\Nabeel\AppData\Local\Programs\Microsoft VS Code
    • Flutter extension can be installed from:
      🔨 https://marketplace.visualstudio.com/items?itemName=Dart-Code.flutter

[✓] Connected device (3 available)
    • Windows (desktop) • windows • windows-x64    • Microsoft Windows [Version 10.0.22000.856]
    • Chrome (web)      • chrome  • web-javascript • unknown
    • Edge (web)        • edge    • web-javascript • Microsoft Edge 104.0.1293.63

[✓] HTTP Host Availability
    • All required HTTP hosts are available

! Doctor found issues in 1 category.

@exaby73 exaby73 added c: regression It was better in the past than it is now framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. a: desktop Running on desktop has reproducible steps The issue has been confirmed reproducible and is ready to work on found in release: 3.1 Found to occur in 3.1 and removed in triage Presently being triaged by the triage team labels Aug 25, 2022
@jason-simmons
Copy link
Member

This is related to 180566f

@HansMuller

@HansMuller
Copy link
Contributor

The flash appears to be due the fact that the button's grey default hover color is inherited from ThemeData which defines it unconditionally. It looks like you can workaround the problem by having the ButtonStyle overlayColor return Colors.transparent instead of the null.

@exaby73
Copy link
Member

exaby73 commented Aug 26, 2022

@Wissperwind Is the solution proposed above suitable for your use case?

@HansMuller Is this intended or still considered a bug? If it's intended, would it make sense to document this behavior?

@exaby73 exaby73 added the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Aug 26, 2022
@bleroux
Copy link
Contributor

bleroux commented Aug 26, 2022

About the splash not showing, it is related to this change

final Color? resolvedOverlayColor = widget.overlayColor?.resolve(statesController.value);

When the InkWell is focused and pressed, statesController.value contains both MaterialState.hovered and MaterialState.pressed so if the resolveWith’s callback checks for MaterialState.hovered before MaterialState.pressed, the hover color is always returned.

While waiting for a fix in the framework, a workaround is to reorder the if statements in the resolveWith’s callback.

Updated code sample with splash workaround
import 'package:flutter/material.dart';

void main() => runApp(const MyApp());

class MyApp extends StatelessWidget {
  const MyApp({Key? key}) : super(key: key);

  static const String _title = 'Flutter Code Sample';

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      title: _title,
      home: Scaffold(
        appBar: AppBar(title: const Text(_title)),
        body: const MyStatefulWidget(),
      ),
    );
  }
}

class MyStatefulWidget extends StatefulWidget {
  const MyStatefulWidget({Key? key}) : super(key: key);

  @override
  State<MyStatefulWidget> createState() => _MyStatefulWidgetState();
}

class _MyStatefulWidgetState extends State<MyStatefulWidget> {
  bool inFocus = false;

  @override
  Widget build(BuildContext context) {
    ButtonStyle buttonStyle = ButtonStyle(
      overlayColor: MaterialStateProperty.resolveWith<Color?>(
        (Set<MaterialState> states) {
          if (states.contains(MaterialState.pressed)) {
            return Colors.yellowAccent;
          }
          if (states.contains(MaterialState.focused)) {
            return Colors.white;
          }
          if (states.contains(MaterialState.hovered)) {
            return Colors.orange;
          }
          return null; // Defer to the widget's default.
        },
      ),
    );

    return Center(
      child: Column(
        mainAxisSize: MainAxisSize.min,
        children: <Widget>[
          Container(
            color: Colors.blue,
            child: SizedBox(
              height: 100,
              width: 100,
              child: TextButton(
                onPressed: () {},
                style: buttonStyle,
                child: const Text(
                  "Hi",
                  style: TextStyle(color: Colors.black),
                ),
              ),
            ),
          ),
        ],
      ),
    );
  }
}

@Wissperwind
Copy link
Author

Changing the return null to a return Colors.transparent does not change anything as far as I observe.

@github-actions github-actions bot removed the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Aug 26, 2022
@HansMuller
Copy link
Contributor

@Wissperwind - this flaw isn't easy to see since hovering in and out of a brightly colored button creates a bit of an optical illusion around the button's color. I was watching the return value of getHighlightColorForType() (material/ink_well.dart) and it appeared that returning the default grey correlated with the flashing you've brought up.

Does @bleroux 's workaround work for you? #110266 (comment)

@exaby73 - this is still a bug.

@exaby73
Copy link
Member

exaby73 commented Aug 26, 2022

@HansMuller Thanks for the clarification :)

@HansMuller
Copy link
Contributor

@bleroux - (#110266 (comment)), the original test case did not show the pressed (yellow) overlay color because - as you pointed out - the button's state is {hovered, pressed} when it is pressed. There's no bug - the widget is WAI - although your version of the test makes more sense.

@Wissperwind - I'm not really sure what failure mode you're reporting. You said "I experienced that the flash on a button click has some black glitches" but you were "not able to reproduce the exact behaviour" (in a small test case). To fix the problem you've run into we really need a test case, a description of the failure mode, and the interaction needed to reproduce the problem.

@exaby73 exaby73 added the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Aug 30, 2022
@Wissperwind
Copy link
Author

Wissperwind commented Aug 30, 2022

Ok, let's just fix the problems that were reproducable in the code Example I provided. The one that someone migrated to null saftety:

  • If you move the mouse away from the button, the button is black for one frame. It should rather change from the hover color directly to the normal button color.
  • The splash animation is not displayed if you leave the mouse on the button. Only if you click and than remove the mouse fastly from the button, you can see the rest of the splash animation.

@github-actions github-actions bot removed the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Aug 30, 2022
@HansMuller
Copy link
Contributor

HansMuller commented Aug 31, 2022

If you move the mouse away from the button, the button is black for one frame. It should rather change from the hover color directly to the normal button color.

The test case below demos the problem. If you quickly slide the mouse in and out of the square on top there does appear to be a black flash in between frames. However the button on the bottom is configured in the same way, except that the background and overlay colors have alpha=0.25. In that case I don't see any black flash (or any flash at all). Is the black flash just an optical illusion?

hover test case
import 'package:flutter/material.dart';

class Home extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return Scaffold(
      body: Center(
        child: Column(
          mainAxisSize: MainAxisSize.min,
          children: <Widget>[
            Container(
              color: Colors.blue,
              child: SizedBox(
                height: 100,
                width: 100,
                child: TextButton(
                  style: ButtonStyle(
                    overlayColor: MaterialStateProperty.resolveWith<Color?>(
                      (Set<MaterialState> states) {
                        if (states.contains(MaterialState.hovered)) {
                          return Colors.orange;
                        }
                        return Colors.transparent;
                      },
                    ),
                  ),
                  onPressed: () {},
                  child: const Text('Hello', style: TextStyle(color: Colors.black)),
                ),
              ),
            ),
            SizedBox(height: 16),
            Container(
              color: Colors.blue.withOpacity(0.25),
              child: SizedBox(
                height: 100,
                width: 100,
                child: TextButton(
                  style: ButtonStyle(
                    overlayColor: MaterialStateProperty.resolveWith<Color?>(
                      (Set<MaterialState> states) {
                        if (states.contains(MaterialState.hovered)) {
                          return Colors.orange.withOpacity(0.25);
                        }
                        return Colors.transparent;
                      }
                    ),
                  ),
                  onPressed: () {},
                  child: const Text('Hello', style: TextStyle(color: Colors.black)),
                ),
              ),
            ),
          ],
        ),
      ),
    );
  }
}

void main() {
  runApp(MaterialApp(home: Home()));
}

@Wissperwind
Copy link
Author

No, it is not. I will post a video tomorrow morning.
Try on web. I don't know why there is a desktop lapble on this issue. I said I am working on web. If you start chrome from android studio it renders slower than in production and you can clearly see a black flash. I'll do a highspeed video with my phone tomorrow and post it here.

@HansMuller
Copy link
Contributor

I'd appreciate that. I've tried running the test case on Chrome/macOS and if there's a flash it's not obvious.

@HansMuller
Copy link
Contributor

Here's a version of the test case with timeDilation increased from 1 to 10. In this case it's much clearer that there is an unexpected overlay color transition. The problem reproduces equally well on macOS and web.

hover test case with timeDilation=10
import 'package:flutter/material.dart';
import 'package:flutter/scheduler.dart' show timeDilation;

class Home extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return Scaffold(
      body: Center(
        child: Column(
          mainAxisSize: MainAxisSize.min,
          children: <Widget>[
            Container(
              color: Colors.blue,
              child: SizedBox(
                height: 100,
                width: 100,
                child: TextButton(
                  style: ButtonStyle(
                    overlayColor: MaterialStateProperty.resolveWith<Color?>(
                      (Set<MaterialState> states) {
                        if (states.contains(MaterialState.hovered)) {
                          return Colors.orange;
                        }
                        return null;
                      },
                    ),
                  ),
                  onPressed: () {},
                  child: const Text('Hello', style: TextStyle(color: Colors.black)),
                ),
              ),
            ),
            SizedBox(height: 16),
            Container(
              color: Colors.blue.withOpacity(0.25),
              child: SizedBox(
                height: 100,
                width: 100,
                child: TextButton(
                  style: ButtonStyle(
                    overlayColor: MaterialStateProperty.resolveWith<Color?>(
                      (Set<MaterialState> states) {
                        if (states.contains(MaterialState.hovered)) {
                          return Colors.orange.withOpacity(0.25);
                        }
                        return null;
                      }
                    ),
                  ),
                  onPressed: () {},
                  child: const Text('Hello', style: TextStyle(color: Colors.black)),
                ),
              ),
            ),
          ],
        ),
      ),
    );
  }
}

void main() {
  timeDilation = 10;
  runApp(MaterialApp(home: Home()));
}

@HansMuller HansMuller self-assigned this Sep 1, 2022
@gspencergoog gspencergoog added the P2 Important issues not at the top of the work list label Sep 1, 2022
@exaby73 exaby73 added the r: fixed Issue is closed as already fixed in a newer version label Sep 8, 2022
@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a: desktop Running on desktop c: regression It was better in the past than it is now f: material design flutter/packages/flutter/material repository. found in release: 3.1 Found to occur in 3.1 framework flutter/packages/flutter repository. See also f: labels. has reproducible steps The issue has been confirmed reproducible and is ready to work on P2 Important issues not at the top of the work list r: fixed Issue is closed as already fixed in a newer version
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants