Skip to content

ui: For boring, non-emoji font, use one that iOS recognizes #290

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

Merged
merged 1 commit into from
Sep 1, 2023

Conversation

chrisbobbe
Copy link
Collaborator

It's strange that iOS really doesn't seem to know what to do with 'sans-serif' here. Nor "SF Pro", "SF-Pro", nor any obvious variation on that, even though it's supposedly the system font:
https://developer.apple.com/fonts/

But a GitHub comment popped up a few hours ago (linked in the added code), and that's the first thing I've found that seems to work, as a clear pointer to the default system font. Shrug.

Later, we'll want to use Sorce Sans 3 for almost all the text in the app. But we'll want to take more care with that than I have bandwidth for right now (around it being a variable-weight font); see discussion:
https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/flutter.3A.20text.20all.20weird/near/1633648

Fixes: #289

@chrisbobbe chrisbobbe added the a-iOS Issues specific to iOS, or requiring iOS-specific work label Aug 31, 2023
@chrisbobbe chrisbobbe requested a review from gnprice August 31, 2023 02:23
Copy link
Member

@gnprice gnprice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for debugging this! One comment below.

Also a commit-message nit: s/Sorce/Source/.

Would you file a separate issue for the variable-weight thing? That will help us to not forget about it.

fontFamilyFallback: [
// …since apparently iOS doesn't support 'sans-serif', use this instead:
// https://github.com/flutter/flutter/issues/63507#issuecomment-1698504425
if (Platform.isIOS) '.AppleSystemUIFont' else 'sans-serif',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be the Flutter "target platform", I think, rather than the actual current platform. Cf f669f16.

@chrisbobbe
Copy link
Collaborator Author

Thanks for the review! Revision pushed, and I'll go file that issue now.

@chrisbobbe
Copy link
Collaborator Author

#294

@gnprice
Copy link
Member

gnprice commented Sep 1, 2023

Thanks! Tweaked the link in the commit message to point to that issue #294 (which in turn points to the chat thread the commit message had linked to); merging.

It's strange that iOS really doesn't seem to know what to do with
'sans-serif' here. Nor "SF Pro", "SF-Pro", nor any obvious variation
on that, even though it's supposedly the system font:
  https://developer.apple.com/fonts/

But a GitHub comment popped up a few hours ago (linked in the added
code), and that's the first thing I've found that seems to work, as
a clear pointer to the default system font. Shrug.

Later, we'll want to use Source Sans 3 for almost all the text in the
app. But we'll want to take more care with that than I have
bandwidth for right now (around it being a variable-weight font);
see zulip#294.

Fixes: zulip#289
@gnprice gnprice merged commit b10296f into zulip:main Sep 1, 2023
@chrisbobbe chrisbobbe deleted the pr-ios-font-fix branch September 1, 2023 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-iOS Issues specific to iOS, or requiring iOS-specific work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

iOS: Non-emoji text wrongly being rendered with "Noto Color Emoji" font
2 participants