-
Notifications
You must be signed in to change notification settings - Fork 3.5k
feat: Migrate lists to interactive keyboard dismissal #71440
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
base: main
Are you sure you want to change the base?
feat: Migrate lists to interactive keyboard dismissal #71440
Conversation
c45f442 to
7fc2c0c
Compare
|
@parasharrajat 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] |
|
@mountiny @roryabraham merged conflicts on this one after the previous PR has been merged. Will test interactive keyboard dismissal later |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4762b4ce65
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ({offsetY, kHeight}) => { | ||
| const correctedOffsetY = kHeight + offsetY; | ||
|
|
||
| scrollingVerticalOffsetRef.current = correctedOffsetY; | ||
| }, |
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.
Don’t mutate JS refs inside a Reanimated worklet
useAnimatedReaction runs on the UI thread; writing to scrollingVerticalOffsetRef.current inside the worklet won’t update the JS thread value. The ref is later read on the JS side in ReportActionsList to decide unread-marker placement and when to mark messages read, so it can stay at the initial value and treat the user as always at the bottom (e.g., unread messages marked read while scrolled up). Use a SharedValue end‑to‑end or runOnJS to update the JS ref.
Useful? React with 👍 / 👎.
|
thanks |
|
Today's update:
|
|
Update: Bumping |
|
Both iOS and Android work reliably well now, i've added screen recordings above. The only thing blocking us right now is this RKNC issue. Right now i have a workaround on iOS, which can cause a slight flicker since we have to omit the first keyboard drag event. Trying to work out the issue with @kirillzyusko and i'm addressing failing checks right now |
|
@kirillzyusko already found a fix for the RNKC issue mentioned above. The fix won't be published until new minor version We'll need to hold this PR on this RKNC version bump PR though. |
|
|
@roryabraham @trjExpensify @parasharrajat
Explanation of Change
Uses the new newly added components and logic from #71438 and migrates current list pages to integrate interactive keyboard dismissal.
This PR adapts the lists on:
ReportScreenMoneyRequestReportActionViewFixed Issues
$ #54270
PROPOSAL:
Tests
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand 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.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
Screen_Recording_20260130_114326_New.Expensify.mp4
iOS: Native
Simulator.Screen.Recording.-.iPhone.17.Pro.-.2026-01-29.at.17.09.45.mov