-
Notifications
You must be signed in to change notification settings - Fork 3k
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
[Ready for Payment][$1000] iOS - Chat - "Edit comment" input field scrolls up when tap enter in 1st line of text #19507
Comments
Triggered auto assignment to @alexpensify ( |
Bug0 Triage Checklist (Main S/O)
|
I was assigned when I was offline, and I'm still catching up from being OOO. I'll test tomorrow. |
@kbecciv - to confirm, is this bug within the native app or when using Safari on the iPhone? These two points need to be clarified (see screenshot). I tried testing but need to confirm if I need to be using the iPhone application or the app via a mobile browser. Thanks! |
Waiting for the updated testing steps |
@alexpensify Issue is occurring on only IOS native, updated the step 1. Sorry for confusion. |
Thanks for confirming @kbecciv - I'll test using iOS native. |
I'm having trouble with BrowserStack, I'll try testing again soon. |
Tom helped me test on the native app and was able to replicate it. |
Job added to Upwork: https://www.upwork.com/jobs/~0113571ce6c5a8b04e |
Current assignee @alexpensify is eligible for the External assigner, not assigning anyone new. |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @eVoloshchak ( |
Triggered auto assignment to @deetergp ( |
✅ Contributor details stored successfully. Thank you for contributing to Expensify! |
Waiting for proposals |
@eVoloshchak - when you get a chance, please review the recent proposal. Thank you! |
@eVoloshchak - I realized that I bumped this GH over the weekend. Keep us posted if the proposal will work or if there are required edits. |
Huh, that is an interesting issue @khaterdev's proposal looks promising, but I'm not 100% sure just yet and wanna do some testing. I'll get back with a review tomorrow |
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸 |
@deetergp @alexpensify @eVoloshchak this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks! |
…n prepending new lines to the text (#38679) Summary: ### Please re-state the problem that we are trying to solve in this issue. Multiline TextInput with a fixed height will scroll to the bottom of the screen when prepending new lines to the text. ### What is the root cause of that problem? The issue is caused by iOS UITextView: - The cursor moves to the end of the text when prepending new lines. - Moving the cursor to the end of the text triggers the scroll to the bottom. The behavior was reproduced on an iOS App (without react-native). The example included below implements a Component RCTUITextView based on UITextView, which modifies the UITextView attributedText with the textViewDidChange callback (source code available in this [comment](Expensify/App#19507 (comment))). Adding a new line on top of the UITextView on iOS results in: Issue 1) The cursor moves to the end of TextInput text Issue 2) The TextInput scrolls to the bottom <details><summary>Reproducing the issue on an iOS App without react-native</summary> <p> <video src="https://user-images.githubusercontent.com/24992535/246601549-99f480f3-ce80-4678-9378-f71c8aa67e17.mp4" width="900" /> </p> </details> Issue 1) is already fixed in react-native, which restores the previous cursor position (on Fabric with [_setAttributedString](https://github.com/fabriziobertoglio1987/react-native/blob/71e7bbbc2cf21abacf7009e300f5bba737e20d17/packages/react-native/React/Fabric/Mounting/ComponentViews/TextInput/RCTTextInputComponentView.mm#L600-L610)) after changing the text. Issue 2) needs to be fixed in react-native. ### What changes do you think we should make in order to solve the problem? Setting the correct TextInput scroll position after re-setting the cursor. ## Changelog: [IOS] [FIXED] - Fix Multiline TextInput with a fixed height scrolls to the bottom when changing AttributedText Pull Request resolved: #38679 Test Plan: Fabric (reproduces on controlled/not controlled TextInput example): | Before | After | | ----------- | ----------- | | <video src="https://github.com/facebook/react-native/assets/24992535/e06b31fe-407d-4897-b608-73e0cc0f224a" width="350" /> | <video src="https://github.com/facebook/react-native/assets/24992535/fa2eaa31-c616-43c5-9596-f84e7b70d80a" width="350" /> | Paper (reproduces only on controlled TextInput example): ```javascript function TextInputExample() { const [text, setText] = React.useState(''); return ( <View style={{marginTop: 200}}> <TextInput style={{height: 50, backgroundColor: 'white'}} multiline={true} value={text} onChangeText={text => { setText(text); }} /> </View> ); } ``` | Before | After | | ----------- | ----------- | | <video src="https://github.com/facebook/react-native/assets/24992535/6cb1f2de-717e-4dce-be0a-644f6a051c08" width="350" /> | <video src="https://github.com/facebook/react-native/assets/24992535/dee6edb6-76c6-48b0-b78f-99626235d30e" width="350" /> | Reviewed By: sammy-SC, cipolleschi Differential Revision: D48674090 Pulled By: NickGerleman fbshipit-source-id: 349e7b0910e314ec94b45b68c38571fed41ef117
The commit was cherry picked in 0.74. Thanks |
Oh okay awesome! So basically we're just waiting for our own upgrade to RN 0.74? |
@dangrous Yes. It was picked in 0.74.3. https://github.com/orgs/reactwg/projects/5/views/3?filterQuery=Multiline. |
Monthly Update: I think we are still waiting for 0.74 |
Looks like the 0.74 upgrade is nearly ready to merge! |
Updated the title since we're not on hold anymore, just waiting for the merge |
@dangrous should we move this to Weekly now? |
I'm not quite sure how long it's going to be... I guess we could update to hold for #37374 instead? |
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸 |
Why is melvin talking about proposals? That's weird. I'll put it on hold for that PR to stop that from happening. |
ooh okay so we just merged to RN 0.75 (which presumably includes the changes in 0.74). Can we retest this and hopefully finally close? |
oh wait it's not on staging yet so we can't test yet. But soooooon |
on staging I think! Let's test and close out, if we can! |
Heads up, I will be offline until Tuesday, September 3, 2024, and will not actively watch over this GitHub during that period. If anything urgent is needed here, please ask for help in the #expensify-open-source Slack Room-- thanks! If we can wait, I can work on the payment process next week. |
Confirmed tested on iOS staging! We should be good (finally) for payment! @alexpensify is out until 9/3, but ready when you're back! |
@dangrous, thanks for the update! Tomorrow, I'll start working on the payment process. |
Payouts due: 4 September 2024
Upwork job is here. @fabOnReact in Upwork says this project was paid in August 2023. Can you please double-check and reply if Upworks's information is inaccurate? Thanks! For now, I'm going to close this GH. |
$1,000 approved for @eVoloshchak |
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Action Performed:
Precondition - User sent a text message in chat (e.g. 1 word or 1 line of text)
9, Place cursor back at the end of the 1st line
Expected Result:
1st line of text hides above and cursor is focus on the next line of text and visible
Actual Result:
At step 7 the comment field scrolls slightly up and cursor also scrolls out of the view.
At step 10 field focus scrolls back to line 7/8
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Version Number: 1.3.17.0
Reproducible in staging?: yes
Reproducible in production?: yes
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: Any additional supporting documentation
https://platform.applause.com/services/links/v1/external/197b3277eef42e2b057288d9b61067c2fce3fc4a98ffe01c313143ea19511833
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @alexpensifyThe text was updated successfully, but these errors were encountered: