Skip to content

text: Bold text is sometimes way bolder than intended #500

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

Open
chrisbobbe opened this issue Feb 1, 2024 · 9 comments
Open

text: Bold text is sometimes way bolder than intended #500

chrisbobbe opened this issue Feb 1, 2024 · 9 comments
Labels
a-design Visual and UX design beta feedback Things beta users have specifically asked for upstream Would benefit from work in Flutter or another upstream

Comments

@chrisbobbe
Copy link
Collaborator

chrisbobbe commented Feb 1, 2024

See how bold the sender names and topic headers are in this screenshot on the office Android device, compared to a better alternative:

Current Better alternative
image image

The "better alternative" can be reached in this case by commenting out a line in weightVariableTextStyle:

diff --git lib/widgets/text.dart lib/widgets/text.dart
index 1a80cee4b..04d001bbd 100644
--- lib/widgets/text.dart
+++ lib/widgets/text.dart
@@ -69,7 +69,7 @@ TextStyle weightVariableTextStyle(BuildContext? context, {
     // This use of `fontWeight` shouldn't affect glyphs in the preferred,
     // "wght"-axis font. If it does, see for debugging:
     //   https://github.com/zulip/zulip-flutter/issues/65#issuecomment-1550666764
-    fontWeight: clampVariableFontWeight(value),
+    // fontWeight: clampVariableFontWeight(value),
 
     inherit: true);
 }

It seems that the comment about "This use of fontWeight shouldn't [etc.]" has proven helpful: the use of fontWeight is indeed affecting glyphs in Source Sans 3. The "see for debugging" link is relevant, but I plan to replace it with a link to an upstream Flutter issue with the triaged-engine label: that's flutter/flutter#136779 .

Removing that fontWeight line would be sad; it exists for the sake of glyphs that can't be rendered in the primary font family (like Source Sans 3) and might need to be rendered in a fallback font that doesn't have a "wght" axis.

@chrisbobbe
Copy link
Collaborator Author

We've seen this on Greg's Android device, and (as screenshotted above) on the office Android device. I don't think we've yet had reports from iOS.

@chrisbobbe chrisbobbe changed the title text: Some bold text is way bolder than intended text: Bold text is sometimes way bolder than intended Feb 1, 2024
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue Feb 1, 2024
@gnprice
Copy link
Member

gnprice commented Feb 2, 2024

Thanks for spotting this. I'd actually noticed that the recipient headers and sender names seemed bolder than I'd expect; but our source code seemed to clearly call for the same weights that Vlad specified in the design, so I figured that was just his design choice. Apparently not!

I don't think we've yet had reports from iOS.

(And in particular it doesn't seem to reproduce on your iPhone.)

On that upstream issue flutter/flutter#136779, it looks like the next step is to work out what the desired behavior should be when both a fontWeight and a wght variation are specified. So probably the next step for this issue is for me to try to move that upstream issue forward, by writing a comment that tries to answer that question.

@gnprice gnprice added the a-design Visual and UX design label Feb 2, 2024
@gnprice gnprice added this to the Beta 2 milestone Feb 2, 2024
@gnprice gnprice self-assigned this Feb 2, 2024
@chrisbobbe
Copy link
Collaborator Author

Sounds good. Thank you!!

chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue Feb 2, 2024
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue Feb 2, 2024
@gnprice gnprice added the beta feedback Things beta users have specifically asked for label Feb 9, 2024
@gnprice
Copy link
Member

gnprice commented Feb 9, 2024

@gnprice
Copy link
Member

gnprice commented Feb 16, 2024

OK, I've posted a proposed solution on that upstream thread, starting at flutter/flutter#136779 (comment) . I'll follow up with whatever discussion that leads to.

@gnprice
Copy link
Member

gnprice commented May 9, 2024

No reply yet on that thread. I've now filed my proposed upstream solution as its own issue:

Partly that's prompted by seeing there was another report of the same underlying problem. (In that report, the issue was that Flutter's own Material implementation doesn't work right on variable fonts, because it sets fontWeight but not a wght variation.)

Perhaps the new writeup will make it easier to see what my actual proposal is, as opposed to the reasoning leading up to it, and perhaps it'll help spur someone to pick it up and implement it. We'll see.

@chrisbobbe
Copy link
Collaborator Author

Thank you for filing that!!

@gnprice gnprice modified the milestones: B2: pre-summer, Launch May 9, 2024
@gnprice
Copy link
Member

gnprice commented Jul 31, 2024

I'm not reproducing this when I look now (on Android). Did we fix this along the way at some point?

@chrisbobbe
Copy link
Collaborator Author

I haven't made any code changes to fix this, no. Maybe something upstream?

@gnprice gnprice modified the milestones: Launch, Post-launch Nov 21, 2024
@gnprice gnprice removed their assignment Nov 21, 2024
@gnprice gnprice added the upstream Would benefit from work in Flutter or another upstream label Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-design Visual and UX design beta feedback Things beta users have specifically asked for upstream Would benefit from work in Flutter or another upstream
Projects
Status: No status
Development

No branches or pull requests

2 participants