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

Fix ScrollbarPainter thumbExtent calculation and add padding #31763

Merged
Merged
Show file tree
Hide file tree
Changes from 40 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
c46cec9
WIP
LongCatIsLooong Apr 25, 2019
42bf521
page calculation
LongCatIsLooong Apr 25, 2019
ef0a54e
WIP
LongCatIsLooong Apr 25, 2019
a2cec76
WIP
LongCatIsLooong Apr 26, 2019
4432ff3
scrollbar x position
LongCatIsLooong Apr 26, 2019
134df49
fix tests
LongCatIsLooong Apr 26, 2019
7792e4d
WIP
LongCatIsLooong Apr 27, 2019
0386126
add tests
LongCatIsLooong Apr 27, 2019
71edbc8
Merge remote-tracking branch 'upstream/master' into scrollable-obstru…
LongCatIsLooong Apr 27, 2019
4e4c640
Merge remote-tracking branch 'upstream/master' into scrollable-obstru…
LongCatIsLooong Apr 27, 2019
c06299f
fix tests
LongCatIsLooong Apr 28, 2019
4433a94
more tests
LongCatIsLooong Apr 28, 2019
ffddf9c
remove newline
LongCatIsLooong Apr 28, 2019
5833e34
typo
LongCatIsLooong Apr 28, 2019
96924ec
comments
LongCatIsLooong Apr 29, 2019
74c79a8
replace new syntax
LongCatIsLooong Apr 29, 2019
6a62e9c
Merge branch 'scrollable-obstructions' of github.com:LongCatIsLooong/…
LongCatIsLooong Apr 29, 2019
2532fdb
Merge branch 'master' into scrollable-obstructions
LongCatIsLooong Apr 30, 2019
5e1e04a
Review
LongCatIsLooong Apr 30, 2019
f267818
review
LongCatIsLooong Apr 30, 2019
1d94387
Use ready-made wheels
LongCatIsLooong Apr 30, 2019
5aaf9d1
WIP, updating according to iOS behavior
LongCatIsLooong May 1, 2019
0ab4191
WIP
LongCatIsLooong May 1, 2019
7eaacda
Update thumbExtent calculation
LongCatIsLooong May 1, 2019
8e31413
fix extentInside
LongCatIsLooong May 1, 2019
12885d2
Could this be working?
LongCatIsLooong May 2, 2019
a78021f
fix docs
LongCatIsLooong May 2, 2019
6ded32b
english
LongCatIsLooong May 2, 2019
4d3e39e
review, WIP
LongCatIsLooong May 7, 2019
2357997
add new test for material
LongCatIsLooong May 7, 2019
4271b67
Update scrollbar.dart
LongCatIsLooong May 7, 2019
387c5cd
update comments and tests
LongCatIsLooong May 7, 2019
5578ad2
Merge branch 'scrollable-obstructions' of github.com:LongCatIsLooong/…
LongCatIsLooong May 7, 2019
128c12b
indentation
LongCatIsLooong May 7, 2019
535d25c
change wording
LongCatIsLooong May 9, 2019
be616d3
review
LongCatIsLooong May 9, 2019
d6c9632
remove whitespace
LongCatIsLooong May 13, 2019
67d9e58
trailing comma
LongCatIsLooong May 29, 2019
2bea4cf
pixel -> logical pixel
LongCatIsLooong May 29, 2019
54d37d1
must -> should
LongCatIsLooong May 29, 2019
ff2fff9
Merge branch 'master' into scrollable-obstructions
LongCatIsLooong May 29, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 11 additions & 3 deletions packages/flutter/lib/src/cupertino/scrollbar.dart
Expand Up @@ -8,15 +8,22 @@ import 'package:flutter/widgets.dart';

// All values eyeballed.
const Color _kScrollbarColor = Color(0x99777777);
const double _kScrollbarThickness = 2.5;
const double _kScrollbarMainAxisMargin = 4.0;
const double _kScrollbarCrossAxisMargin = 2.5;
const double _kScrollbarMinLength = 36.0;
const double _kScrollbarMinOverscrollLength = 8.0;
const Radius _kScrollbarRadius = Radius.circular(1.25);
const Duration _kScrollbarTimeToFade = Duration(milliseconds: 50);
const Duration _kScrollbarFadeDuration = Duration(milliseconds: 250);

// These values are measured using screenshots from an iPhone XR 12.1 simulator.
const double _kScrollbarThickness = 2.5;
// This is the amount of space from the top of a vertical scrollbar to the
// top edge of the scrollable, measured when the vertical scrollbar overscrolls
// to the top.
// TODO(LongCatIsLooong): fix https://github.com/flutter/flutter/issues/32175
const double _kScrollbarMainAxisMargin = 3.0;
const double _kScrollbarCrossAxisMargin = 3.0;


/// An iOS style scrollbar.
///
/// A scrollbar indicates which portion of a [Scrollable] widget is actually
Expand Down Expand Up @@ -89,6 +96,7 @@ class _CupertinoScrollbarState extends State<CupertinoScrollbar> with TickerProv
mainAxisMargin: _kScrollbarMainAxisMargin,
crossAxisMargin: _kScrollbarCrossAxisMargin,
radius: _kScrollbarRadius,
padding: MediaQuery.of(context).padding,
minLength: _kScrollbarMinLength,
minOverscrollLength: _kScrollbarMinOverscrollLength,
);
Expand Down
1 change: 1 addition & 0 deletions packages/flutter/lib/src/material/scrollbar.dart
Expand Up @@ -104,6 +104,7 @@ class _ScrollbarState extends State<Scrollbar> with TickerProviderStateMixin {
textDirection: _textDirection,
thickness: _kScrollbarThickness,
fadeoutOpacityAnimation: _fadeoutOpacityAnimation,
padding: MediaQuery.of(context).padding,
);
}

Expand Down
5 changes: 3 additions & 2 deletions packages/flutter/lib/src/rendering/viewport_offset.dart
Expand Up @@ -123,8 +123,9 @@ abstract class ViewportOffset extends ChangeNotifier {
/// Called when the viewport's content extents are established.
///
/// The arguments are the minimum and maximum scroll extents respectively. The
/// minimum will be equal to or less than zero, the maximum will be equal to
/// or greater than zero.
/// minimum will be equal to or less than the maximum. In the case of slivers,
/// the minimum will be equal to or less than zero, the maximum will be equal
/// to or greater than zero.
///
/// The maximum scroll extent has the viewport dimension subtracted from it.
/// For instance, if there is 100.0 pixels of scrollable content, and the
Expand Down
34 changes: 19 additions & 15 deletions packages/flutter/lib/src/widgets/scroll_metrics.dart
Expand Up @@ -60,14 +60,16 @@ abstract class ScrollMetrics {
///
/// The actual [pixels] value might be [outOfRange].
///
/// This value can be negative infinity, if the scroll is unbounded.
/// This value should typically be non-null and less than or equal to
/// [maxScrollExtent]. It can be negative infinity, if the scroll is unbounded.
double get minScrollExtent;
Copy link
Contributor

Choose a reason for hiding this comment

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

FixedScrollMetrics should assert that these constraints are true.


/// The maximum in-range value for [pixels].
///
/// The actual [pixels] value might be [outOfRange].
///
/// This value can be infinity, if the scroll is unbounded.
/// This value should typically be non-null and greater than or equal to
/// [minScrollExtent]. It can be infinity, if the scroll is unbounded.
double get maxScrollExtent;

/// The current scroll position, in logical pixels along the [axisDirection].
Expand All @@ -90,25 +92,27 @@ abstract class ScrollMetrics {
/// [maxScrollExtent].
bool get atEdge => pixels == minScrollExtent || pixels == maxScrollExtent;

/// The quantity of content conceptually "above" the currently visible content
/// of the viewport in the scrollable. This is the content above the content
/// described by [extentInside].
/// The quantity of content conceptually "above" the viewport in the scrollable.
/// This is the content above the content described by [extentInside].
double get extentBefore => math.max(pixels - minScrollExtent, 0.0);

/// The quantity of visible content.
/// The quantity of content conceptually "inside" the viewport in the scrollable.
///
/// If [extentBefore] and [extentAfter] are non-zero, then this is typically
/// the height of the viewport. It could be less if there is less content
/// visible than the size of the viewport.
/// The value is typically the height of the viewport when [outOfRange] is false.
/// It could be less if there is less content visible than the size of the
/// viewport, such as when overscrolling.
///
/// The value is always non-negative, and less than or equal to [viewportDimension].
double get extentInside {
return math.min(pixels, maxScrollExtent) -
math.max(pixels, minScrollExtent) +
math.min(viewportDimension, maxScrollExtent - minScrollExtent);
LongCatIsLooong marked this conversation as resolved.
Show resolved Hide resolved
return viewportDimension
// "above" overscroll value
- (minScrollExtent - pixels).clamp(0, viewportDimension)
// "below" overscroll value
- (pixels - maxScrollExtent).clamp(0, viewportDimension);
}

/// The quantity of content conceptually "below" the currently visible content
/// of the viewport in the scrollable. This is the content below the content
/// described by [extentInside].
/// The quantity of content conceptually "below" the viewport in the scrollable.
/// This is the content below the content described by [extentInside].
double get extentAfter => math.max(maxScrollExtent - pixels, 0.0);
}

Expand Down
3 changes: 3 additions & 0 deletions packages/flutter/lib/src/widgets/scroll_position.dart
Expand Up @@ -450,6 +450,9 @@ abstract class ScrollPosition extends ViewportOffset with ScrollMetrics {
if (!nearEqual(_minScrollExtent, minScrollExtent, Tolerance.defaultTolerance.distance) ||
!nearEqual(_maxScrollExtent, maxScrollExtent, Tolerance.defaultTolerance.distance) ||
_didChangeViewportDimensionOrReceiveCorrection) {
assert(minScrollExtent != null);
assert(maxScrollExtent != null);
assert(minScrollExtent <= maxScrollExtent);
LongCatIsLooong marked this conversation as resolved.
Show resolved Hide resolved
_minScrollExtent = minScrollExtent;
_maxScrollExtent = maxScrollExtent;
_haveDimensions = true;
Expand Down