superlistapp
[FEATURE] - Allow vertical scrolling of a multi-line SuperTextField when its text overflows (Resolves #2140 and #1795)
I checked usages of TextScrollView and you're correct that it's only currently used for Android and iOS SuperTextFields.
Did you check what happens with auto-scrolling when dragging selection handles, dragging the caret, etc?
Wrapped around the TextScrollView is a AndroidTextFieldTouchInteractor and a IOSTextFieldTouchInteractor, respectively. Both of those interactors are given access to the scroll controller for the TextScrollView.
I assume there was a reason that I disabled the standard scroll physics. I'd like to find what that was and then ensure we didn't break those requirements with this change. If this does break those requirements, then we might need to find another way to accomplish this.
Also, let's move this out of draft mode, and can you also check the failing tests in CI?
@matthew-carroll - the tests I added for mobile and desktop are now passing. These tests check to see that multi-line SuperText fields can scroll up and down when the text overflows.
I'm not sure if I'll be able to help figure out the reason scroll physics were originally disabled.
And in terms of checking the existing functionality with selection handle, magnifiers, scrolling etc.: all existing tests pass, which I hope cover the rules in more detail than I can manually test. In any case I did some manual checks - see:
https://github.com/user-attachments/assets/a051b198-ef1c-4e51-9ecc-61c553491d20
Let me know if there is more you would like me to manually test, or if I should write any more tests.
I also now have some golden failures. I tried to update the goldens locally but it ended up with way too many unrelated files changed. Is there something I'm missing with regard to updating the goldens? I am running latest Flutter main:
Flutter 3.24.0-1.0.pre.153 • channel main • https://github.com/flutter/flutter.git
Framework • revision 49d6d520cb (45 minutes ago) • 2024-07-17 11:18:10 -0400
Engine • revision 7e25796340
Tools • Dart 3.6.0 (build 3.6.0-48.0.dev) • DevTools 2.37.1
@matthew-carroll all tests (including new tests & goldens) are now passing.
Not sure what to do about the Netlify/deploymant job failures.
It seems to be in the middle of setting up Flutter, then switches to stable - I guess this is a problem, as it should be on main/master?
I don't think anything I did should affect these jobs?
@matthew-carroll I think we should fix the goldens: merge either (https://github.com/superlistapp/super_editor/pull/2183) or (https://github.com/superlistapp/super_editor/pull/2184), and then I will rebase this PR.
I see that option 1 works and 2 does not, and also the Netlify jobs didn't fail in option 1 so probably will succeed here after I rebase this PR.
Please let me know how we should proceed.
@matthew-carroll this one has been rebased and tests are passing, and ready for another look!
@CillianMyles do you know if those demo site failures are new to this PR vs broken in general? I don't think I saw those failing in other PRs...
@matthew-carroll over the past few days/weeks working on a few PRs, I have noticed these fail sometimes. I can't remember in which PRs and it's a bit hard to track down older runs due to rebasing.
To be honest thought I can't think why the code changes in this PR could have any affect on these demo site failures.
Not sure if it's any use, but now come to think of it, something I do remember when I created the option 1 and option 2 PRs at virtually the same time, the PR checks were actually running at the same time on both, and I noted that the demo site jobs failed on one PR and not the other. Not sure if it could be anything to do with concurrent jobs.
Aside from that, I don't have any ideas unfortunately.
@matthew-carroll I rebased and addressed all your points. Looking forward to your feedback!