Skip to content
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

[SuperEditor] Fix scrolling issue when the editor is inside a horizontal Scrollable (Resolves #501) #708

Merged
merged 8 commits into from
Jul 26, 2022

Conversation

angelosilvestre
Copy link
Collaborator

[SuperEditor] Fix scrolling issue when the editor is inside a horizontal Scrollable. Resolves #501

When adding the editor to a TabBarView, once the screen is filled with text, the TabBarView automatically changes to the next tab rather than scroll down in the editor.

A similar issue happens when adding the editor inside a horizontal ListView, where adding text causes the ListView to scroll.

The problem is that when SuperEditor has an ancestor Scrollable, it doesn't create its own SingleChildScrollView and scrolls the ancestor Scrollable instead. Because of that, widgets that use a horizontal Scrollable are affected.

I changed it to ignore the ancestor Scrollable if it has a horizontal AxisDirection.

I also added an option in TestDocumentConfigurator to configure the subtree between MaterialApp and SuperEditor.

@angelosilvestre angelosilvestre marked this pull request as draft July 24, 2022 17:12
@angelosilvestre
Copy link
Collaborator Author

@matthew-carroll I changed SuperEditor to pass its scrollController to DocumentScrollable because it was being ignored.

@angelosilvestre angelosilvestre marked this pull request as ready for review July 24, 2022 17:41
@@ -41,6 +42,10 @@ class DocumentScrollable extends StatefulWidget {
/// debugging, when `true`.
final bool showDebugPaint;

/// The [ScrollController] that governs this [DocumentScrollable]'s scroll
/// offset.
Copy link
Contributor

Choose a reason for hiding this comment

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

This controller isn't used when there's an ancestor Scrollable, right? If so, you should probably mention that here, and also mention it in SuperEditor's docs for its ScrollController.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added the comment in DocumentScrollable. It was already present in SuperEditor

Copy link
Contributor

@matthew-carroll matthew-carroll left a comment

Choose a reason for hiding this comment

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

LGTM - I left one last request for a clarifying comment.

(superEditor) => MaterialApp(
home: ConstrainedBox(
constraints: const BoxConstraints(
minWidth: 300,
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a comment about why we need such a tiny editor for the test?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

@matthew-carroll matthew-carroll merged commit f8aec2e into superlistapp:main Jul 26, 2022
@angelosilvestre angelosilvestre deleted the issue-501 branch July 26, 2022 22:53
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.

Problem using the SuperEditor with a TabBar
2 participants