Commit 55002b1
authored
fix(android): initial measuring tweaks (#784)
# Summary
This fixes a bug where the component's initial height calculation would
break if `defaultValue` was provided without an explicit `fontSize` in
`htmlStyle` prop. The root cause was a mix of a bad initial estimate and
invalid measurements caching:
During `initialMeasure`, missing a passed `fontSize` in props, meant it
defaulted to 0, which caused the entire height estimate to be wrong. I
fixed this by adding a fallback to the default font size, matching how
`EnrichedText` measurements already handle it.
After `initialMeasure`, `setValue` (caused by the present `defaultValue`
prop) is run, which tries to update the shadow node state, but
`stateWrapper` is still `null`. This caused the `MeasurementStore`
internal cache to be out of sync with the actual shadow node's state.
Fixed this by providing an early return in `invalidateLayout()` - we
don't call `Measurement.store()` at all, if `stateWrapper` is
unavailable.
I've also tweaked two small things: in `EnrichedTextInputShadowNode.cpp`
the `forceHeightRecalculationCounter_` was never initialized before
accessing its value, which would make it have random, garbage values.
I've also adjusted the provided `forceHeightRecalculationCounter` to the
`stateWrapper`, as its value was always pre-increment during
`invalidateLayout()`. I believe it's better to have the shadow node's
and component's counters' values consistent.
Sometimes the `setValue`'s layout invalidation could run before the
`initialMeasure`. That made the `MeasurementStore` cached measurements
correct from the start, as after `setValue` runs, the actual component's
size is measured, not estimated like it's the case with
`initialMeasure`. You can see this race condition in the attached video.
## Test Plan
I've provided a modified example app, so you can see the bug yourself -
you can run
```sh
git checkout 212b2a3
```
and then you can check out how it works after the fix
```sh
git checkout 4c47809
```
## Screenshots / Videos
The race condition in action (stumbled across in a different app):
https://github.com/user-attachments/assets/1ded9bcc-ce87-49a1-82d8-3300dd41a4a3
## Compatibility
| OS | Implemented |
| ------- | :---------: |
| iOS | ❌ |
| Android | ✅ |
| Web | ❌ |
## Checklist
- [x] E2E tests are passing
- [ ] Required E2E tests have been added (if applicable)1 parent 49da66a commit 55002b1
3 files changed
Lines changed: 6 additions & 5 deletions
File tree
- android/src/main
- java/com/swmansion/enriched/textinput
- new_arch/react/renderer/components/ReactNativeEnrichedSpec
Lines changed: 4 additions & 3 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
| 11 | + | |
| 12 | + | |
11 | 13 | | |
12 | 14 | | |
13 | 15 | | |
14 | 16 | | |
15 | 17 | | |
16 | 18 | | |
17 | | - | |
18 | 19 | | |
19 | 20 | | |
20 | | - | |
21 | | - | |
| 21 | + | |
| 22 | + | |
22 | 23 | | |
23 | 24 | | |
24 | 25 | | |
| |||
Lines changed: 1 addition & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
130 | 130 | | |
131 | 131 | | |
132 | 132 | | |
133 | | - | |
| 133 | + | |
134 | 134 | | |
135 | 135 | | |
136 | 136 | | |
| |||
Lines changed: 1 addition & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
47 | 47 | | |
48 | 48 | | |
49 | 49 | | |
50 | | - | |
| 50 | + | |
51 | 51 | | |
52 | 52 | | |
53 | 53 | | |
0 commit comments