Skip to content

[Android] Improve consistency for overflowed TextInput#45

Merged
johnmlee101 merged 1 commit intoExpensify:Expensify-0.70.4-alpha.2from
redstar504:Brayden-TextInputRenderPosition-Exfy
Mar 31, 2023
Merged

[Android] Improve consistency for overflowed TextInput#45
johnmlee101 merged 1 commit intoExpensify:Expensify-0.70.4-alpha.2from
redstar504:Brayden-TextInputRenderPosition-Exfy

Conversation

@redstar504
Copy link

Upstream PR Link

facebook#36428

Summary

Coming from this issue: Expensify/App#15310

In the linked issue we have identified a difference in how TextInput components behave when the text is longer than the width of the field on Android compared to other platforms. On other platforms, the beginning of the line of text is shown and the end is clipped. However, on Android, the end of the line of text is displayed during render and auto-fill events.

To make the behavior consistent across all platforms, this pull request updates the Android implementation of TextInput to show the beginning of the line and clip the end, like on other platforms. This is done by adding a scroll function to the afterChangedText event in the existing TextWatcher used for detecting changes in text for the Android widget. This function is triggered when the field is unfocused, and therefore only applies to initial rendering and auto-filling events.

cc @s77rt @johnmlee101

Changelog

[ANDROID] [Fixed] - TextInput not displaying start of text on render like other platforms

Test Plan

Before Change
ex-platform-test-before.mp4
After Change
ex-platform-test-after.mp4

@s77rt
Copy link
Member

s77rt commented Mar 10, 2023

@johnmlee101 Should we wait for RN review on this one?

@johnmlee101
Copy link

I think it's been long enough, we should probably be fine updating this here for now and still continue to push in upstream

@s77rt
Copy link
Member

s77rt commented Mar 30, 2023

@johnmlee101 Would there be a diff if this get merged today or later on? We are currently updating our RN version on E/App. If this get merged sooner can we include the fix in the current in-process RN upgrade?

@johnmlee101
Copy link

Maybe! Who is currently leading that RN upgrade?

@s77rt
Copy link
Member

s77rt commented Mar 30, 2023

Expensify/App#15124

@johnmlee101 johnmlee101 merged commit 75b71b3 into Expensify:Expensify-0.70.4-alpha.2 Mar 31, 2023
@s77rt
Copy link
Member

s77rt commented Apr 12, 2023

@johnmlee101 Looks like you merged the commit to the wrong branch. Can you arrange a new RN upgrade v0.71.2-alpha.4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants