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 15 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
5 changes: 3 additions & 2 deletions packages/flutter/lib/src/cupertino/scrollbar.dart
Expand Up @@ -76,11 +76,11 @@ class _CupertinoScrollbarState extends State<CupertinoScrollbar> with TickerProv
void didChangeDependencies() {
super.didChangeDependencies();
_textDirection = Directionality.of(context);
_painter = _buildCupertinoScrollbarPainter();
_painter = _buildCupertinoScrollbarPainter(MediaQuery.of(context).padding);
LongCatIsLooong marked this conversation as resolved.
Show resolved Hide resolved
}

/// Returns a [ScrollbarPainter] visually styled like the iOS scrollbar.
ScrollbarPainter _buildCupertinoScrollbarPainter() {
ScrollbarPainter _buildCupertinoScrollbarPainter(EdgeInsets padding) {
return ScrollbarPainter(
color: _kScrollbarColor,
textDirection: _textDirection,
Expand All @@ -89,6 +89,7 @@ class _CupertinoScrollbarState extends State<CupertinoScrollbar> with TickerProv
mainAxisMargin: _kScrollbarMainAxisMargin,
crossAxisMargin: _kScrollbarCrossAxisMargin,
radius: _kScrollbarRadius,
padding: padding,
minLength: _kScrollbarMinLength,
minOverscrollLength: _kScrollbarMinOverscrollLength,
);
Expand Down
5 changes: 3 additions & 2 deletions packages/flutter/lib/src/material/scrollbar.dart
Expand Up @@ -93,17 +93,18 @@ class _ScrollbarState extends State<Scrollbar> with TickerProviderStateMixin {
case TargetPlatform.fuchsia:
_themeColor = theme.highlightColor.withOpacity(1.0);
_textDirection = Directionality.of(context);
_materialPainter = _buildMaterialScrollbarPainter();
_materialPainter = _buildMaterialScrollbarPainter(MediaQuery.of(context).padding);
LongCatIsLooong marked this conversation as resolved.
Show resolved Hide resolved
break;
}
}

ScrollbarPainter _buildMaterialScrollbarPainter() {
ScrollbarPainter _buildMaterialScrollbarPainter(EdgeInsets padding) {
return ScrollbarPainter(
color: _themeColor,
textDirection: _textDirection,
thickness: _kScrollbarThickness,
fadeoutOpacityAnimation: _fadeoutOpacityAnimation,
padding: 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
23 changes: 18 additions & 5 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 must not be null and must be 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 must not be null and must be greater than or equal to
LongCatIsLooong marked this conversation as resolved.
Show resolved Hide resolved
/// [minScrollExtent]. It can be infinity, if the scroll is unbounded.
double get maxScrollExtent;

/// The current scroll position, in logical pixels along the [axisDirection].
Expand Down Expand Up @@ -98,12 +100,23 @@ abstract class ScrollMetrics {
/// The quantity of visible content.
///
/// 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 height of the viewport. The value will always be greater than or equal
/// to the size of the viewport.
///
/// The implementation assumes [maxScrollExtent] is greater than or equal to
/// [minScrollExtent] and thus the value will always be greater than or equal
/// to [viewportDimension].
///
/// See also:
///
/// * [ViewportOffset.applyContentDimensions]
double get extentInside {
assert(maxScrollExtent != null);
assert(minScrollExtent != null);
assert(minScrollExtent <= maxScrollExtent);
return math.min(pixels, maxScrollExtent) -
math.max(pixels, minScrollExtent) +
math.min(viewportDimension, maxScrollExtent - minScrollExtent);
LongCatIsLooong marked this conversation as resolved.
Show resolved Hide resolved
viewportDimension;
}

/// The quantity of content conceptually "below" the currently visible content
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