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

Add StyleSheet.setStyleAttributePreprocessor #11138

Closed

Conversation

brentvatne
Copy link
Collaborator

@brentvatne brentvatne commented Nov 25, 2016

Motivation

On Exponent we load fonts dynamically and assign their native names by appending a session id, so that fonts from one Exponent "experience" do not clash with each other. So, before sending the fontFamily to native, we want to change it to the Exponent-scoped fontFamily.

Example:

// Before rendering your app
StyleSheet.setStyleAttributePreprocessor('fontFamily', _processFontFamily);

function _processFontFamily(name) {
  // Pass system fonts through
  if (!name || Constants.systemFonts.indexOf(name) >= 0) {
    return name;
  }

  if (!Font.isLoaded(name)) {
    if (__DEV__) {
      console.error(`${name} is not a system font and has not been loaded through Exponent.Font.loadAsync. If you intended to use a system font, make sure you typed the name correctly and that it is supported by the current operating system. If this is a custom font, be sure to load it with Exponent.Font.loadAsync`);
    } else {
      return 'system';
    }
  }

  return `ExponentFont-${Constants.sessionId}-${name}`;
}

See the tests for another example which always converts fontFamily to Wingdings.

I marked this function as experimental because it seems possible that the attribute API will change, and because I built this specifically for the above use case and others may need to expand the API to work in their case -- I did not attempt to anticipate how others would use it.

Test plan

Run npm test. This does not impact any app that does not use the API.

@facebook-github-bot
Copy link
Contributor

By analyzing the blame information on this pull request, we identified @sahrens and @ide to be potential reviewers.

@facebook-github-bot facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Nov 25, 2016
@mkonicek
Copy link
Contributor

mkonicek commented Nov 26, 2016

One thing I didn't understand from the description: Are there use cases for this when not using Exponent? Could the code live in Exponent only? Why add it to RN?

@brentvatne
Copy link
Collaborator Author

@mkonicek - it's possible that some people might want to do this too, for example in our discussion around CRNA it has been brought up that we want the new project to support other clients. Other clients would likely need to do the same thing for their font apis.

@mkonicek
Copy link
Contributor

👍 Thanks for the explanation Brent!

@facebook-github-bot shipit

@facebook-github-bot facebook-github-bot added GH Review: accepted Import Started This pull request has been imported. This does not imply the PR has been approved. and removed GH Review: review-needed labels Nov 29, 2016
@facebook-github-bot
Copy link
Contributor

@mkonicek has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

ide pushed a commit to expo/react-native that referenced this pull request Nov 30, 2016
Summary:
**Motivation**

On Exponent we load fonts dynamically and assign their native names by appending a session id, so that fonts from one Exponent "experience" do not clash with each other. So, before sending the `fontFamily` to native, we want to change it to the Exponent-scoped `fontFamily`.

Example:

```js
// Before rendering your app
StyleSheet.setStyleAttributePreprocessor('fontFamily', _processFontFamily);

function _processFontFamily(name) {
  // Pass system fonts through
  if (!name || Constants.systemFonts.indexOf(name) >= 0) {
    return name;
  }

  if (!Font.isLoaded(name)) {
    if (__DEV__) {
      console.error(`${name} is not a system font and has not been loaded through Exponent.Font.loadAsync. If you intended to use a system font, make sure you typed the name correctly and that it is supported by the current operating system. If this is a custom font, be sure to load it with Exponent.Font.loadAsync`);
    } else {
      return 'system';
    }
  }

  return `ExponentFont-
Closes facebook#11138

Differential Revision: D4245518

Pulled By: mkonicek

fbshipit-source-id: bd2452b1129d6675aa7b88e41351f8bb61fa20a3
robclouth pushed a commit to robclouth/react-native that referenced this pull request Dec 7, 2016
Summary:
**Motivation**

On Exponent we load fonts dynamically and assign their native names by appending a session id, so that fonts from one Exponent "experience" do not clash with each other. So, before sending the `fontFamily` to native, we want to change it to the Exponent-scoped `fontFamily`.

Example:

```js
// Before rendering your app
StyleSheet.setStyleAttributePreprocessor('fontFamily', _processFontFamily);

function _processFontFamily(name) {
  // Pass system fonts through
  if (!name || Constants.systemFonts.indexOf(name) >= 0) {
    return name;
  }

  if (!Font.isLoaded(name)) {
    if (__DEV__) {
      console.error(`${name} is not a system font and has not been loaded through Exponent.Font.loadAsync. If you intended to use a system font, make sure you typed the name correctly and that it is supported by the current operating system. If this is a custom font, be sure to load it with Exponent.Font.loadAsync`);
    } else {
      return 'system';
    }
  }

  return `ExponentFont-
Closes facebook#11138

Differential Revision: D4245518

Pulled By: mkonicek

fbshipit-source-id: bd2452b1129d6675aa7b88e41351f8bb61fa20a3
ide pushed a commit to expo/react-native that referenced this pull request Dec 29, 2016
Summary:
**Motivation**

On Exponent we load fonts dynamically and assign their native names by appending a session id, so that fonts from one Exponent "experience" do not clash with each other. So, before sending the `fontFamily` to native, we want to change it to the Exponent-scoped `fontFamily`.

Example:

```js
// Before rendering your app
StyleSheet.setStyleAttributePreprocessor('fontFamily', _processFontFamily);

function _processFontFamily(name) {
  // Pass system fonts through
  if (!name || Constants.systemFonts.indexOf(name) >= 0) {
    return name;
  }

  if (!Font.isLoaded(name)) {
    if (__DEV__) {
      console.error(`${name} is not a system font and has not been loaded through Exponent.Font.loadAsync. If you intended to use a system font, make sure you typed the name correctly and that it is supported by the current operating system. If this is a custom font, be sure to load it with Exponent.Font.loadAsync`);
    } else {
      return 'system';
    }
  }

  return `ExponentFont-
Closes facebook#11138

Differential Revision: D4245518

Pulled By: mkonicek

fbshipit-source-id: bd2452b1129d6675aa7b88e41351f8bb61fa20a3
DanielMSchmidt pushed a commit to DanielMSchmidt/react-native that referenced this pull request Jan 4, 2017
Summary:
**Motivation**

On Exponent we load fonts dynamically and assign their native names by appending a session id, so that fonts from one Exponent "experience" do not clash with each other. So, before sending the `fontFamily` to native, we want to change it to the Exponent-scoped `fontFamily`.

Example:

```js
// Before rendering your app
StyleSheet.setStyleAttributePreprocessor('fontFamily', _processFontFamily);

function _processFontFamily(name) {
  // Pass system fonts through
  if (!name || Constants.systemFonts.indexOf(name) >= 0) {
    return name;
  }

  if (!Font.isLoaded(name)) {
    if (__DEV__) {
      console.error(`${name} is not a system font and has not been loaded through Exponent.Font.loadAsync. If you intended to use a system font, make sure you typed the name correctly and that it is supported by the current operating system. If this is a custom font, be sure to load it with Exponent.Font.loadAsync`);
    } else {
      return 'system';
    }
  }

  return `ExponentFont-
Closes facebook#11138

Differential Revision: D4245518

Pulled By: mkonicek

fbshipit-source-id: bd2452b1129d6675aa7b88e41351f8bb61fa20a3
@brentvatne brentvatne deleted the @brentvatne/style-preprocessor branch May 13, 2019 18:58
facebook-github-bot pushed a commit that referenced this pull request Aug 25, 2022
Summary:
from the original design of `StyleSheet.setStyleAttributePreprocessor()` in #11138, the overwriting warning shows when the existing preprocess is be overwritten.  the behavior changes from 33b385825c72. This PR revises the logic back to original design.

## Changelog

[Internal] [Fixed] - Show warning only when overwriting existing preprocessor in `StyleSheet.setStyleAttributePreprocessor()`

Pull Request resolved: #34479

Test Plan:
Unit Test

```
 PASS  Libraries/StyleSheet/__tests__/StyleSheet-test.js
  setStyleAttributePreprocessor
    ✓ should not show warning when set preprocessor first time (2 ms)
    ✓ should show warning when overwrite the preprocessor (1 ms)
```

Reviewed By: dmitryrykun

Differential Revision: D38940676

Pulled By: cipolleschi

fbshipit-source-id: 80cf30fff62f4a02c17f7f42b3260a6011d5fc82
raykle pushed a commit to raykle/react-native that referenced this pull request Aug 27, 2022
Summary:
from the original design of `StyleSheet.setStyleAttributePreprocessor()` in facebook#11138, the overwriting warning shows when the existing preprocess is be overwritten.  the behavior changes from facebook@33b385825c72. This PR revises the logic back to original design.

## Changelog

[Internal] [Fixed] - Show warning only when overwriting existing preprocessor in `StyleSheet.setStyleAttributePreprocessor()`

Pull Request resolved: facebook#34479

Test Plan:
Unit Test

```
 PASS  Libraries/StyleSheet/__tests__/StyleSheet-test.js
  setStyleAttributePreprocessor
    ✓ should not show warning when set preprocessor first time (2 ms)
    ✓ should show warning when overwrite the preprocessor (1 ms)
```

Reviewed By: dmitryrykun

Differential Revision: D38940676

Pulled By: cipolleschi

fbshipit-source-id: 80cf30fff62f4a02c17f7f42b3260a6011d5fc82
dmytrorykun pushed a commit that referenced this pull request Sep 14, 2022
Summary:
from the original design of `StyleSheet.setStyleAttributePreprocessor()` in #11138, the overwriting warning shows when the existing preprocess is be overwritten.  the behavior changes from 33b385825c72. This PR revises the logic back to original design.

## Changelog

[Internal] [Fixed] - Show warning only when overwriting existing preprocessor in `StyleSheet.setStyleAttributePreprocessor()`

Pull Request resolved: #34479

Test Plan:
Unit Test

```
 PASS  Libraries/StyleSheet/__tests__/StyleSheet-test.js
  setStyleAttributePreprocessor
    ✓ should not show warning when set preprocessor first time (2 ms)
    ✓ should show warning when overwrite the preprocessor (1 ms)
```

Reviewed By: dmitryrykun

Differential Revision: D38940676

Pulled By: cipolleschi

fbshipit-source-id: 80cf30fff62f4a02c17f7f42b3260a6011d5fc82
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. Import Started This pull request has been imported. This does not imply the PR has been approved.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants