Skip to content
This repository has been archived by the owner on Feb 25, 2024. It is now read-only.

Menu does not stand out from the background #179

Closed
jpnurmi opened this issue May 30, 2022 · 6 comments
Closed

Menu does not stand out from the background #179

jpnurmi opened this issue May 30, 2022 · 6 comments

Comments

@jpnurmi
Copy link
Member

jpnurmi commented May 30, 2022

Yaru Material
Screenshot from 2022-05-30 17-21-13 Screenshot from 2022-05-30 17-20-58
Screenshot from 2022-05-30 17-21-31 Screenshot from 2022-05-30 17-21-22
import 'package:flutter/material.dart';
import 'package:yaru/yaru.dart';

void main() {
  runApp(const MyApp());
}

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

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      theme: yaruDark,
      home: Scaffold(
        body: Builder(
          builder: (context) {
            return GestureDetector(
              onSecondaryTapDown: (details) {
                showMenu(
                  context: context,
                  position: RelativeRect.fromSize(
                    details.globalPosition & Size.zero,
                    MediaQuery.of(context).size,
                  ),
                  items: [
                    PopupMenuItem(
                      onTap: () {},
                      child: const Text('Foo'),
                    ),
                    PopupMenuItem(
                      onTap: () {},
                      child: const Text('Bar'),
                    ),
                    PopupMenuItem(
                      onTap: () {},
                      child: const Text('Baz'),
                    ),
                  ],
                );
              },
            );
          },
        ),
      ),
    );
  }
}
@jpnurmi
Copy link
Member Author

jpnurmi commented Jun 10, 2022

flutter/flutter#101017

@jpnurmi jpnurmi closed this as completed Jun 10, 2022
@rydmike
Copy link

rydmike commented Jun 30, 2022

@jpnurmi There are a bunch of issues with elevation when you set ThemeData.useMaterial3 to true. They all come from that Material does not get any elevation, when it is true, unless you also set its surfaceTint and shadowColor, if they are not set there is no elevation of used Material even if elevation is none zero.

For Material itself you can fix the issue by assigning these props for it, but it is big change in expected default behavior of Material over past M2 behavior. One might expect that default for Material in M3 would use the theme.colorScheme.shadowColor and surfaceTint colors for those props by default if not assigned when using Material 3, so that Material gets elevation by default when elevation is given. If it was so we would not see these issues during the transitional period for Material 3, even if the widgets do not yet implement the full M3 design.

Since it does not behave like this, this currently causes widgets like popup menu, most dialogs and drawer to get no elevation. Since you cannot access the underlaying Material props, via these widgets' props or themes, the only fix is to wrap such widgets with a new theme where Material3 is not being used.

This effectively renders opting in using Material3 in its current state, even for the widgets that are implemented quite a big mess, since getting elevation on dialogs, drawer and popup menu without this hassle is rather critical.

Can't recommend using Material3 opt-in on any channel in Flutter yet.

@jpnurmi
Copy link
Member Author

jpnurmi commented Jul 1, 2022

@Feichtmeier What do you think about switching back to M2 until M3 is in better shape?

@jpnurmi
Copy link
Member Author

jpnurmi commented Jul 1, 2022

@rydmike Thanks for chiming in, by the way. This is very valuable information, much appreciated!

@Feichtmeier
Copy link
Member

Apart from the context menu I really enjoy the M3 parts in yaru.dart as for example the appbar and bottombar color having the same color as the rest of the window so it does not look so alien and the tint colors in the cards :)
Also it looks like the issue might be soon tackled inside flutter/flutter so maybe we could get away by just waiting until this is fixed? I don't see any other issue in yaru.dart with m3

@rydmike
Copy link

rydmike commented Jul 1, 2022

@jpnurmi and @Feichtmeier, if you don't have so many of them, or just make a custom popup menu wrapper if you do, you can fix the none elevated popup / context menu by wrapping it in a copy of your theme with useMaterial3 set to false, that is then only used by the menu. The poupup menu wont get any M3 tint colored based elevation then, but is not doing that yet anyway since it is not implemented yet.

An example is shown here: flutter/flutter#105101
Although it does not use full copy of current theme, but you get the idea.

In this issue I discuss and explain this issue a bit more if it is of interest: rydmike/flex_color_scheme#54

I had been thinking of opening an issue where I would explain all the issues the current approach is causing during the transition for users that want use M3 already now, but seeing how it was shut down in other attempts I doubt it would have any success either. It is really Material that would have needed to implemented its M3 support a bit differently, then this issue could have been avoided during the transitional period too.

FlexColorScheme can also create M3 like inspired themes without using M3, so there not using it is an option.

However, I do agree with @Feichtmeier that if you are using it already, then M3 brings many other desirable designs by default, that are hard to drop once you have used them, so keeping it is probably what you prefer. If you do not have a drawer and many dialogs where the elevation is also an issue, then wrapping the context menu with a theme where M3 is false might be a nice temp workaround, that you can easily remove when it is fixed in the framework.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants