material-foundation / flutter-packages

A collection of useful Material Design packages
https://pub.dev/publishers/material.io
Apache License 2.0
834 stars 157 forks source link

Bug with font weight when using GoogleFonts.xxxTextTheme? #35

Open aliceblock opened 4 years ago

aliceblock commented 4 years ago

I can use only 2 font weight if I set it as textTheme.

Code

import 'package:flutter/material.dart';
import 'package:google_fonts/google_fonts.dart';

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

class MyApp extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      theme: ThemeData(
        textTheme: GoogleFonts.mitrTextTheme(),
      ),
      home: Scaffold(
        body: SafeArea(
          child: Container(
            constraints: BoxConstraints.expand(),
            child: Row(
              mainAxisAlignment: MainAxisAlignment.center,
              children: <Widget>[
                Column(
                  children: <Widget>[
                    Text('w100',
                        style: GoogleFonts.mitr(
                            fontSize: 18.0, fontWeight: FontWeight.w100)),
                    Text('w200',
                        style: GoogleFonts.mitr(
                            fontSize: 18.0, fontWeight: FontWeight.w200)),
                    Text('w300',
                        style: GoogleFonts.mitr(
                            fontSize: 18.0, fontWeight: FontWeight.w300)),
                    Text('w400',
                        style: GoogleFonts.mitr(
                            fontSize: 18.0, fontWeight: FontWeight.w400)),
                    Text('w500',
                        style: GoogleFonts.mitr(
                            fontSize: 18.0, fontWeight: FontWeight.w500)),
                    Text('w600',
                        style: GoogleFonts.mitr(
                            fontSize: 18.0, fontWeight: FontWeight.w600)),
                    Text('w700',
                        style: GoogleFonts.mitr(
                            fontSize: 18.0, fontWeight: FontWeight.w700)),
                    Text('w800',
                        style: GoogleFonts.mitr(
                            fontSize: 18.0, fontWeight: FontWeight.w800)),
                    Text('w900',
                        style: GoogleFonts.mitr(
                            fontSize: 18.0, fontWeight: FontWeight.w900)),
                  ],
                ),
                SizedBox(
                  width: 20.0,
                ),
                Column(
                  children: <Widget>[
                    Text('w100',
                        style: TextStyle(
                            fontSize: 18.0, fontWeight: FontWeight.w100)),
                    Text('w200',
                        style: TextStyle(
                            fontSize: 18.0, fontWeight: FontWeight.w200)),
                    Text('w300',
                        style: TextStyle(
                            fontSize: 18.0, fontWeight: FontWeight.w300)),
                    Text('w400',
                        style: TextStyle(
                            fontSize: 18.0, fontWeight: FontWeight.w400)),
                    Text('w500',
                        style: TextStyle(
                            fontSize: 18.0, fontWeight: FontWeight.w500)),
                    Text('w600',
                        style: TextStyle(
                            fontSize: 18.0, fontWeight: FontWeight.w600)),
                    Text('w700',
                        style: TextStyle(
                            fontSize: 18.0, fontWeight: FontWeight.w700)),
                    Text('w800',
                        style: TextStyle(
                            fontSize: 18.0, fontWeight: FontWeight.w800)),
                    Text('w900',
                        style: TextStyle(
                            fontSize: 18.0, fontWeight: FontWeight.w900)),
                  ],
                )
              ],
            ),
          ),
        ),
      ),
    );
  }
}

Result

Left side: set it as textStyle Right side: set it as textTheme Screenshot_1577205739

Thibault2ss commented 4 years ago

I have the same

aliceblock commented 4 years ago

They set only 'xxx_Regular' font when using xxxTextTheme. So I have to set text style separately if I want to use other font weight.

johnsonmh commented 4 years ago

Thanks for filing this.

For now, our advice is to prefer using the first column's technique when you're writing one-off text styles. The reason you're seeing what you're seeing is that in order to pick the correct font file, we need to know the specific fontWeight and fontStyle from the TextStyle.

The xxxTextTheme methods were designed to be used for people constructing TextThemes where every TextStyle has it's own weight and style, then throughout the app, you'd only need to call Text('foo', style: Theme.of(context).textTheme.body2) for example.

We're working on ways to make using TextTheme more ergonomic, but it's tricky given the constraint I mentioned above.

IsaiahByDayah commented 4 years ago

Oh man, I was seriously sitting here thinking I was going crazy because I could only see 2 different font weights regardless of what value I set (with the xxxTextTheme approach). Going to try the more explicit approach tonight.

@johnsonmh Is this something a beginner contributor could help with? I'm really loving Flutter and would love to help give back in some way

adamhalesworth commented 4 years ago

I was confused by this too. Perhaps the docs just need updating to make this more explicit?

tomasbaran commented 4 years ago

Same happening here. I have found a way around this—instead of setting the font weight in the fontWeightproperty, do it in the fontFamily property like this:

fontFamily: GoogleFonts.encodeSansExpanded(fontWeight: FontWeight.w200).fontFamily,

pedrovilela commented 4 years ago

Same problem here

johnsonmh commented 4 years ago

Thanks for the feedback everyone, right now we do not have concrete plans for adjusting the API on this. Again, the problem is that we need to know the font weight when you're requesting it from GoogleFonts in order to pick the right file.

@tomasbaran solution accomplishes this, but similarly you could use your first approach, and make sure you pass fontWeight to the GoogleFonts. method call. For example: GoogelFonts.mitr(fontWeight: FontWeight.w200).

If anyone has good ideas for how we can make the API for this more ergonomic, or how we might be able to document this better, we would be happy to review contributions!

tplkn commented 4 years ago

Looks like the only way to make it works across the app is manually specify fonts in pubspec. For example:

fonts:
  - family: RobotoSlab
    fonts:
      - asset: google_fonts/RobotoSlab-Thin.ttf
        weight: 100
      - asset: google_fonts/RobotoSlab-ExtraLight.ttf
        weight: 200
      - asset: google_fonts/RobotoSlab-Light.ttf
        weight: 300
      - asset: google_fonts/RobotoSlab-Regular.ttf
        weight: 400
      - asset: google_fonts/RobotoSlab-Medium.ttf
        weight: 500
      - asset: google_fonts/RobotoSlab-SemiBold.ttf
        weight: 600
      - asset: google_fonts/RobotoSlab-Bold.ttf
        weight: 700
      - asset: google_fonts/RobotoSlab-ExtraBold.ttf
        weight: 800
      - asset: google_fonts/RobotoSlab-Black.ttf
        weight: 900

and then use fontFamily: 'RobotoSlab' for the app theme. Even generated methods won't work. (fontFamily: GoogleFonts.robotoSlab().fontFamily,).

I guess, it's better to specify it in documentation. Right now, the most confusing part is

Note: Since these files are listed as assets, there is no need to list them in the fonts section of the pubspec.yaml. This can be done because the files are consistently named from the Google Fonts API (so be sure not to rename them!)
BunnyMan1 commented 3 years ago

My way of implementation:

// Setup
// NTheme.dart

class NTheme {
  static TextStyle fontWeightApplier(FontWeight fw, TextStyle ts) {
    return ts.merge(GoogleFonts.nunitoSans(
      fontWeight: fw,
    ));
  }

  static mainTheme(BuildContext context) {
    final textTheme = Theme.of(context).textTheme;
    return ThemeData(
      ...,
      textTheme: GoogleFonts.nunitoSansTextTheme(
        textTheme
            .apply(
              bodyColor: Colors.grey[800],
              displayColor: Colors.grey[800],
            ),
      ),
      ...,
    );
  }
}
// Usage Example
Text(
  data,
  style: NTheme.fontWeightApplier(
    FontWeight.w600,
    textTheme.bodyText1.copyWith(
      color: Colors.grey[800],
      fontSize: 18,
    ),
  ),
),
amake commented 2 years ago

I have a use case where I need regular, italic, bold, and bold+italic, but for various reasons it's not feasible for me to call a GoogleFonts method to grab the variant TextStyle every time; I need to be able to get real italic (not synthetic italic) by 1) setting the default text style to something like GoogleFonts.getFont('MyFont') and then 2) later doing DefaultTextStyle.of(context).copyWith(fontStyle: FontStyle.italic) at the point of use where I need italic.

The reason this doesn't work with this package as it is currently implemented is because each variant is explicitly loaded as a separate font here: https://github.com/material-foundation/google-fonts-flutter/blob/feb1f6da7603b1f4183f9fd0117b62f558631c9a/lib/src/google_fonts_base.dart#L178-L191

Thus you will always get synthetic bolds and italics in my use case.

The core challenge as I understand it is that in order to get the behavior I want (which I think most people would assume is how it should work already, but here we are), you need to eagerly download all variants together so that they can be loaded by the same FontLoader instance under a single family name. That goes against the design of this package, which seems to be aiming for maximal laziness, i.e. no variant should be downloaded unless the user explicitly requests it.

In my case I know that I want regular, italic, bold, and bold+italic, and I want to eagerly load them at whatever cost.

I maintain a modified version of this package called dynamic_fonts which is basically this package but instead of pulling from Google Fonts you can pull down your own privately hosted fonts (like from S3 or whatever). There, I am squaring this circle by introducing a flag that allows for optional eager loading when registering a font. It looks like this: https://github.com/amake/dynamic-fonts-flutter/commit/1a5c09d08c143e74ff8824f15ef0a0fe49d12657

(There is no equivalent to DynamicFonts.register in GoogleFonts; instead I guess googleFontsTextStyle and anything that forwards to it would have to gain an eager flag, if you were to copy my approach.)

Of course in Google Fonts you have the issue that some fonts provide a huge number of variants, and users may only want to eagerly load a subset of them. That's an API challenge that I don't have: I assume that the user will only register variants that he or she wants to use, so eager loads everything.

caseycrogers commented 1 year ago

Can we at least make the package throw with an informative error if the developer tries to access a font family that isn't dynamically available? Making the package actually work would be ideal, but absent that it should at least throw with an informative error message instead of working improperly.

guidezpl commented 1 year ago

Thanks for the feedback everyone, right now we do not have concrete plans for adjusting the API on this. Again, the problem is that we need to know the font weight when you're requesting it from GoogleFonts in order to pick the right file.

@tomasbaran solution accomplishes this, but similarly you could use your first approach, and make sure you pass fontWeight to the GoogleFonts. method call. For example: GoogelFonts.mitr(fontWeight: FontWeight.w200).

If anyone has good ideas for how we can make the API for this more ergonomic, or how we might be able to document this better, we would be happy to review contributions!

This is our current thinking, happy to welcome contributions!

iampato commented 1 year ago

Any working solution

rubenferreira97 commented 1 year ago

This workaround prevents Text widgets from being declared as const 😕

const Text(
  'Hi',
  style: TextStyle(
    fontSize: 16,
    fontWeight: FontWeight.w500,
  ),
);
Text(
  'Hi',
  style: GoogleFonts.mitr(
    fontSize: 16,
    fontWeight: FontWeight.w500,
  ),
);

Which at first does not seems a big of a deal but if your whole widget was const, we now need multiple const declarations to keep sub widgets const.

lukepighetti commented 7 months ago

This is a terrible wont fix. Let's do the user journey

  1. member wants to try out some fonts, installs google_fonts, sets the material text theme to GoogleFonts.interTextTheme()
  2. member only gets w400 and w700, all other weights not working
  3. member spends an hour trying to find a solution, finds out the best option is to add fonts to the asset folder and include them normally

This is clearly broken and in practice means google_fonts has very little value

guidezpl commented 7 months ago

Agreed this is not ideal. If you would like to improve this, please see https://github.com/material-foundation/flutter-packages/blob/main/CONTRIBUTING.md

aikins01 commented 6 months ago

Interesting, this issue is still here, let me try it

estebangarcia21 commented 5 months ago

I have devised a workaround that I am currently using. I have not tested it on all fonts.

TextStyle mergeGoogleFontWeight(FontWeight fw, TextStyle ts) {
  // Insert spaces before capital letters and remove any leading spaces. Ex: 'WorkSans' -> 'Work Sans'
  final fontBaseName = ts.fontFamily!
      .split('_')[0]
      .replaceAllMapped(
        RegExp(r'(?<=[a-z])(?=[A-Z])'),
        (Match match) => ' ',
      )
      .trim();

  final googleFont = GoogleFonts.getFont(
    fontBaseName,
    fontWeight: fw,
  );

  return ts.merge(googleFont);
}

I have some base styles such as

class Styles {
  static TextStyle ui = GoogleFonts.workSans(
    fontWeight: FontWeight.normal,
    fontSize: 14,
  );
}

and I can merge the font weight while applying new styles:

Text(
  ...,
  style: mergeGoogleFontWeight(
    FontWeight.w600,
    Styles.ui.copyWith(
      color: Colors.black,
    ),
  ),
)
darajava commented 1 month ago

fonts:

  • family: RobotoSlab fonts:
    • asset: google_fonts/RobotoSlab-Thin.ttf weight: 100
    • asset: google_fonts/RobotoSlab-ExtraLight.ttf weight: 200
    • asset: google_fonts/RobotoSlab-Light.ttf weight: 300
    • asset: google_fonts/RobotoSlab-Regular.ttf weight: 400
    • asset: google_fonts/RobotoSlab-Medium.ttf weight: 500
    • asset: google_fonts/RobotoSlab-SemiBold.ttf weight: 600
    • asset: google_fonts/RobotoSlab-Bold.ttf weight: 700
    • asset: google_fonts/RobotoSlab-ExtraBold.ttf weight: 800
    • asset: google_fonts/RobotoSlab-Black.ttf weight: 900

Yes, this is a bit more effort, but the only reliable way to use these fonts.

xortrike commented 2 weeks ago

I have the same problem for Android. Interestingly, it works on the virtual device but not on the real device. I tried both options and neither works on the real device.

I also tried adding another font via pubspec.yaml and it works. So. It seems that the problem is precisely in this package fonts.

Screenshot

Package: google_fonts 6.2.1

Flutter 3.24.0 • channel stable • https://github.com/flutter/flutter.git Framework • revision 80c2e84975 (6 weeks ago) • 2024-07-30 23:06:49 +0700 Engine • revision b8800d88be Tools • Dart 3.5.0 • DevTools 2.37.2