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

Move cancelRequest_ in move ctor in ImageRequest #37221

Closed
wants to merge 2 commits into from

Conversation

sammy-SC
Copy link
Contributor

@sammy-SC sammy-SC commented May 3, 2023

Summary:
changelog: [internal]

Add missing cancelRequest_ std::move.

Differential Revision: D45524704

@analysis-bot
Copy link

analysis-bot commented May 3, 2023

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,628,584 -6
android hermes armeabi-v7a 7,940,679 -3
android hermes x86 9,115,571 -6
android hermes x86_64 8,970,183 -5
android jsc arm64-v8a 9,192,767 -4
android jsc armeabi-v7a 8,382,319 -4
android jsc x86 9,251,105 -3
android jsc x86_64 9,509,352 -3

Base commit: 5eabbd7
Branch: main

@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. p: Facebook Partner: Facebook Partner labels May 3, 2023
j-piasecki and others added 2 commits May 3, 2023 07:30
…tecture (facebook#37109)

Summary:
Currently, when `fontFamily` style is set to a specific font instead of a font family, [that specific font is used](https://github.com/facebook/react-native/blob/2058da8f2012578c3e82f1af19c3248346655f9a/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/RCTFontUtils.mm#L126) to display the text on iOS when using the new architecture. This is different behavior to the old architecture, where the font family and [font properties were extracted from the specified](https://github.com/facebook/react-native/blob/2058da8f2012578c3e82f1af19c3248346655f9a/packages/react-native/React/Views/RCTFont.mm#L450-L457) font and overridden if not provided by the user.

## Changelog:

[IOS] [FIXED] - Make font resolution work when using specific font name on the new architecture

Pull Request resolved: facebook#37109

Test Plan:
You can verify the problem on a simple snippet:
```jsx
import React from 'react';
import {SafeAreaView, Text} from 'react-native';

function App() {
  return (
    <SafeAreaView style={{flex: 1}}>
      <Text
        style={{
          fontFamily: 'Helvetica Light Oblique',
          fontWeight: 'bold',
          fontStyle: 'normal',
        }}>
        Some random text
      </Text>
    </SafeAreaView>
  );
}

export default App;
```

<details>
<summary>
Here's before & after
</summary>

Without changes from this PR:
<img src="https://user-images.githubusercontent.com/21055725/234618852-07cbe67c-f534-4b04-b760-828f4edef549.png" width=400 />

With changes from this PR:
<img src="https://user-images.githubusercontent.com/21055725/234618902-9e44a389-8f27-4ab0-95dc-e34ca781d2ed.png" width=400 />

</details>

Differential Revision: D45351185

Pulled By: sammy-SC

fbshipit-source-id: 09b17b75d58f2cfe82dc755424632ab8cc6da55e
Summary:
changelog: [internal]

Add missing cancelRequest_ std::move.

Reviewed By: javache, cortinico

Differential Revision: D45524704

fbshipit-source-id: 54e6cb8749c7a5229463d888aec943c0e34af3c6
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D45524704

sammy-SC added a commit to sammy-SC/react-native that referenced this pull request May 3, 2023
Summary:
Pull Request resolved: facebook#37221

changelog: [internal]

Add missing cancelRequest_ std::move.

Reviewed By: javache, cortinico

Differential Revision: D45524704

fbshipit-source-id: 269674caf09ba0d58b3457af629fc27b080c124c
sammy-SC added a commit to sammy-SC/react-native that referenced this pull request May 3, 2023
Summary:
Pull Request resolved: facebook#37221

changelog: [internal]

Add missing cancelRequest_ std::move.

Reviewed By: javache, cortinico

Differential Revision: D45524704

fbshipit-source-id: 097abe789ab5ab578ed639ac8657968fe53d26ce
sammy-SC added a commit to sammy-SC/react-native that referenced this pull request May 3, 2023
Summary:
Pull Request resolved: facebook#37221

changelog: [internal]

Add missing cancelRequest_ std::move.

Reviewed By: javache, cortinico

Differential Revision: D45524704

fbshipit-source-id: 513ad009dbae2fc6fbe3a3a10414961a2dec5021
sammy-SC added a commit to sammy-SC/react-native that referenced this pull request May 3, 2023
Summary:
Pull Request resolved: facebook#37221

changelog: [internal]

Add missing cancelRequest_ std::move.

Reviewed By: javache, cortinico

Differential Revision: D45524704

fbshipit-source-id: b3bb30a0accc44d116470de9e57b3f8b70f738ee
sammy-SC added a commit to sammy-SC/react-native that referenced this pull request May 4, 2023
Summary:
Pull Request resolved: facebook#37221

changelog: [internal]

Add missing cancelRequest_ std::move.

Reviewed By: javache, cortinico

Differential Revision: D45524704

fbshipit-source-id: 0cc5c30c2696be058306e08f137fd90288578a38
sammy-SC added a commit to sammy-SC/react-native that referenced this pull request May 4, 2023
Summary:
Pull Request resolved: facebook#37221

changelog: [internal]

Add missing cancelRequest_ std::move.

Reviewed By: javache, cortinico

Differential Revision: D45524704

fbshipit-source-id: e492ba4763f66977c8407b09ad838f7f028cff4e
@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label May 4, 2023
@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 9132d7a.

jeongshin pushed a commit to jeongshin/react-native that referenced this pull request May 7, 2023
Summary:
Pull Request resolved: facebook#37221

changelog: [internal]

Add missing cancelRequest_ std::move.

bypass-github-export-checks

Reviewed By: javache, cortinico

Differential Revision: D45524704

fbshipit-source-id: 1dd0d627549dab353872654d78f7b59d1b2d7174
kelset pushed a commit that referenced this pull request May 10, 2023
Summary:
Pull Request resolved: #37221

changelog: [internal]

Add missing cancelRequest_ std::move.

bypass-github-export-checks

Reviewed By: javache, cortinico

Differential Revision: D45524704

fbshipit-source-id: 1dd0d627549dab353872654d78f7b59d1b2d7174
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. Merged This PR has been merged. p: Facebook Partner: Facebook Partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants