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

No hot reloading when change font scale #45655

Open
yzhe554 opened this issue Jul 24, 2024 · 6 comments
Open

No hot reloading when change font scale #45655

yzhe554 opened this issue Jul 24, 2024 · 6 comments
Labels
Issue: Author Provided Repro This issue can be reproduced in Snack or an attached project. Type: New Architecture Issues and PRs related to new architecture (Fabric/Turbo Modules)

Comments

@yzhe554
Copy link

yzhe554 commented Jul 24, 2024

Description

In old arch, we can change font scale in realtime with command option +/-.
In new arch, it has to be refreshed manually with pressing R to get new font scale applied.

Steps to reproduce

  1. gh repo clone yzhe554/new-arch-font-scale
  2. cd new-arch-font-scale
  3. npm i
  4. npm run ios
  5. change font scale with option command +/-

React Native Version

0.74.3

Affected Platforms

Runtime - iOS

Areas

Fabric - The New Renderer

Output of npx react-native info

System:
  OS: macOS 14.5
  CPU: (10) arm64 Apple M1 Max
  Memory: 107.00 MB / 32.00 GB
  Shell:
    version: "5.9"
    path: /bin/zsh
Binaries:
  Node:
    version: 18.18.0
    path: ~/.nvm/versions/node/v18.18.0/bin/node
  Yarn:
    version: 1.22.18
    path: /usr/local/bin/yarn
  npm:
    version: 9.8.1
    path: ~/.nvm/versions/node/v18.18.0/bin/npm
  Watchman:
    version: 2024.05.06.00
    path: /opt/homebrew/bin/watchman
Managers:
  CocoaPods:
    version: 1.15.2
    path: /opt/homebrew/bin/pod
SDKs:
  iOS SDK:
    Platforms:
      - DriverKit 23.5
      - iOS 17.5
      - macOS 14.5
      - tvOS 17.5
      - visionOS 1.2
      - watchOS 10.5
  Android SDK: Not Found
IDEs:
  Android Studio: 2023.2 AI-232.10300.40.2321.11567975
  Xcode:
    version: 15.4/15F31d
    path: /usr/bin/xcodebuild
Languages:
  Java:
    version: 17.0.9
    path: /usr/bin/javac
  Ruby:
    version: 2.7.6
    path: /Users/yuzheng/.rbenv/shims/ruby
npmPackages:
  "@react-native-community/cli": Not Found
  react:
    installed: 18.2.0
    wanted: 18.2.0
  react-native:
    installed: 0.74.3
    wanted: 0.74.3
  react-native-macos: Not Found
npmGlobalPackages:
  "*react-native*": Not Found
Android:
  hermesEnabled: Not found
  newArchEnabled: Not found
iOS:
  hermesEnabled: true
  newArchEnabled: true

Stacktrace or Logs

N/A

Reproducer

https://github.com/yzhe554/new-arch-font-scale

Screenshots and Videos

In Old Arch

Screen.Recording.2024-07-23.at.4.55.31.PM.mov

In New Arch

Screen.Recording.2024-07-25.at.9.34.58.AM.mov
@yzhe554 yzhe554 added Needs: Triage 🔍 Type: New Architecture Issues and PRs related to new architecture (Fabric/Turbo Modules) labels Jul 24, 2024
@cortinico cortinico added Issue: Author Provided Repro This issue can be reproduced in Snack or an attached project. and removed Needs: Triage 🔍 labels Jul 25, 2024
@jpudysz
Copy link

jpudysz commented Jul 29, 2024

Hey @cortinico, there is one more side effect connected to this issue. I'm the author of react-native-unistyles, and Unistyles automatically re-renders the view on different events like font scale changes. In the new architecture, there is different behavior, and the text is cut off at the edges. It's not initially visible because the user needs to reset the app as mentioned above, but with Unistyles, it's visible after the first re-render.

Here is link to the unistyles issue: jpudysz/react-native-unistyles#256
Repro: as above linked by @yzhe554

Minimal steps to reproduce:

export default function App() {
    const [counter, setCounter] = useState(0)
    
    return (
        <View 
            style={{
              flex: 1,
              backgroundColor: '#fff',
              alignItems: 'center',
              justifyContent: 'center',
            }}
        >
          <Text style={{fontSize: 24}}>RN 24 Label Testing!!!</Text>
          <Text style={{fontSize: 18}}>RN  18 Label Testing!!!</Text>
          <Text style={{fontSize: 16}}>RN 16 Label Testing!!!</Text>
          <Button onPress={() => setCounter(prevState => prevState + 1)} title="Re-render" />
          <StatusBar style="auto" />
        </View>
    )
}

Try to change font scale and then press re-render button.

Result for new architecture:

Screen.Recording.2024-07-29.at.10.49.33.mov

Old arch:

Screen.Recording.2024-07-29.at.10.52.41.mov

@cipolleschi
Copy link
Contributor

Hi there! These are actually two separate issues. So, let's tackle them separatedly.

First, @yzhe554 thanks for reporting the issue.
I looked into it and discussed it with some colleagues that have more context.

This is a known limitation, as of today, because in the New Architecture we don't have a good mechanism to notify React that all the nodes received the updated text multiplier. It is also an edge case, as in production it is very rare that a user go in the settings to change the preferred text size and then it returns to the app. Furthermore, the issue solves itself if you navigate to another screen or you do a state update that force a re-render

We are exploring how to implement some features that can be used as foundations to fix this one as well, so the fix to this will wait until that thread of work is done.


Then, @jpudysz, this is a different issue. I'd love if you can do a test for me.
What happens if the user:

  • change the text
  • press on re-render
  • reload the app from Metro (r in the simulator)

The reason I ask is that my gut feeling is that this scenario will "fix" your issue. Text multiplier affects layout as well and, from the video, it looks like that the rerender from unistyle is not updating the layout correctly: the left margin in the New Architecture is unchanged, while, in the old architecture, the left margin shrinks while the font increase its size.

@yzhe554
Copy link
Author

yzhe554 commented Aug 1, 2024

@cipolleschi Thanks for the quick feedback.
I am working on an enterprise level project that is trying to introduce RN as a brownfield project. This would be a key point whether we can go with new arch. If you have a rough timeline on the fix, it will be really helpful for us to plan.

@jpudysz
Copy link

jpudysz commented Aug 1, 2024

hey @cipolleschi , I will create a separate issue with repro and all the details

@cipolleschi
Copy link
Contributor

@yzhe554 we don't have a rough timeline as of today, unfortunately.
Is it a use case you face often, of people going to the setting page, come back to the app and expect to see the app updated?

@yzhe554
Copy link
Author

yzhe554 commented Aug 8, 2024

@yzhe554 we don't have a rough timeline as of today, unfortunately. Is it a use case you face often, of people going to the setting page, come back to the app and expect to see the app updated?

Nah, I agree it's an edge case. But just need to convince other team(non tech). I will follow this for now. Thanks again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue: Author Provided Repro This issue can be reproduced in Snack or an attached project. Type: New Architecture Issues and PRs related to new architecture (Fabric/Turbo Modules)
Projects
None yet
Development

No branches or pull requests

4 participants