-
Notifications
You must be signed in to change notification settings - Fork 24.6k
Fix changing font scale breaking text #45978
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
Conversation
packages/react-native/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp
Outdated
Show resolved
Hide resolved
packages/react-native/ReactCommon/react/renderer/mounting/ShadowTree.cpp
Outdated
Show resolved
Hide resolved
packages/react-native/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp
Outdated
Show resolved
Hide resolved
packages/react-native/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp
Outdated
Show resolved
Hide resolved
packages/react-native/ReactCommon/react/renderer/mounting/ShadowTree.cpp
Outdated
Show resolved
Hide resolved
7c19db4
to
257a501
Compare
d72c6d3
to
861b708
Compare
@coado has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
2a2976d
to
8a015d4
Compare
@j-piasecki has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Summary: Fixes facebook#45857 The general idea behind this PR is the same for both platforms: dirty all nodes with `MeasurableYogaNode` trait when the layout is constrained with a new `fontSizeMultiplier`. There were a few caveats: - `ParagraphShadowNode` marks its layout as clean in the constructor in most cases. To prevent that from using a stale measurement, I've added a `fontSizeMultiplier_` field in the node that keeps track of the font scale it was last laid out with. That value is then compared with the scale used to create the attributed string kept in the node's state. If those differ, the layout is not cleared. - On Android, font scale wasn't passed down to the `SurfaceHandler` - On Android, text measurement relies on cached `DisplayMetrics` which were not updated when the system font scale changed. - `AndroidTextInputShadowNode` wasn't using `fontSizeMultiplier` at all. I needed to add it in all places where an `AttributedString` is constructed. - When the `fontSizeMultiplier` is changed, the entire `ShadowTree` is cloned. I'm not sure if there's a reliable way of determining whether the node is mutable or not. - Changing font scale and navigating back to the app on Android has the potential to cause a `SIGSEGV` but it also happens without changes in this PR. ## Changelog: [GENERAL] [FIXED] - Fixed text not updating correctly after changing font scale in settings Test Plan: So far tested on the following code: ```jsx function App() { const [counter,setCounter] = useState(0); const [text,setText] = useState('TextInput'); const [flag,setFlag] = useState(true); return ( <SafeAreaView style={{ flex: 1, backgroundColor: '#fff', alignItems: 'center', justifyContent: 'center', }} > <Text style={{fontSize: 24}}>RN 24 Label Testing {flag ? 'A' : 'B'}</Text> <TextInput value={text} onChangeText={setText} style={{fontSize: 24, borderWidth: 1}} placeholder="Placeholder" /> <Pressable onPress={() => setCounter(prevState => prevState + 1)} style={{backgroundColor: counter % 2 === 0 ? 'red' : 'blue', width: 200, height: 50}} /> <Pressable onPress={() => setFlag(!flag)} style={{backgroundColor: 'green', width: 200, height: 50}} /> </SafeAreaView> ); } ``` Differential Revision: D71727907 Pulled By: j-piasecki
8a015d4
to
57ab723
Compare
This pull request was exported from Phabricator. Differential Revision: D71727907 |
This pull request was exported from Phabricator. Differential Revision: D71727907 |
57ab723
to
b8d1587
Compare
Summary: Fixes facebook#45857 The general idea behind this PR is the same for both platforms: dirty all nodes with `MeasurableYogaNode` trait when the layout is constrained with a new `fontSizeMultiplier`. There were a few caveats: - `ParagraphShadowNode` marks its layout as clean in the constructor in most cases. To prevent that from using a stale measurement, I've added a `fontSizeMultiplier_` field in the node that keeps track of the font scale it was last laid out with. That value is then compared with the scale used to create the attributed string kept in the node's state. If those differ, the layout is not cleared. - On Android, font scale wasn't passed down to the `SurfaceHandler` - On Android, text measurement relies on cached `DisplayMetrics` which were not updated when the system font scale changed. - `AndroidTextInputShadowNode` wasn't using `fontSizeMultiplier` at all. I needed to add it in all places where an `AttributedString` is constructed. - When the `fontSizeMultiplier` is changed, the entire `ShadowTree` is cloned. I'm not sure if there's a reliable way of determining whether the node is mutable or not. - Changing font scale and navigating back to the app on Android has the potential to cause a `SIGSEGV` but it also happens without changes in this PR. ## Changelog: [GENERAL] [FIXED] - Fixed text not updating correctly after changing font scale in settings Pull Request resolved: facebook#45978 Test Plan: So far tested on the following code: ```jsx function App() { const [counter,setCounter] = useState(0); const [text,setText] = useState('TextInput'); const [flag,setFlag] = useState(true); return ( <SafeAreaView style={{ flex: 1, backgroundColor: '#fff', alignItems: 'center', justifyContent: 'center', }} > <Text style={{fontSize: 24}}>RN 24 Label Testing {flag ? 'A' : 'B'}</Text> <TextInput value={text} onChangeText={setText} style={{fontSize: 24, borderWidth: 1}} placeholder="Placeholder" /> <Pressable onPress={() => setCounter(prevState => prevState + 1)} style={{backgroundColor: counter % 2 === 0 ? 'red' : 'blue', width: 200, height: 50}} /> <Pressable onPress={() => setFlag(!flag)} style={{backgroundColor: 'green', width: 200, height: 50}} /> </SafeAreaView> ); } ``` Differential Revision: D71727907 Pulled By: j-piasecki
@j-piasecki has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
This pull request was exported from Phabricator. Differential Revision: D71727907 |
Summary: Fixes facebook#45857 The general idea behind this PR is the same for both platforms: dirty all nodes with `MeasurableYogaNode` trait when the layout is constrained with a new `fontSizeMultiplier`. There were a few caveats: - `ParagraphShadowNode` marks its layout as clean in the constructor in most cases. To prevent that from using a stale measurement, I've added a `fontSizeMultiplier_` field in the node that keeps track of the font scale it was last laid out with. That value is then compared with the scale used to create the attributed string kept in the node's state. If those differ, the layout is not cleared. - On Android, font scale wasn't passed down to the `SurfaceHandler` - On Android, text measurement relies on cached `DisplayMetrics` which were not updated when the system font scale changed. - `AndroidTextInputShadowNode` wasn't using `fontSizeMultiplier` at all. I needed to add it in all places where an `AttributedString` is constructed. - When the `fontSizeMultiplier` is changed, the entire `ShadowTree` is cloned. I'm not sure if there's a reliable way of determining whether the node is mutable or not. - Changing font scale and navigating back to the app on Android has the potential to cause a `SIGSEGV` but it also happens without changes in this PR. ## Changelog: [GENERAL] [FIXED] - Fixed text not updating correctly after changing font scale in settings Pull Request resolved: facebook#45978 Test Plan: So far tested on the following code: ```jsx function App() { const [counter,setCounter] = useState(0); const [text,setText] = useState('TextInput'); const [flag,setFlag] = useState(true); return ( <SafeAreaView style={{ flex: 1, backgroundColor: '#fff', alignItems: 'center', justifyContent: 'center', }} > <Text style={{fontSize: 24}}>RN 24 Label Testing {flag ? 'A' : 'B'}</Text> <TextInput value={text} onChangeText={setText} style={{fontSize: 24, borderWidth: 1}} placeholder="Placeholder" /> <Pressable onPress={() => setCounter(prevState => prevState + 1)} style={{backgroundColor: counter % 2 === 0 ? 'red' : 'blue', width: 200, height: 50}} /> <Pressable onPress={() => setFlag(!flag)} style={{backgroundColor: 'green', width: 200, height: 50}} /> </SafeAreaView> ); } ``` Differential Revision: D71727907 Pulled By: j-piasecki
83fc24f
to
829e111
Compare
This pull request was exported from Phabricator. Differential Revision: D71727907 |
Summary: Fixes facebook#45857 The general idea behind this PR is the same for both platforms: dirty all nodes with `MeasurableYogaNode` trait when the layout is constrained with a new `fontSizeMultiplier`. There were a few caveats: - `ParagraphShadowNode` marks its layout as clean in the constructor in most cases. To prevent that from using a stale measurement, I've added a `fontSizeMultiplier_` field in the node that keeps track of the font scale it was last laid out with. That value is then compared with the scale used to create the attributed string kept in the node's state. If those differ, the layout is not cleared. - On Android, font scale wasn't passed down to the `SurfaceHandler` - On Android, text measurement relies on cached `DisplayMetrics` which were not updated when the system font scale changed. - `AndroidTextInputShadowNode` wasn't using `fontSizeMultiplier` at all. I needed to add it in all places where an `AttributedString` is constructed. - When the `fontSizeMultiplier` is changed, the entire `ShadowTree` is cloned. I'm not sure if there's a reliable way of determining whether the node is mutable or not. - Changing font scale and navigating back to the app on Android has the potential to cause a `SIGSEGV` but it also happens without changes in this PR. ## Changelog: [GENERAL] [FIXED] - Fixed text not updating correctly after changing font scale in settings Pull Request resolved: facebook#45978 Test Plan: So far tested on the following code: ```jsx function App() { const [counter,setCounter] = useState(0); const [text,setText] = useState('TextInput'); const [flag,setFlag] = useState(true); return ( <SafeAreaView style={{ flex: 1, backgroundColor: '#fff', alignItems: 'center', justifyContent: 'center', }} > <Text style={{fontSize: 24}}>RN 24 Label Testing {flag ? 'A' : 'B'}</Text> <TextInput value={text} onChangeText={setText} style={{fontSize: 24, borderWidth: 1}} placeholder="Placeholder" /> <Pressable onPress={() => setCounter(prevState => prevState + 1)} style={{backgroundColor: counter % 2 === 0 ? 'red' : 'blue', width: 200, height: 50}} /> <Pressable onPress={() => setFlag(!flag)} style={{backgroundColor: 'green', width: 200, height: 50}} /> </SafeAreaView> ); } ``` Differential Revision: D71727907 Pulled By: j-piasecki
829e111
to
02a8f8d
Compare
This pull request was exported from Phabricator. Differential Revision: D71727907 |
Summary: Fixes facebook#45857 The general idea behind this PR is the same for both platforms: dirty all nodes with `MeasurableYogaNode` trait when the layout is constrained with a new `fontSizeMultiplier`. There were a few caveats: - `ParagraphShadowNode` marks its layout as clean in the constructor in most cases. To prevent that from using a stale measurement, I've added a `fontSizeMultiplier_` field in the node that keeps track of the font scale it was last laid out with. That value is then compared with the scale used to create the attributed string kept in the node's state. If those differ, the layout is not cleared. - On Android, font scale wasn't passed down to the `SurfaceHandler` - On Android, text measurement relies on cached `DisplayMetrics` which were not updated when the system font scale changed. - `AndroidTextInputShadowNode` wasn't using `fontSizeMultiplier` at all. I needed to add it in all places where an `AttributedString` is constructed. - When the `fontSizeMultiplier` is changed, the entire `ShadowTree` is cloned. I'm not sure if there's a reliable way of determining whether the node is mutable or not. - Changing font scale and navigating back to the app on Android has the potential to cause a `SIGSEGV` but it also happens without changes in this PR. ## Changelog: [GENERAL] [FIXED] - Fixed text not updating correctly after changing font scale in settings Pull Request resolved: facebook#45978 Test Plan: So far tested on the following code: ```jsx function App() { const [counter,setCounter] = useState(0); const [text,setText] = useState('TextInput'); const [flag,setFlag] = useState(true); return ( <SafeAreaView style={{ flex: 1, backgroundColor: '#fff', alignItems: 'center', justifyContent: 'center', }} > <Text style={{fontSize: 24}}>RN 24 Label Testing {flag ? 'A' : 'B'}</Text> <TextInput value={text} onChangeText={setText} style={{fontSize: 24, borderWidth: 1}} placeholder="Placeholder" /> <Pressable onPress={() => setCounter(prevState => prevState + 1)} style={{backgroundColor: counter % 2 === 0 ? 'red' : 'blue', width: 200, height: 50}} /> <Pressable onPress={() => setFlag(!flag)} style={{backgroundColor: 'green', width: 200, height: 50}} /> </SafeAreaView> ); } ``` Differential Revision: D71727907 Pulled By: j-piasecki
02a8f8d
to
8135477
Compare
fdad3c6
to
f4ec65b
Compare
Summary: Fixes facebook#45857 The general idea behind this PR is the same for both platforms: dirty all nodes with `MeasurableYogaNode` trait when the layout is constrained with a new `fontSizeMultiplier`. There were a few caveats: - `ParagraphShadowNode` marks its layout as clean in the constructor in most cases. To prevent that from using a stale measurement, I've added a `fontSizeMultiplier_` field in the node that keeps track of the font scale it was last laid out with. That value is then compared with the scale used to create the attributed string kept in the node's state. If those differ, the layout is not cleared. - On Android, font scale wasn't passed down to the `SurfaceHandler` - On Android, text measurement relies on cached `DisplayMetrics` which were not updated when the system font scale changed. - `AndroidTextInputShadowNode` wasn't using `fontSizeMultiplier` at all. I needed to add it in all places where an `AttributedString` is constructed. - When the `fontSizeMultiplier` is changed, the entire `ShadowTree` is cloned. I'm not sure if there's a reliable way of determining whether the node is mutable or not. - Changing font scale and navigating back to the app on Android has the potential to cause a `SIGSEGV` but it also happens without changes in this PR. ## Changelog: [GENERAL] [FIXED] - Fixed text not updating correctly after changing font scale in settings Test Plan: So far tested on the following code: ```jsx function App() { const [counter,setCounter] = useState(0); const [text,setText] = useState('TextInput'); const [flag,setFlag] = useState(true); return ( <SafeAreaView style={{ flex: 1, backgroundColor: '#fff', alignItems: 'center', justifyContent: 'center', }} > <Text style={{fontSize: 24}}>RN 24 Label Testing {flag ? 'A' : 'B'}</Text> <TextInput value={text} onChangeText={setText} style={{fontSize: 24, borderWidth: 1}} placeholder="Placeholder" /> <Pressable onPress={() => setCounter(prevState => prevState + 1)} style={{backgroundColor: counter % 2 === 0 ? 'red' : 'blue', width: 200, height: 50}} /> <Pressable onPress={() => setFlag(!flag)} style={{backgroundColor: 'green', width: 200, height: 50}} /> </SafeAreaView> ); } ``` Differential Revision: D71727907 Pulled By: j-piasecki
This pull request was exported from Phabricator. Differential Revision: D71727907 |
f4ec65b
to
c1c34b5
Compare
Summary: Fixes facebook#45857 The general idea behind this PR is the same for both platforms: dirty all nodes with `MeasurableYogaNode` trait when the layout is constrained with a new `fontSizeMultiplier`. There were a few caveats: - `ParagraphShadowNode` marks its layout as clean in the constructor in most cases. To prevent that from using a stale measurement I'm using the font scale multiplier stored in `content_` property of the node. That value is then compared with the scale used to create the attributed string kept in the node's state. If those differ, the layout is not cleared. - On Android, font scale wasn't passed down to the `SurfaceHandler` - On Android, text measurement relies on cached `DisplayMetrics` which were not updated when the system font scale changed. - `AndroidTextInputShadowNode` wasn't using `fontSizeMultiplier` at all. I needed to add it in all places where an `AttributedString` is constructed. ## Changelog: [GENERAL] [FIXED] - Fixed text not updating correctly after changing font scale in settings Test Plan: So far tested on the following code: ```jsx function App() { const [counter,setCounter] = useState(0); const [text,setText] = useState('TextInput'); const [flag,setFlag] = useState(true); return ( <SafeAreaView style={{ flex: 1, backgroundColor: '#fff', alignItems: 'center', justifyContent: 'center', }} > <Text style={{fontSize: 24}}>RN 24 Label Testing {flag ? 'A' : 'B'}</Text> <TextInput value={text} onChangeText={setText} style={{fontSize: 24, borderWidth: 1}} placeholder="Placeholder" /> <Pressable onPress={() => setCounter(prevState => prevState + 1)} style={{backgroundColor: counter % 2 === 0 ? 'red' : 'blue', width: 200, height: 50}} /> <Pressable onPress={() => setFlag(!flag)} style={{backgroundColor: 'green', width: 200, height: 50}} /> </SafeAreaView> ); } ``` Differential Revision: D71727907 Pulled By: j-piasecki
This pull request was exported from Phabricator. Differential Revision: D71727907 |
c1c34b5
to
8140336
Compare
Summary: Fixes facebook#45857 The general idea behind this PR is the same for both platforms: dirty all nodes with `MeasurableYogaNode` trait when the layout is constrained with a new `fontSizeMultiplier`. There were a few caveats: - `ParagraphShadowNode` marks its layout as clean in the constructor in most cases. To prevent that from using a stale measurement I'm using the font scale multiplier stored in `content_` property of the node. That value is then compared with the scale used to create the attributed string kept in the node's state. If those differ, the layout is not cleared. - On Android, font scale wasn't passed down to the `SurfaceHandler` - On Android, text measurement relies on cached `DisplayMetrics` which were not updated when the system font scale changed. - `AndroidTextInputShadowNode` wasn't using `fontSizeMultiplier` at all. I needed to add it in all places where an `AttributedString` is constructed. ## Changelog: [GENERAL] [FIXED] - Fixed text not updating correctly after changing font scale in settings Test Plan: So far tested on the following code: ```jsx function App() { const [counter,setCounter] = useState(0); const [text,setText] = useState('TextInput'); const [flag,setFlag] = useState(true); return ( <SafeAreaView style={{ flex: 1, backgroundColor: '#fff', alignItems: 'center', justifyContent: 'center', }} > <Text style={{fontSize: 24}}>RN 24 Label Testing {flag ? 'A' : 'B'}</Text> <TextInput value={text} onChangeText={setText} style={{fontSize: 24, borderWidth: 1}} placeholder="Placeholder" /> <Pressable onPress={() => setCounter(prevState => prevState + 1)} style={{backgroundColor: counter % 2 === 0 ? 'red' : 'blue', width: 200, height: 50}} /> <Pressable onPress={() => setFlag(!flag)} style={{backgroundColor: 'green', width: 200, height: 50}} /> </SafeAreaView> ); } ``` Differential Revision: D71727907 Pulled By: j-piasecki
This pull request was exported from Phabricator. Differential Revision: D71727907 |
Summary: Fixes facebook#45857 The general idea behind this PR is the same for both platforms: dirty all nodes with `MeasurableYogaNode` trait when the layout is constrained with a new `fontSizeMultiplier`. There were a few caveats: - `ParagraphShadowNode` marks its layout as clean in the constructor in most cases. To prevent that from using a stale measurement I'm using the font scale multiplier stored in `content_` property of the node. That value is then compared with the scale used to create the attributed string kept in the node's state. If those differ, the layout is not cleared. - On Android, font scale wasn't passed down to the `SurfaceHandler` - On Android, text measurement relies on cached `DisplayMetrics` which were not updated when the system font scale changed. - `AndroidTextInputShadowNode` wasn't using `fontSizeMultiplier` at all. I needed to add it in all places where an `AttributedString` is constructed. ## Changelog: [GENERAL] [FIXED] - Fixed text not updating correctly after changing font scale in settings Test Plan: So far tested on the following code: ```jsx function App() { const [counter,setCounter] = useState(0); const [text,setText] = useState('TextInput'); const [flag,setFlag] = useState(true); return ( <SafeAreaView style={{ flex: 1, backgroundColor: '#fff', alignItems: 'center', justifyContent: 'center', }} > <Text style={{fontSize: 24}}>RN 24 Label Testing {flag ? 'A' : 'B'}</Text> <TextInput value={text} onChangeText={setText} style={{fontSize: 24, borderWidth: 1}} placeholder="Placeholder" /> <Pressable onPress={() => setCounter(prevState => prevState + 1)} style={{backgroundColor: counter % 2 === 0 ? 'red' : 'blue', width: 200, height: 50}} /> <Pressable onPress={() => setFlag(!flag)} style={{backgroundColor: 'green', width: 200, height: 50}} /> </SafeAreaView> ); } ``` Differential Revision: D71727907 Pulled By: j-piasecki
8140336
to
4aebab2
Compare
This pull request was exported from Phabricator. Differential Revision: D71727907 |
Summary: Fixes facebook#45857 The general idea behind this PR is the same for both platforms: dirty all nodes with `MeasurableYogaNode` trait when the layout is constrained with a new `fontSizeMultiplier`. There were a few caveats: - `ParagraphShadowNode` marks its layout as clean in the constructor in most cases. To prevent that from using a stale measurement I'm using the font scale multiplier stored in `content_` property of the node. That value is then compared with the scale used to create the attributed string kept in the node's state. If those differ, the layout is not cleared. - On Android, font scale wasn't passed down to the `SurfaceHandler` - On Android, text measurement relies on cached `DisplayMetrics` which were not updated when the system font scale changed. - `AndroidTextInputShadowNode` wasn't using `fontSizeMultiplier` at all. I needed to add it in all places where an `AttributedString` is constructed. ## Changelog: [GENERAL] [FIXED] - Fixed text not updating correctly after changing font scale in settings Test Plan: So far tested on the following code: ```jsx function App() { const [counter,setCounter] = useState(0); const [text,setText] = useState('TextInput'); const [flag,setFlag] = useState(true); return ( <SafeAreaView style={{ flex: 1, backgroundColor: '#fff', alignItems: 'center', justifyContent: 'center', }} > <Text style={{fontSize: 24}}>RN 24 Label Testing {flag ? 'A' : 'B'}</Text> <TextInput value={text} onChangeText={setText} style={{fontSize: 24, borderWidth: 1}} placeholder="Placeholder" /> <Pressable onPress={() => setCounter(prevState => prevState + 1)} style={{backgroundColor: counter % 2 === 0 ? 'red' : 'blue', width: 200, height: 50}} /> <Pressable onPress={() => setFlag(!flag)} style={{backgroundColor: 'green', width: 200, height: 50}} /> </SafeAreaView> ); } ``` Reviewed By: NickGerleman Differential Revision: D71727907 Pulled By: j-piasecki
4aebab2
to
db9ab31
Compare
This pull request was exported from Phabricator. Differential Revision: D71727907 |
Summary: Fixes facebook#45857 The general idea behind this PR is the same for both platforms: dirty all nodes with `MeasurableYogaNode` trait when the layout is constrained with a new `fontSizeMultiplier`. There were a few caveats: - `ParagraphShadowNode` marks its layout as clean in the constructor in most cases. To prevent that from using a stale measurement I'm using the font scale multiplier stored in `content_` property of the node. That value is then compared with the scale used to create the attributed string kept in the node's state. If those differ, the layout is not cleared. - On Android, font scale wasn't passed down to the `SurfaceHandler` - On Android, text measurement relies on cached `DisplayMetrics` which were not updated when the system font scale changed. - `AndroidTextInputShadowNode` wasn't using `fontSizeMultiplier` at all. I needed to add it in all places where an `AttributedString` is constructed. ## Changelog: [GENERAL] [FIXED] - Fixed text not updating correctly after changing font scale in settings Test Plan: So far tested on the following code: ```jsx function App() { const [counter,setCounter] = useState(0); const [text,setText] = useState('TextInput'); const [flag,setFlag] = useState(true); return ( <SafeAreaView style={{ flex: 1, backgroundColor: '#fff', alignItems: 'center', justifyContent: 'center', }} > <Text style={{fontSize: 24}}>RN 24 Label Testing {flag ? 'A' : 'B'}</Text> <TextInput value={text} onChangeText={setText} style={{fontSize: 24, borderWidth: 1}} placeholder="Placeholder" /> <Pressable onPress={() => setCounter(prevState => prevState + 1)} style={{backgroundColor: counter % 2 === 0 ? 'red' : 'blue', width: 200, height: 50}} /> <Pressable onPress={() => setFlag(!flag)} style={{backgroundColor: 'green', width: 200, height: 50}} /> </SafeAreaView> ); } ``` Reviewed By: NickGerleman Differential Revision: D71727907 Pulled By: j-piasecki
db9ab31
to
6db29a1
Compare
This pull request was exported from Phabricator. Differential Revision: D71727907 |
6db29a1
to
cb9ae42
Compare
Summary: Fixes facebook#45857 The general idea behind this PR is the same for both platforms: dirty all nodes with `MeasurableYogaNode` trait when the layout is constrained with a new `fontSizeMultiplier`. There were a few caveats: - `ParagraphShadowNode` marks its layout as clean in the constructor in most cases. To prevent that from using a stale measurement I'm using the font scale multiplier stored in `content_` property of the node. That value is then compared with the scale used to create the attributed string kept in the node's state. If those differ, the layout is not cleared. - On Android, font scale wasn't passed down to the `SurfaceHandler` - On Android, text measurement relies on cached `DisplayMetrics` which were not updated when the system font scale changed. - `AndroidTextInputShadowNode` wasn't using `fontSizeMultiplier` at all. I needed to add it in all places where an `AttributedString` is constructed. ## Changelog: [GENERAL] [FIXED] - Fixed text not updating correctly after changing font scale in settings Test Plan: So far tested on the following code: ```jsx function App() { const [counter,setCounter] = useState(0); const [text,setText] = useState('TextInput'); const [flag,setFlag] = useState(true); return ( <SafeAreaView style={{ flex: 1, backgroundColor: '#fff', alignItems: 'center', justifyContent: 'center', }} > <Text style={{fontSize: 24}}>RN 24 Label Testing {flag ? 'A' : 'B'}</Text> <TextInput value={text} onChangeText={setText} style={{fontSize: 24, borderWidth: 1}} placeholder="Placeholder" /> <Pressable onPress={() => setCounter(prevState => prevState + 1)} style={{backgroundColor: counter % 2 === 0 ? 'red' : 'blue', width: 200, height: 50}} /> <Pressable onPress={() => setFlag(!flag)} style={{backgroundColor: 'green', width: 200, height: 50}} /> </SafeAreaView> ); } ``` Reviewed By: NickGerleman Differential Revision: D71727907 Pulled By: j-piasecki
This pull request was exported from Phabricator. Differential Revision: D71727907 |
Summary: Fixes facebook#45857 The general idea behind this PR is the same for both platforms: dirty all nodes with `MeasurableYogaNode` trait when the layout is constrained with a new `fontSizeMultiplier`. There were a few caveats: - `ParagraphShadowNode` marks its layout as clean in the constructor in most cases. To prevent that from using a stale measurement I'm using the font scale multiplier stored in `content_` property of the node. That value is then compared with the scale used to create the attributed string kept in the node's state. If those differ, the layout is not cleared. - On Android, font scale wasn't passed down to the `SurfaceHandler` - On Android, text measurement relies on cached `DisplayMetrics` which were not updated when the system font scale changed. - `AndroidTextInputShadowNode` wasn't using `fontSizeMultiplier` at all. I needed to add it in all places where an `AttributedString` is constructed. ## Changelog: [GENERAL] [FIXED] - Fixed text not updating correctly after changing font scale in settings Test Plan: So far tested on the following code: ```jsx function App() { const [counter,setCounter] = useState(0); const [text,setText] = useState('TextInput'); const [flag,setFlag] = useState(true); return ( <SafeAreaView style={{ flex: 1, backgroundColor: '#fff', alignItems: 'center', justifyContent: 'center', }} > <Text style={{fontSize: 24}}>RN 24 Label Testing {flag ? 'A' : 'B'}</Text> <TextInput value={text} onChangeText={setText} style={{fontSize: 24, borderWidth: 1}} placeholder="Placeholder" /> <Pressable onPress={() => setCounter(prevState => prevState + 1)} style={{backgroundColor: counter % 2 === 0 ? 'red' : 'blue', width: 200, height: 50}} /> <Pressable onPress={() => setFlag(!flag)} style={{backgroundColor: 'green', width: 200, height: 50}} /> </SafeAreaView> ); } ``` Reviewed By: NickGerleman Differential Revision: D71727907 Pulled By: j-piasecki
cb9ae42
to
5f899bb
Compare
This pull request was exported from Phabricator. Differential Revision: D71727907 |
This pull request was successfully merged by @j-piasecki in c008604 When will my fix make it into a release? | How to file a pick request? |
@j-piasecki merged this pull request in c008604. |
Summary: Fixes facebook#45857 The general idea behind this PR is the same for both platforms: dirty all nodes with `MeasurableYogaNode` trait when the layout is constrained with a new `fontSizeMultiplier`. There were a few caveats: - `ParagraphShadowNode` marks its layout as clean in the constructor in most cases. To prevent that from using a stale measurement I'm using the font scale multiplier stored in `content_` property of the node. That value is then compared with the scale used to create the attributed string kept in the node's state. If those differ, the layout is not cleared. - On Android, font scale wasn't passed down to the `SurfaceHandler` - On Android, text measurement relies on cached `DisplayMetrics` which were not updated when the system font scale changed. - `AndroidTextInputShadowNode` wasn't using `fontSizeMultiplier` at all. I needed to add it in all places where an `AttributedString` is constructed. ## Changelog: [GENERAL] [FIXED] - Fixed text not updating correctly after changing font scale in settings Pull Request resolved: facebook#45978 Test Plan: So far tested on the following code: ```jsx function App() { const [counter,setCounter] = useState(0); const [text,setText] = useState('TextInput'); const [flag,setFlag] = useState(true); return ( <SafeAreaView style={{ flex: 1, backgroundColor: '#fff', alignItems: 'center', justifyContent: 'center', }} > <Text style={{fontSize: 24}}>RN 24 Label Testing {flag ? 'A' : 'B'}</Text> <TextInput value={text} onChangeText={setText} style={{fontSize: 24, borderWidth: 1}} placeholder="Placeholder" /> <Pressable onPress={() => setCounter(prevState => prevState + 1)} style={{backgroundColor: counter % 2 === 0 ? 'red' : 'blue', width: 200, height: 50}} /> <Pressable onPress={() => setFlag(!flag)} style={{backgroundColor: 'green', width: 200, height: 50}} /> </SafeAreaView> ); } ``` Reviewed By: NickGerleman Differential Revision: D71727907 Pulled By: j-piasecki fbshipit-source-id: 240fb5fa4967a9182bce7e885798b233d1e25aea
Summary:
Fixes #45857
The general idea behind this PR is the same for both platforms: dirty all nodes with
MeasurableYogaNode
trait when the layout is constrained with a newfontSizeMultiplier
. There were a few caveats:ParagraphShadowNode
marks its layout as clean in the constructor in most cases. To prevent that from using a stale measurement, I've added afontSizeMultiplier_
field in the node that keeps track of the font scale it was last laid out with. That value is then compared with the scale used to create the attributed string kept in the node's state. If those differ, the layout is not cleared.SurfaceHandler
DisplayMetrics
which were not updated when the system font scale changed.AndroidTextInputShadowNode
wasn't usingfontSizeMultiplier
at all. I needed to add it in all places where anAttributedString
is constructed.fontSizeMultiplier
is changed, the entireShadowTree
is cloned. I'm not sure if there's a reliable way of determining whether the node is mutable or not.SIGSEGV
but it also happens without changes in this PR.Changelog:
[GENERAL] [FIXED] - Fixed text not updating correctly after changing font scale in settings
Test Plan:
So far tested on the following code: