-
Notifications
You must be signed in to change notification settings - Fork 3.5k
fix: disable ios automatic extra space behavior when pasting #15051
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
|
@sketchydroide @mananjadhav One of you needs to copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
roryabraham
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please do not merge this PR with a patch to React Native via patch-package. We have a fork, let's use it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good to me, but we will have to wait for the react native fork to be updated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As mentioned earlier, we should remove this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will be removed with the RN update
|
Hi @mountiny , I was revisiting my PRs and noticed that the issue related to this one was closed as being fixed. But the issue is not fixed in iOS yet as we can see in the following video: Screen.Recording.2023-05-15.at.11.32.55.movTo actually fix the issue we need to merge this PR. I can confirm that we already have the So, I just need to remove the patch from this PR and merge it to fix the issue. In the following video I added the Screen.Recording.2023-05-15.at.11.35.23.mov |
|
@fabioh8010 thanks for the heads up, I think we can keep that issue closed Sorry I got a little bit lost on this PR, are we holding for the RN update you are working on right? |
|
@mountiny What I mean is that the fix I did in our RN fork is already available on Expensify/App PR which got merged: Expensify/react-native#40 |
|
@fabioh8010 ah, ok then please update the PR and we can tag C+ to help with the PR testing |
|
@mountiny Done! |
|
@fabioh8010 the HOLD in the title is not needed anymore right? |
|
@mountiny Yes, we can remove the HOLD and test the PR, I removed the patch. |
Reviewer Checklist
Screenshots/VideosWebweb-copy-no-space.movMobile Web - Chromemweb-chrome-copy-no-space.movMobile Web - Safarimweb-safari-copy-no-space.movDesktopdesktop-copy-no-space.moviOSios-copy-no-space.movAndroidandroid-copy-no-space.movThanks for the patience here. @mountiny All yours, tested this on Composer. |
mountiny
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @mananjadhav !
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/mountiny in version: 1.3.17-0 🚀
|
|
🚀 Deployed to production by https://github.com/chiragsalian in version: 1.3.17-5 🚀
|
Details
Implemented a new prop in the RN TextInput called
smartInsertDelete. This prop will allow us to disable the extra space that iOS system put when we paste something inside a input like theComposer.Two PR's were made to the upstream RN repo and Expensify RN repo respectively:
I also included a patch in this PR to address this changes and it can be removed later when we have a new RN version to use.
Fixed Issues
$ #14049
PROPOSAL: #14049 (comment)
Tests
Composera few times. It should not insert extra spaces before the pasted content.Offline tests
This feature doesn't change or is impacted by offline mode.
QA Steps
Same as above.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)myBool && <MyComponent />.src/languages/*files and using the translation methodWaiting for Copylabel for a copy review on the original GH to get the correct copy.STYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)/** comment above it */thisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Web
14049.macos.chrome.mov
14049.macos.safari.mov
Mobile Web - Chrome
14049.android.chrome.v2.mov
Mobile Web - Safari
14049.ios.safari.v2.mov
Desktop
14049.macos.desktop.mov
iOS
14049.ios.native.v2.mov
Android
14049.android.native.v2.mov