Add optional axis specifier to static scrollable methods #124894

Piinks · 2023-04-14T21:36:43Z

This is motivated by part of the 2D scrolling proposal: flutter.dev/go/2D-Foundation

This is one of the last little PRs to prep for the 2D scrolling foundation.
This adds an optional axis parameter to the static Scrollable methods [of, maybeOf, recommendDeferredLoadingForContext]. This allows developers that are nesting scrollables (or one day using 2D scrolling) to look them up instead by a particular axis.

In general, even outside the context of 2D, I think this is helpful. I am often asked how to get the outer scrollable when nesting. Now it can be done.

There is also a small semantic refactor here in ScrollableState.build, this just creates a private method (_buildChrome) that will be overridden in 2D later. It is easier to add now than in the really big PR that will be.

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is test-exempt.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

goderbauer · 2023-04-17T17:51:41Z

Some checks are unhappy 😄

packages/flutter/lib/src/widgets/scrollable.dart

goderbauer · 2023-04-17T18:02:27Z

packages/flutter/lib/src/widgets/scrollable.dart

Do we need to adjust the error message below to indicate that maybe there was a scrollable in the context, it just didn't have the right axis?

packages/flutter/lib/src/widgets/scrollable.dart

goderbauer

LGTM

goderbauer · 2023-04-17T21:47:31Z

packages/flutter/lib/src/widgets/scrollable.dart

+        if (context != originalContext) {
+          // Establish the dependency on the correct context.
+          // Get the element.
+          final InheritedElement element = context.getElementForInheritedWidgetOfExactType<_ScrollableScope>()!;


To consider: I think I would find this code a little cleaner if you remove the getInheritedWidgetOfExactType calls altogether and instead replace them with getElementForInheritedWidgetOfExactType. From there you can access the scrollable via element.widget.scrollable (probably needs a cast). IMO, it makes it a little clearer that the element you depend on is the one you checked the axis on. Right now, it look a little like you look up one inherited widget, check something on it, and then look up another widget to depend on. (sure, in the end you look up the same thing, but it could be made explicit).

Maybe simplify further and remove the branching. The code inside the if is also correct when context == originalContext, right?

Ok! Will do, that would definitely be cleaner

auto-submit · 2023-04-17T23:10:08Z

auto label is removed for flutter/flutter, pr: 124894, due to - The status or check suite Windows framework_tests_misc has failed. Please fix the issues identified (or deflake) before re-applying this label.

…tter/flutter#124894)

flutter/flutter@15cb1f8...42fb0b2 2023-04-18 [email protected] Fix text theme dart fix cases (flutter/flutter#125052) 2023-04-18 [email protected] Update the copy icon in snippets and samples to use the standard one (flutter/flutter#123651) 2023-04-18 [email protected] Remove unused context parameter (flutter/flutter#124254) 2023-04-18 [email protected] iOS spell check cursor placement (flutter/flutter#124875) 2023-04-18 [email protected] Roll Flutter Engine from d2973619074e to 55bb065c607b (1 revision) (flutter/flutter#125047) 2023-04-18 [email protected] Stop running "_impeller_" benchmark variants (flutter/flutter#125044) 2023-04-18 [email protected] Roll Packages from 0277f2a to faf53fb (7 revisions) (flutter/flutter#125040) 2023-04-18 [email protected] Roll Flutter Engine from c4396f9f602f to d2973619074e (6 revisions) (flutter/flutter#125039) 2023-04-18 [email protected] Roll pub packages (flutter/flutter#125005) 2023-04-18 [email protected] [InputDatePickerFormField] adds acceptEmptyDate to InputDatePickerFormField Widget (flutter/flutter#124143) 2023-04-18 [email protected] relayout active ListWheelScrollView children every performLayout (flutter/flutter#124476) 2023-04-18 [email protected] Roll Flutter Engine from 4a603aaff32e to c4396f9f602f (2 revisions) (flutter/flutter#125007) 2023-04-18 [email protected] Roll Flutter Engine from 20034a8d62c4 to 4a603aaff32e (2 revisions) (flutter/flutter#125004) 2023-04-18 [email protected] Add optional axis specifier to static scrollable methods (flutter/flutter#124894) 2023-04-17 [email protected] Update usage of standalone`pub` executable in flutter_tools testing docs (flutter/flutter#124898) 2023-04-17 [email protected] Add Harish Anbalagan to AUTHORS (flutter/flutter#124684) 2023-04-17 [email protected] Roll Flutter Engine from b2d07388ceb6 to 20034a8d62c4 (7 revisions) (flutter/flutter#125001) 2023-04-17 [email protected] Add an example for SearchBar (flutter/flutter#124992) 2023-04-17 [email protected] SelectionContainer's listeners can remove itself during listener callâ�¦ (flutter/flutter#124624) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

Piinks added c: new feature Nothing broken; request for a new capability framework flutter/packages/flutter repository. See also f: labels. f: scrolling Viewports, list views, slivers, etc. labels Apr 14, 2023

Piinks requested a review from goderbauer April 14, 2023 21:36

goderbauer reviewed Apr 17, 2023

View reviewed changes

Piinks added 3 commits April 17, 2023 16:16

Add optional axis specifier to static scrollable methods

fa5e646

++

a9b669f

Remove comment

9cf8a1e

Piinks force-pushed the staticScrollable branch from de147dd to 9cf8a1e Compare April 17, 2023 21:17

Fix doc

98a90da

Piinks requested a review from goderbauer April 17, 2023 21:34

goderbauer approved these changes Apr 17, 2023

View reviewed changes

Piinks added 2 commits April 17, 2023 17:22

Simplify

d916f73

Nit

b7006dd

Piinks added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 17, 2023

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Apr 17, 2023

Piinks added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 17, 2023

auto-submit bot merged commit e867d1c into flutter:master Apr 18, 2023

engine-flutter-autoroll mentioned this pull request Apr 18, 2023

Manual roll Flutter from 15cb1f84d726 to 3ba249a6b37c (9 revisions) flutter/packages#3750

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 18, 2023

e867d1c Add optional axis specifier to static scrollable methods (flu…

58eff9e

…tter/flutter#124894)

engine-flutter-autoroll mentioned this pull request Apr 18, 2023

Roll Flutter from 15cb1f84d726 to b0346b295347 (11 revisions) flutter/packages#3754

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 18, 2023

e867d1c Add optional axis specifier to static scrollable methods (flu…

a52acfa

…tter/flutter#124894)

engine-flutter-autoroll mentioned this pull request Apr 18, 2023

Roll Flutter from 15cb1f84d726 to ec02eb35ad12 (12 revisions) flutter/packages#3755

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 18, 2023

e867d1c Add optional axis specifier to static scrollable methods (flu…

344e8da

…tter/flutter#124894)

engine-flutter-autoroll mentioned this pull request Apr 18, 2023

Roll Flutter from 15cb1f84d726 to 42fb0b23139c (19 revisions) flutter/packages#3756

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 18, 2023

e867d1c Add optional axis specifier to static scrollable methods (flu…

b190a7b

…tter/flutter#124894)

reidbaker mentioned this pull request Apr 20, 2023

i123636 android embedding v2 agp gradle kotlin update #125256

Closed

8 tasks

reidbaker mentioned this pull request Apr 21, 2023

[integration_test] add support for agp8.0 #125310

Closed

8 tasks

HansMuller mentioned this pull request Apr 21, 2023

Flutter Framework Team: Notable Commits #121415

Closed

reidbaker mentioned this pull request Apr 21, 2023

Test AGP 8.0 using java 17 #125323

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add optional axis specifier to static scrollable methods #124894

Add optional axis specifier to static scrollable methods #124894

Uh oh!

Piinks commented Apr 14, 2023

Uh oh!

goderbauer commented Apr 17, 2023

Uh oh!

Uh oh!

Uh oh!

goderbauer Apr 17, 2023

Uh oh!

Uh oh!

Uh oh!

goderbauer left a comment

Uh oh!

goderbauer Apr 17, 2023

Uh oh!

goderbauer Apr 17, 2023

Uh oh!

Piinks Apr 17, 2023

Uh oh!

auto-submit bot commented Apr 17, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Add optional axis specifier to static scrollable methods #124894

Add optional axis specifier to static scrollable methods #124894

Uh oh!

Conversation

Piinks commented Apr 14, 2023

Pre-launch Checklist

Uh oh!

goderbauer commented Apr 17, 2023

Uh oh!

Uh oh!

Uh oh!

goderbauer Apr 17, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

goderbauer left a comment

Choose a reason for hiding this comment

Uh oh!

goderbauer Apr 17, 2023

Choose a reason for hiding this comment

Uh oh!

goderbauer Apr 17, 2023

Choose a reason for hiding this comment

Uh oh!

Piinks Apr 17, 2023

Choose a reason for hiding this comment

Uh oh!

auto-submit bot commented Apr 17, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants