eBay / flutter_glove_box

Various eBay tools for Flutter development
BSD 3-Clause "New" or "Revised" License
317 stars 62 forks source link

Fonts not loading when new buttons (ElevatedButton, TextButton, OutlinedButton) are themed with TextStyle #103

Open minhqdao opened 3 years ago

minhqdao commented 3 years ago

When the new buttons (ElevatedButton, TextButton, OutlinedButton) are styled the recommended way using ThemeData, and TextStyle is added as a property, 'Roboto' is no longer loaded as the default font during golden tests. Instead, 'Ahem' is now displayed as a fallback. Steps to reproduce:

  1. git clone https://github.com/eBay/flutter_glove_box.git
  2. Add an ElevatedButton to the column in packages/golden_toolkit/example/lib/src/flutter_demo_page.dart like so:
children: <Widget>[
  ElevatedButton(onPressed: () {}, child: const Text('Test')),
  const Text(
    'You have pushed the button this many times:',
),
  1. Save and run flutter test --update-goldens to enjoy the ElevatedButton being rendered with text.
  2. Now let's add theme data to the button (here, we want to increase the default size of the button text):
  visualDensity: VisualDensity.adaptivePlatformDensity,
  elevatedButtonTheme: ElevatedButtonThemeData(
    style: ElevatedButton.styleFrom(
      textStyle: const TextStyle(fontSize: 20),
    ),
  ),
),
home: const _MyHomePage(title: 'Flutter Demo Home Page'),
  1. Save and run flutter test --update-goldens again to immediately drown in sadness over button text not being rendered.

If the code is compiled and executed normally, e.g. in the simulator, the button text is shown normally (with the expected larger font size).

As far as I can tell, there are three workarounds:

  1. Specify a fontFamily. If fontFamily: 'Roboto' is used, Roboto will be loaded during golden testing. However, the switch between Android and iOS fonts would no longer happen automatically. You therefore typically want to avoid hardcoding your font.
  2. Avoid specifying the textStyle parameter in ThemeData but use it in the Text widget whenever needed. That, of course, goes against the idea of using themes in general, but in fact is what we now do in our production code to correctly render goldens for our reviewers and not hardcode 'Roboto'.
  3. Specify fontFamily: DefaultTextStyle.of(context).style.fontFamily. This works in the example app but somehow failed in our production code, maybe because we have the text styles deeply nested in providers for each Android and iOS. It also requires context to be passed around.

You see, having loadAppFonts() loading the correct default font with the ability to specify TextStyle would be highly practical for us. 😋

coreysprague commented 3 years ago

@minhqdao Thanks for reporting this. I haven't had a chance to look into it, but will.

My initial suspicion though is that we are going to be out of luck. Unfortunately, when no font family is specified, the defaults are applied outside the context of flutter and are handled by SKIA itself. When running Flutter Test, the default font is set to Ahem with no ability to override.

All that being said, that information is based on on prior versions of Flutter. I haven't had a chance to dig through what changes landed with Flutter 2.0 and if by chance, we have a better injection point to hijack/intercept this behavior.

fredgrott commented 2 years ago

I can update a little since I am on the beta channeland do use he Goolge Fonts plugin.

Certain widgets under Goldens will still display the ahem stuff instead of the custom fonts. For example the app titlein the appbar.

My question would be is that using the new approach of text themes flowing through theme data and then a materialbased themedata for cupertinoapp?

quoc-huynh-cosee commented 1 year ago

Any updates on this? I'm having the same issue.

Ahmed-Omar-Hommir commented 1 year ago

I have the same issue

nilsreichardt commented 10 months ago

~The same issue applies for the "new" text themes, like bodyMedium, headlineSmall.~

Text(
  'This is my text',
  style: Theme.of(context).textTheme.bodyMedium,
),

UPDATE: I found my issue. I overwrote my Theme and haven't copied the base theme:

Before:

class _PageTheme extends StatelessWidget {
  const _PageTheme({
    required this.child,
  });

  final Widget child;

  @override
  Widget build(BuildContext context) {
    final baseTheme = Theme.of(context);
    return Theme(
      data: baseTheme.copyWith(
        textTheme: baseTheme.textTheme.copyWith(
          bodyMedium: TextStyle(
            color: Colors.grey[600],
          ),
        ),
      ),
      child: child,
    );
  }
}

After:

class _PageTheme extends StatelessWidget {
  const _PageTheme({
    required this.child,
  });

  final Widget child;

  @override
  Widget build(BuildContext context) {
    final baseTheme = Theme.of(context);
    return Theme(
      data: baseTheme.copyWith(
        textTheme: baseTheme.textTheme.copyWith(
          bodyMedium: baseTheme.textTheme.bodyMedium?.copyWith( // Use copyWith!
            color: Colors.grey[600],
          ),
        ),
      ),
      child: child,
    );
  }
}