Skip to content

feat(ios): support condensed system font on fabric #52259

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
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ismarbesic
Copy link

Summary:

This PR adds support for using the condensed system font on iOS when passing "SystemCondensed" as fontFamily. This behavior existed in the old architecture but was never ported to the new one, see RCTFont.mm as reference. Fixes #52258.

Changelog:

[IOS] [ADDED] - Add support for condensed system font when using the new react native architecture.

Test Plan:

Before:

After:

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Jun 25, 2025
@javache
Copy link
Member

javache commented Jun 25, 2025

I see isCondensedFont is used in a few more places in the legacy arch implementation (we should really look at unforking this!) - do we need to merge those too?

@ismarbesic
Copy link
Author

ismarbesic commented Jun 25, 2025

I see isCondensedFont is used in a few more places in the legacy arch implementation (we should really look at unforking this!) - do we need to merge those too?

Good question :). The one I linked to above is crucial to have, to set the condensed font descriptor trait when font family equals to SystemCondensed. Regarding the other cases, I am not really convinced we need those.

This check seems to run even if the previous block was able to find a font (since didFindFont is not updated properly) - I assume it's wrong? The isCondensed variable here won't have an effect if my previous statement is valid. The last check here is the one that might make sense to port if the second if-block is supposed to run, otherwise the value would never have changed. Using Avenir Next Condensed for example works without these changes but the system one doesn't.

@ismarbesic
Copy link
Author

Thank you @javache for the initial look at this PR. Could you let me know what the next steps are?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Condensed system font is broken on iOS in the new architecture
3 participants