Skip to content

Commit

Permalink
Fix ScrollbarPainter thumbExtent calculation and add padding (#31763)
Browse files Browse the repository at this point in the history
- Fixed extentInside calculation in ScrollMetrics
- Added asserts to extentInside getter, as well as ScrollPosition.applyContentDimensions to enforce minScrollExtent <= maxScrollExtent
- Added padding to ScrollbarPainter, updated implementation. Took care of some edge cases.
- Changed some scroll bar constants on Cupertino side.
  • Loading branch information
LongCatIsLooong committed May 29, 2019
1 parent c926aae commit 22ea031
Show file tree
Hide file tree
Showing 12 changed files with 872 additions and 274 deletions.
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;

/// 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);
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);
_minScrollExtent = minScrollExtent;
_maxScrollExtent = maxScrollExtent;
_haveDimensions = true;
Expand Down

0 comments on commit 22ea031

Please sign in to comment.