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

[HOLD for facebook/react-native#40762][$2000] Left-most characters are hidden if text in line is longer than screen size #15310

Closed
1 task
kavimuru opened this issue Feb 20, 2023 · 181 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering External Added to denote the issue can be worked on by a contributor Weekly KSv2

Comments

@kavimuru
Copy link

kavimuru commented Feb 20, 2023

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Action Performed:

  1. Open Android
  2. Go to settings and personal information
  3. Go to home address and try to select address with large state/province name. For example, select the Islamabad expressway.
  4. Notice that the first few letters of Islamabad is hidden in the state/province. But in mweb chrome, you’ll see that the last few letters are hidden. Of course, it needs to be scrolled to view the state name , but it should be consistent with the other platforms.

Expected Result:

There should be consistency between MWEB and Android. The last few letters of the state name should be hidden in android too in a similar way to how Mweb displays it.

Actual Result:

Using a big state name in android hides the first few letters of the name unlike MWEB chrome

Workaround:

unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

  • Android / native

Version Number: 1.2.74-0
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:
ANDROID

state.mp4
az_recorder_20230220_152323.1.mp4

Expensify/Expensify Issue URL:
Issue reported by: @priya-zha
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1676873662255729

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01450fe2eb8c10f817
  • Upwork Job ID: 1628799246971928576
  • Last Price Increase: 2023-09-07
  • Automatic offers:
    • s77rt | Reviewer | 26669048
    • priya-zha | Reporter | 26669049
@kavimuru kavimuru added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Feb 20, 2023
@melvin-bot melvin-bot bot locked and limited conversation to collaborators Feb 20, 2023
@MelvinBot
Copy link

Triggered auto assignment to @sonialiap (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

@MelvinBot
Copy link

MelvinBot commented Feb 20, 2023

Bug0 Triage Checklist (Main S/O)

  • This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
  • This bug is not a duplicate report (check E/App issues and #expensify-bugs)
    • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
  • This bug is reproducible using the reproduction steps in the OP. S/O
    • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
    • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
  • This issue is filled out as thoroughly and clearly as possible
    • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
  • I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

@MelvinBot
Copy link

Triggered auto assignment to @johnmlee101 (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@Beamanator
Copy link
Contributor

Yo @johnmlee101 - I implemented this page (Home address) recently, but I'm not sure if this is really a bug that needs to be fixed (a.k.a. i'd be curios to know where the cursor is after typing the long state / province name, and what happens if you change focus to a new field)

Anyway, if this is something that is a bug, I wonder how it could be happening on only the Home address page, I assume it would be happening in other forms too? Maybe? Just adding my 2 cents here :D

@sonialiap
Copy link
Contributor

sonialiap commented Feb 21, 2023

Address line 1, Address line 2, City and State all behave in the same way

The country names in the drop down are all shorter than the fields and the zip code is limited to 10 characters so it's shorter as well.

i'd be curios to know where the cursor is after typing the long state / province name, and what happens if you change focus to a new field

When you change focus the text does not rearrange to focus left or right. It is left in the position you last had it in. I.e. if you were typing on the far right then once you click out then the text will stay with the far right showing. If you then click into the line and scroll all the way to the left so that the cursor is before the first value in the string, then click out. The field will show the left most position.

(cursor was left at the right-most position in all lines before clicking out)
image

(cursor was left at the left-most position on Address Line 2 and City, it was at the right-most position in Line 1 and State)
image

@sonialiap
Copy link
Contributor

sonialiap commented Feb 21, 2023

  1. Save change to address
  2. Return to Personal details
  3. Click into Home Address again
  4. See that all long lines show the right-most point of the string

image

@sonialiap
Copy link
Contributor

This isn't huge, but I think we should change the behavior so that the lines default to showing the left-most position. English is read left to right so we should facilitate that

@sonialiap
Copy link
Contributor

sonialiap commented Feb 21, 2023

Same behavior on the Legal Name tab

image

Same on Display Name

image

Same on General Settings

image

(These three screenshots were taken after saving, going back one page, returning to page in screenshot)

@sonialiap sonialiap changed the title Personal details - Home address: Few characters are hidden if the state/province name is longer Left-most characters are hidden if text in line is longer than screen size Feb 21, 2023
@johnmlee101
Copy link
Contributor

Agreed, @Beamanator I think that the expectation is that pre-filled entries should be defaulted to the left-most cursor. I bet in our other forms/fields we don't auto-fill entries in the same way, right?

@Beamanator
Copy link
Contributor

Beamanator commented Feb 22, 2023

Thanks for so much testing @sonialiap ! I feel like showing the text starting from the leftmost characters makes the most sense in those flows

I bet in our other forms/fields we don't auto-fill entries in the same way, right?

Good question? I assumed these forms would act the same as other pages using the Form component, no?

@johnmlee101
Copy link
Contributor

Yeah I bet the solution here would probably include a larger change to prefilling Form components and defaulting the cursor to the left, not the right. Something about onInputChange on the Form component when its prefilled here:

props.onInputChange(value, inputKey);

@Beamanator
Copy link
Contributor

Ya that would make sense to me 👍

@johnmlee101
Copy link
Contributor

Opening this up since I bet we need to have some cases where onInputChange should change cursor position while others do not

@johnmlee101 johnmlee101 added the External Added to denote the issue can be worked on by a contributor label Feb 23, 2023
@melvin-bot melvin-bot bot unlocked this conversation Feb 23, 2023
@melvin-bot melvin-bot bot changed the title Left-most characters are hidden if text in line is longer than screen size [$1000] Left-most characters are hidden if text in line is longer than screen size Feb 23, 2023
@MelvinBot
Copy link

Job added to Upwork: https://www.upwork.com/jobs/~01450fe2eb8c10f817

@MelvinBot
Copy link

Current assignee @sonialiap is eligible for the External assigner, not assigning anyone new.

@MelvinBot
Copy link

Triggered auto assignment to Contributor-plus team member for initial proposal review - @Santhosh-Sellavel (External)

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Feb 23, 2023
@MelvinBot
Copy link

Current assignee @johnmlee101 is eligible for the External assigner, not assigning anyone new.

@redstar504
Copy link
Contributor

redstar504 commented Feb 23, 2023

Proposal

Please re-state the problem that we are trying to solve in this issue.

The leftmost characters of a TextInput on Android native are hidden after blurring the field or auto-filling text.

What is the root cause of that problem?

The issue is caused by the lack of scrollTo implementation in the TextInput component.

What changes do you think we should make in order to solve the problem?

Implement changes into our RN fork for setting scrollTo upon specific events:

  • ReactTextInputManager.java: Add a new TextWatcher to cover initial render and autofilled text. To accomplish this we add code inside the createViewInstance method. (sample)
  • ReactEditText.java: Modify the onFocusChanged method to cover the scenario when focus switches to another text input that doesn't close the keyboard. We add code to the end of the method. (sample)
  • ReactEditText.java: Change the hideSoftKeyboard method to use a receiver callback that covers the case of the keyboard being dismissed. We overwrite the whole method. (sample)

Extend the ReactEditText widget in /android and call scrollTo(0, 0) after relevant events.

Register the extended ReactEditText with the ReactTextInput view manager so it gets used by default for TextInput components.

Here's an example (some code removed to protect my idea): https://gist.github.com/redstar504/92cf6b724ee42ae67c61446b7a46c545

What alternative solutions did you explore? (Optional)

I originally had an implementation using setSelection but I wasn't happy with it because it relied on hacking the selection/cursor. Using scrollTo seems like the right way to handle it.

@sonialiap sonialiap changed the title [$2000] Left-most characters are hidden if text in line is longer than screen size [HOLD for facebook/react-native#40762][$2000] Left-most characters are hidden if text in line is longer than screen size Oct 16, 2023
@melvin-bot
Copy link

melvin-bot bot commented Oct 16, 2023

@johnmlee101, @s77rt, @sonialiap, @OlimpiaZurek Whoops! This issue is 2 days overdue. Let's get this updated quick!

@sonialiap sonialiap added Weekly KSv2 and removed Daily KSv2 labels Oct 16, 2023
@melvin-bot melvin-bot bot removed the Overdue label Oct 16, 2023
@sonialiap
Copy link
Contributor

I'm going OOO until Oct 23, changing to Weekly because of that and the hold

@priya-zha
Copy link

📣 @priya-zha 🎉 An offer has been automatically sent to your Upwork account for the Reporter role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job

Hi, why is melvin sending 50$ offer? I am well aware of the new rule of $50 but this issue was created on Feb 20, thus it should have sent me $250. Thus, I have accepted the $50 offer, can anyone send me the remaining $200, when the issue gets resolved?
Thank you

@melvin-bot melvin-bot bot added the Overdue label Oct 24, 2023
@sonialiap
Copy link
Contributor

@priya-zha the issue creation/offer sending automation doesn't account for dates so it sends the $50 new default regardless of when the issue was reported. Since the issue was reported in Feb, 2023 you should be paid $250 👍

@melvin-bot melvin-bot bot removed the Overdue label Oct 25, 2023
@sonialiap
Copy link
Contributor

Still on hold for facebook/react-native#40762

@melvin-bot melvin-bot bot added the Overdue label Nov 2, 2023
@s77rt
Copy link
Contributor

s77rt commented Nov 2, 2023

Still on hold

@melvin-bot melvin-bot bot removed the Overdue label Nov 2, 2023
@melvin-bot melvin-bot bot added the Overdue label Nov 13, 2023
@s77rt
Copy link
Contributor

s77rt commented Nov 13, 2023

Same ^

@melvin-bot melvin-bot bot removed the Overdue label Nov 13, 2023
@melvin-bot melvin-bot bot added the Overdue label Nov 21, 2023
@s77rt
Copy link
Contributor

s77rt commented Nov 21, 2023

Same ^

@melvin-bot melvin-bot bot removed the Overdue label Nov 21, 2023
@melvin-bot melvin-bot bot added the Overdue label Dec 1, 2023
@sonialiap
Copy link
Contributor

Still on hold for facebook/react-native#40762

@melvin-bot melvin-bot bot removed the Overdue label Dec 1, 2023
@melvin-bot melvin-bot bot added the Overdue label Dec 11, 2023
@s77rt
Copy link
Contributor

s77rt commented Dec 11, 2023

Same ^

@melvin-bot melvin-bot bot removed the Overdue label Dec 11, 2023
@melvin-bot melvin-bot bot added the Overdue label Dec 19, 2023
@sonialiap
Copy link
Contributor

This is held on an upstream fix that hasn't been worked on since Oct. I'm closing this as low priority

@melvin-bot melvin-bot bot removed the Overdue label Dec 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering External Added to denote the issue can be worked on by a contributor Weekly KSv2
Projects
None yet
Development

No branches or pull requests