Bump react-native-keyboard-controller to 1.16.7#57580
Bump react-native-keyboard-controller to 1.16.7#57580luacmartins merged 6 commits intoExpensify:mainfrom
Conversation
|
|
|
@QichenZhu @kirillzyusko @chrispader this seems to be blocking #57128. What are the next steps here? |
This comment was marked as resolved.
This comment was marked as resolved.
@mountiny @QichenZhu no it's not blocking, it was just my mistake bumping the lib in the other PR. We don't need the updated version |
|
Sorry for the confusion @QichenZhu |
|
|
|
|
|
@dukenv0307 Please 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] |
| @@ -0,0 +1,5 @@ | |||
| function getBottomSuggestionPadding(): number { | |||
There was a problem hiding this comment.
Can you add the comment why we need this specific logic on android?
There was a problem hiding this comment.
How about: "Selection position returned from react-native-keyboard-controller is platform-dependent, so set different paddings for Android and iOS"?
There was a problem hiding this comment.
| function getBottomSuggestionPadding(): number { | |
| // Selection position returned from react-native-keyboard-controller is platform-dependent, so set different paddings for Android and iOS. | |
| function getBottomSuggestionPadding(): number { |
There was a problem hiding this comment.
comment seems fine, but do we think this is a 🐛 in react-native-keyboard-controller?
There was a problem hiding this comment.
Maybe, for me it also looks suspicious - I'll be glad to look into it, if needed
(though I wouldn't like to block this PR from being merged because it resolves a lot of bugs I believe)
There was a problem hiding this comment.
do we think this is a 🐛 in react-native-keyboard-controller?
In my opinion, it’s best not to have it, but it’s fine as long as the difference is predictable or due to the platform’s nature.
I wouldn't like to block this PR from being merged because it resolves a lot of bugs I believe
I agree it isn’t a blocker for this PR.
There was a problem hiding this comment.
@kirillzyusko it'd be great if you could investigate this one. I agree that we shouldn't block the PR on it though
Reviewer Checklist
Screenshots/Videos |
|
|
|
@roryabraham all yours |
|
@dukenv0307, Rory is OOO. |
|
I'll review this since it's blocking another PR |
| @@ -0,0 +1,5 @@ | |||
| function getBottomSuggestionPadding(): number { | |||
There was a problem hiding this comment.
@kirillzyusko it'd be great if you could investigate this one. I agree that we shouldn't block the PR on it though
|
✋ 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/luacmartins in version: 9.1.14-0 🚀
|
|
🚀 Deployed to production by https://github.com/luacmartins in version: 9.1.14-5 🚀
|










Explanation of Change
Fixed Issues
$ #56156
PROPOSAL: #56156 (comment)
Tests
This test applies to mobile devices only.
Precondition: Log in to an account with 8 or more expenses. Create some if necessary.
Offline tests
N/A. This test requires an active internet connection.
QA Steps
This test applies to mobile devices only.
Precondition: Log in to an account with 8 or more expenses. Create some if necessary.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionN/A: This test requires an active internet connection.
QA stepssectionhttps://expensify.slack.com/archives/C01GTK53T8Q/p1741041211288999
https://expensify.slack.com/archives/C049HHMV9SM/p1741433172815909
toggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.Not added as 1: This repo doesn't currently support automation tests on Android. 2. Most of the changes are covered by the upstream library's e2e tests.
mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
android-native.mp4
Android: mWeb Chrome
android-web.mp4
iOS: Native
ios-native.mp4
iOS: mWeb Safari
ios-web.mp4
MacOS: Chrome / Safari
mac-web.mov
MacOS: Desktop
mac-desktop.mp4