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.

Sign up for GitHub

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

Migrate linked_scroll_controller to null safety. #3623

Merged
merged 11 commits into from
Feb 4, 2022
Merged

Migrate linked_scroll_controller to null safety. #3623

Changes from 6 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
33fc2fe
Update linked_scroll_controller.dart
polina-c Feb 3, 2022
8183de2
Update linked_scroll_controller_test.dart
polina-c Feb 3, 2022
ff10512
Update linked_scroll_controller.dart
polina-c Feb 4, 2022
74c3f0e
Update linked_scroll_controller_test.dart
polina-c Feb 4, 2022
e3b5e93
Update linked_scroll_controller_test.dart
polina-c Feb 4, 2022
1a142a3
Revert "Update linked_scroll_controller_test.dart"
polina-c Feb 4, 2022
5849a02
Update linked_scroll_controller.dart
polina-c Feb 4, 2022
5699ebf
Update linked_scroll_controller.dart
polina-c Feb 4, 2022
902e74d
Update linked_scroll_controller.dart
polina-c Feb 4, 2022
cdac947
Update linked_scroll_controller.dart
polina-c Feb 4, 2022
1a5d833
Update linked_scroll_controller.dart
polina-c Feb 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 29 additions & 31 deletions packages/devtools_app/lib/src/primitives/flutter_widgets/linked_scroll_controller.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

// @dart=2.9

import 'package:flutter/material.dart';
import 'package:flutter/rendering.dart';

Expand All @@ -29,8 +27,8 @@ class LinkedScrollControllerGroup {

final _allControllers = <_LinkedScrollController>[];

ChangeNotifier get offsetNotifier => _offsetNotifier;
_LinkedScrollControllerGroupOffsetNotifier _offsetNotifier;
ChangeNotifier? get offsetNotifier => _offsetNotifier;
_LinkedScrollControllerGroupOffsetNotifier? _offsetNotifier;

bool get hasAttachedControllers => _attachedControllers.isNotEmpty;

Expand Down Expand Up @@ -62,18 +60,18 @@ class LinkedScrollControllerGroup {
final controller =
_LinkedScrollController(this, initialScrollOffset: initialScrollOffset);
_allControllers.add(controller);
controller.addListener(_offsetNotifier.notifyListeners);
controller.addListener(_offsetNotifier!.notifyListeners);
return controller;
}

/// Adds a callback that will be called when the value of [offset] changes.
void addOffsetChangedListener(VoidCallback onChanged) {
_offsetNotifier.addListener(onChanged);
_offsetNotifier!.addListener(onChanged);
}

/// Removes the specified offset changed listener.
void removeOffsetChangedListener(VoidCallback listener) {
_offsetNotifier.removeListener(listener);
_offsetNotifier!.removeListener(listener);
}

Iterable<_LinkedScrollController> get _attachedControllers =>
Expand All @@ -82,8 +80,8 @@ class LinkedScrollControllerGroup {
/// Animates the scroll position of all linked controllers to [offset].
Future<void> animateTo(
double offset, {
@required Curve curve,
@required Duration duration,
required Curve curve,
required Duration duration,
}) async {
// All scroll controllers are already linked with their peers, so we only
// need to interact with one controller to mirror the interaction with all
Expand Down Expand Up @@ -126,7 +124,7 @@ class _LinkedScrollControllerGroupOffsetNotifier extends ChangeNotifier {
/// The cached offset for the group.
///
/// This value will be used in determining whether to notify listeners.
double _cachedOffset;
double? _cachedOffset;

@override
void notifyListeners() {
Expand All @@ -141,7 +139,8 @@ class _LinkedScrollControllerGroupOffsetNotifier extends ChangeNotifier {
/// A scroll controller that mirrors its movements to a peer, which must also
/// be a [_LinkedScrollController].
class _LinkedScrollController extends ScrollController {
_LinkedScrollController(this._controllers, {double initialScrollOffset})
_LinkedScrollController(this._controllers,
{required double initialScrollOffset})
: super(initialScrollOffset: initialScrollOffset);

final LinkedScrollControllerGroup _controllers;
Expand All @@ -158,15 +157,16 @@ class _LinkedScrollController extends ScrollController {
position is _LinkedScrollPosition,
'_LinkedScrollControllers can only be used with'
' _LinkedScrollPositions.');
final _LinkedScrollPosition linkedPosition = position;
final _LinkedScrollPosition linkedPosition =
position as _LinkedScrollPosition;
assert(linkedPosition.owner == this,
'_LinkedScrollPosition cannot change controllers once created.');
super.attach(position);
}

@override
_LinkedScrollPosition createScrollPosition(ScrollPhysics physics,
ScrollContext context, ScrollPosition oldPosition) {
ScrollContext context, ScrollPosition? oldPosition) {
return _LinkedScrollPosition(
this,
physics: physics,
Expand All @@ -177,7 +177,7 @@ class _LinkedScrollController extends ScrollController {
}

@override
_LinkedScrollPosition get position => super.position;
_LinkedScrollPosition get position => super.position as _LinkedScrollPosition;

Iterable<_LinkedScrollController> get _allPeersWithClients =>
_controllers._attachedControllers.where((peer) => peer != this);
Expand All @@ -194,8 +194,8 @@ class _LinkedScrollController extends ScrollController {
Iterable<_LinkedScrollActivity> link(_LinkedScrollPosition driver) {
assert(hasClients);
final activities = <_LinkedScrollActivity>[];
for (_LinkedScrollPosition position in positions) {
activities.add(position.link(driver));
for (ScrollPosition position in positions) {
activities.add((position as _LinkedScrollPosition).link(driver));
}
return activities;
}
Expand All @@ -211,12 +211,11 @@ class _LinkedScrollController extends ScrollController {
class _LinkedScrollPosition extends ScrollPositionWithSingleContext {
_LinkedScrollPosition(
this.owner, {
ScrollPhysics physics,
ScrollContext context,
double initialPixels,
ScrollPosition oldPosition,
}) : assert(owner != null),
super(
required ScrollPhysics physics,
required ScrollContext context,
double? initialPixels,
ScrollPosition? oldPosition,
}) : super(
physics: physics,
context: context,
initialPixels: initialPixels,
Expand All @@ -238,13 +237,11 @@ class _LinkedScrollPosition extends ScrollPositionWithSingleContext {

// Calls hold without propagating to peers.
void _holdInternal() {
// TODO: passing null to hold seems fishy, but it doesn't
// appear to hurt anything. Revisit this if bad things happen.
super.hold(null);
super.hold(() {});
}

@override
void beginActivity(ScrollActivity newActivity) {
void beginActivity(ScrollActivity? newActivity) {
if (newActivity == null) {
return;
}
Expand Down Expand Up @@ -307,7 +304,8 @@ class _LinkedScrollPosition extends ScrollPositionWithSingleContext {
if (this.activity is! _LinkedScrollActivity) {
beginActivity(_LinkedScrollActivity(this));
}
final _LinkedScrollActivity activity = this.activity;
final _LinkedScrollActivity activity =
this.activity as _LinkedScrollActivity;
activity.link(driver);
return activity;
}
Expand All @@ -334,7 +332,7 @@ class _LinkedScrollActivity extends ScrollActivity {
_LinkedScrollActivity(_LinkedScrollPosition delegate) : super(delegate);

@override
_LinkedScrollPosition get delegate => super.delegate;
_LinkedScrollPosition get delegate => super.delegate as _LinkedScrollPosition;

final Set<_LinkedScrollPosition> drivers = <_LinkedScrollPosition>{};

Expand All @@ -345,7 +343,7 @@ class _LinkedScrollActivity extends ScrollActivity {
void unlink(_LinkedScrollPosition driver) {
drivers.remove(driver);
if (drivers.isEmpty) {
delegate?.goIdle();
delegate.goIdle();
}
}

Expand All @@ -372,14 +370,14 @@ class _LinkedScrollActivity extends ScrollActivity {

void _updateUserScrollDirection() {
assert(drivers.isNotEmpty);
ScrollDirection commonDirection;
ScrollDirection? commonDirection;
Copy link
Member

Choose a reason for hiding this comment

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

this looks like it can be late instead of nullable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It cannot because the line "commonDirection ??= driver.userScrollDirection" first read and then initializes.

for (var driver in drivers) {
commonDirection ??= driver.userScrollDirection;
if (driver.userScrollDirection != commonDirection) {
commonDirection = ScrollDirection.idle;
}
}
delegate.updateUserScrollDirection(commonDirection);
delegate.updateUserScrollDirection(commonDirection!);
}

@override
Expand Down