Skip to content

@relextm19/scroll position - #850

Open
relextm19 wants to merge 34 commits into
mainfrom
@relextm19/scroll-position
Open

relextm19 wants to merge 34 commits into
mainfrom
@relextm19/scroll-position

Conversation

@relextm19

@relextm19 relextm19 commented Oct 29, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Closes #169

Add onScroll event to EnrichedTextInput triggered when the content is scrolled. Payload is the same as in the React Native implementation

nativeEvent:{
  contentOffset: {
      x: number;
      y: number;
    };
}

Test Plan

Check the metro logs when running the mobile example or browser logs when running the web test. You can also write your own handler for onScroll and test it yourself

Screenshots / Videos

Compatibility

OS Implemented
iOS ✅
Android ✅
Web ✅

Checklist

  • E2E tests are passing
  • Required E2E tests have been added (if applicable)

Copilot AI balanced review requested due to automatic review settings October 29, 2026 11:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The new fullscreen row invalidates existing screenshot baselines, and scroll events have cross-platform throttling, unit, and precision errors.

6 open issues

Review effort: Balanced

Comment thread apps/example/src/screens/TestScreen.tsx Resolved
Comment thread src/web/EnrichedTextInput.tsx Resolved
Comment thread src/web/EnrichedTextInput.tsx Resolved
@relextm19
relextm19 marked this pull request as draft October 29, 2026 19:32
@relextm19
relextm19 force-pushed the @relextm19/scroll-position branch from e7a6c07 to 2e5d181 Compare October 30, 2026 11:33
@relextm19
relextm19 marked this pull request as ready for review November 4, 2026 12:06

@exploIF exploIF left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should use similar approach to what we are doing for onChangeHtml. If handler is not set up we should avoid any calculations. See how isOnChangeTextSet is implemented

Comment thread apps/example/src/screens/TestScreen.tsx Resolved
@relextm19
relextm19 force-pushed the @relextm19/scroll-position branch from 25a8a54 to 17fe873 Compare November 11, 2026 16:58
@szydlovsky
szydlovsky self-requested a review November 12, 2026 10:39

@szydlovsky szydlovsky left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did a checkup over the layoutSubviews change that bugged me and it looks legit!

@szydlovsky
szydlovsky requested a review from exploIF November 12, 2026 11:55
@exploIF

exploIF commented Nov 12, 2026

Copy link
Copy Markdown
Collaborator

Code-wise looks good, asked @szydlovsky to do a final testing check 🙌

@szydlovsky

Copy link
Copy Markdown
Collaborator

@relextm19 actually, I'm seeing some strange Android value jumps from the onScroll, specifically when we're still within the maxHeight (so theoretically input should have no scroll offset) and adding some new lines.

Screen.Recording.2026-11-12.at.18.28.38.mov

Having said that, I think it might be rather an issue of our jumps on layout when adding new lines, rather than the actual offset calculation. The issue is probably unrelated to this PR but I'll do some digging and check what happens on the current main. Maybe we can just block event's emitting as long as the layout process is running?

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature request] scroll position

4 participants