Skip to content
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

feat(android): font variation settings #44667

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

matinzd
Copy link
Contributor

@matinzd matinzd commented May 23, 2024

Summary

Continuation of the old PR: #36371

This PR adds font variation settings feature for variable fonts in react native. The fontVariationSettings property provides low-level control over variable font characteristics, by specifying the four letter axis names of the characteristics you want to vary, along with their values.

Registered axes are the most commonly encountered — common enough that the authors of the specification felt they were worth standardizing. Note that this doesn't mean that the author has to include all of these in their font.

Custom axes can be anything the font designer wants to vary in their font, for example ascender or descender heights, the size of serifs, or anything else they can imagine. Any axis can be used as long as it is given a unique 4-character axis. Some will end up becoming more common, and may even become registered over time.

Example

<Text
  style={{
    fontFamily: 'MaterialSymbolsSharp',
    fontVariationSettings: "'wght' 100, 'FILL' 0",
  }}>
  {String.fromCodePoint(59576)}
</Text>

Font characteristics set using fontVariationSettings will always override those set using the corresponding basic font properties, e.g. fontWeight regardless of the order.

References

Android Documentation
iOS Documentation
MDN Documentation
Specification

Screenshots

Paper Fabric

This is a part of the other work for iOS that is being done by @davebcn87.

Changelog

[ANDROID] [ADDED] - Add fontVariationSettings support for variable font in Android

Test Plan

  • Ensure functionality on Paper (old architecture)
  • Ensure functionality on Fabric (new architecture)
  • Test custom axes for specific font families
  • Verify that registered axes ('wght', 'wdth', 'slnt', 'ital', 'opsz') work if supported by the font

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label May 23, 2024
Copy link

github-actions bot commented May 23, 2024

Fails
🚫

📋 Missing Changelog - Please add a Changelog to your PR description. See Changelog format

Generated by 🚫 dangerJS against 424323b

@analysis-bot
Copy link

analysis-bot commented May 23, 2024

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 23,234,793 +2,791,000
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 26,433,219 +2,790,748
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: 4c6bff0
Branch: main

@matinzd matinzd marked this pull request as ready for review May 23, 2024 20:45
@facebook-github-bot facebook-github-bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label May 23, 2024
@matinzd
Copy link
Contributor Author

matinzd commented May 23, 2024

Not sure why the bot still says the changelog is missing :)

https://regex101.com/r/hcLY01/1

Copy link
Contributor

@NickGerleman NickGerleman left a comment

Choose a reason for hiding this comment

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

This seems pretty sane to me, though we need to update the logic around Span management in ReactEditText a bit, for the new fields added to CustomStyleSpan.

Could we make sure to validate against Paper and Fabric, for both Text and TextInput?

@matinzd matinzd force-pushed the feat/font_variation_settings_android branch from e63e994 to 9dd41dd Compare May 30, 2024 07:28
@matinzd matinzd force-pushed the feat/font_variation_settings_android branch from 8549425 to afba1de Compare June 6, 2024 10:21
@matinzd
Copy link
Contributor Author

matinzd commented Jun 6, 2024

I updated some stuff regarding the edit text for paper but unfortunetaly it's not working. What am I missing here? @NickGerleman

@matinzd matinzd requested a review from NickGerleman June 6, 2024 10:43
@NickGerleman
Copy link
Contributor

I updated some stuff regarding the edit text for paper but unfortunetaly it's not working. What am I missing here? @NickGerleman

Hard to tell without debugging. Does prop setter for ShadowNode get hit?

@matinzd
Copy link
Contributor Author

matinzd commented Jun 12, 2024

I debugged it last week using a debugger, and it was being hit. I will do some more testing.

@matinzd matinzd force-pushed the feat/font_variation_settings_android branch from afba1de to 424323b Compare June 17, 2024 18:21
@matinzd
Copy link
Contributor Author

matinzd commented Jun 17, 2024

Screenshot 2024-06-17 at 20 45 28

This is old arch but still nothing is being applied for text input.

Screenshot 2024-06-17 at 20 46 50

@NickGerleman

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.

4 participants