Skip to content

Commit a5934ee

Browse files
authored
fix(scrollbar): Update padding type to EdgeInsetsGeometry (flutter#172056)
<!-- Thanks for filing a pull request! Reviewers are typically assigned within a week of filing a request. To learn more about code review, see our documentation on Tree Hygiene: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md --> This PR enhances RTL support for the RawScrollbar widget by updating its padding type and adding comprehensive tests. The changes include: 1. Updating RawScrollbar's `padding` parameter from `EdgeInsets` to `EdgeInsetsGeometry` - This change is non-breaking because `EdgeInsets` is a subclass of `EdgeInsetsGeometry` - Existing code using `EdgeInsets` will continue to work as before - Users can now optionally use `EdgeInsetsDirectional` for RTL support 2. Adding a test case that verifies scrollbar positioning with EdgeInsetsDirectional padding 3. Testing both overscroll and regular scroll scenarios with directional padding These changes ensure that the scrollbar works correctly in both LTR and RTL layouts, improving accessibility and internationalization support. Fixes: flutter#167922 ## Pre-launch Checklist - [x] I read the [Contributor Guide](https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview) and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene](https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md) wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide](https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md), including [Features we expect every widget to implement](https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement). - [x] I signed the [CLA](https://cla.developers.google.com/). - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord](https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md). <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
1 parent 5f9b9ab commit a5934ee

2 files changed

Lines changed: 98 additions & 16 deletions

File tree

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

Lines changed: 33 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,9 @@ enum ScrollbarOrientation {
6464
/// proportional to the percentage of content completely visible on screen,
6565
/// as long as its size isn't less than [minLength] and it isn't overscrolling.
6666
///
67+
/// If [padding] is an [EdgeInsetsDirectional], a non-null [textDirection] must
68+
/// be provided to properly resolve the padding values.
69+
///
6770
/// Unlike [CustomPainter]s that subclasses [CustomPainter] and only repaint
6871
/// when [shouldRepaint] returns true (which requires this [CustomPainter] to
6972
/// be rebuilt), this painter has the added optimization of repainting and not
@@ -95,7 +98,7 @@ class ScrollbarPainter extends ChangeNotifier implements CustomPainter {
9598
Color trackBorderColor = const Color(0x00000000),
9699
TextDirection? textDirection,
97100
double thickness = _kScrollbarThickness,
98-
EdgeInsets padding = EdgeInsets.zero,
101+
EdgeInsetsGeometry padding = EdgeInsets.zero,
99102
double mainAxisMargin = 0.0,
100103
double crossAxisMargin = 0.0,
101104
Radius? radius,
@@ -110,12 +113,17 @@ class ScrollbarPainter extends ChangeNotifier implements CustomPainter {
110113
assert(minOverscrollLength == null || minOverscrollLength <= minLength),
111114
assert(minOverscrollLength == null || minOverscrollLength >= 0),
112115
assert(padding.isNonNegative),
116+
assert(
117+
padding is! EdgeInsetsDirectional || textDirection != null,
118+
'A non-null textDirection must be provided when using EdgeInsetsDirectional for padding.',
119+
),
113120
_color = color,
114121
_textDirection = textDirection,
115122
_thickness = thickness,
116123
_radius = radius,
117124
_shape = shape,
118125
_padding = padding,
126+
_resolvedPadding = padding.resolve(textDirection),
119127
_mainAxisMargin = mainAxisMargin,
120128
_crossAxisMargin = crossAxisMargin,
121129
_minLength = minLength,
@@ -190,6 +198,7 @@ class ScrollbarPainter extends ChangeNotifier implements CustomPainter {
190198
}
191199

192200
_textDirection = value;
201+
_resolvedPadding = _padding.resolve(_textDirection);
193202
notifyListeners();
194203
}
195204

@@ -290,14 +299,20 @@ class ScrollbarPainter extends ChangeNotifier implements CustomPainter {
290299
///
291300
/// Defaults to [EdgeInsets.zero]. Offsets from all four directions must be
292301
/// greater than or equal to zero.
293-
EdgeInsets get padding => _padding;
294-
EdgeInsets _padding;
295-
set padding(EdgeInsets value) {
302+
///
303+
/// For RTL (right-to-left) support, you can provide [EdgeInsetsDirectional],
304+
/// but you must also provide a non-null [textDirection] to properly resolve
305+
/// the padding values. The scrollbar will automatically adjust the padding
306+
/// based on the text direction.
307+
EdgeInsetsGeometry get padding => _padding;
308+
EdgeInsetsGeometry _padding;
309+
set padding(EdgeInsetsGeometry value) {
296310
if (padding == value) {
297311
return;
298312
}
299313

300314
_padding = value;
315+
_resolvedPadding = _padding.resolve(_textDirection);
301316
notifyListeners();
302317
}
303318

@@ -388,17 +403,19 @@ class ScrollbarPainter extends ChangeNotifier implements CustomPainter {
388403
// - Scrollbar Details
389404

390405
Rect? _trackRect;
406+
EdgeInsets? _resolvedPadding;
391407
// The full painted length of the track
392408
double get _trackExtent => _lastMetrics!.viewportDimension - _totalTrackMainAxisOffsets;
393409
// The full length of the track that the thumb can travel
394410
double get _traversableTrackExtent => _trackExtent - (2 * mainAxisMargin);
395411
// Track Offsets
396412
// The track is offset by only padding.
397-
double get _totalTrackMainAxisOffsets => _isVertical ? padding.vertical : padding.horizontal;
413+
double get _totalTrackMainAxisOffsets =>
414+
_isVertical ? _resolvedPadding!.vertical : _resolvedPadding!.horizontal;
398415

399416
double get _leadingTrackMainAxisOffset => switch (_resolvedOrientation) {
400-
ScrollbarOrientation.left || ScrollbarOrientation.right => padding.top,
401-
ScrollbarOrientation.top || ScrollbarOrientation.bottom => padding.left,
417+
ScrollbarOrientation.left || ScrollbarOrientation.right => _resolvedPadding!.top,
418+
ScrollbarOrientation.top || ScrollbarOrientation.bottom => _resolvedPadding!.left,
402419
};
403420

404421
Rect? _thumbRect;
@@ -557,7 +574,6 @@ class ScrollbarPainter extends ChangeNotifier implements CustomPainter {
557574
textDirection != null,
558575
'A TextDirection must be provided before a Scrollbar can be painted.',
559576
);
560-
561577
final double x, y;
562578
final Size thumbSize, trackSize;
563579
final Offset trackOffset, borderStart, borderEnd;
@@ -566,15 +582,15 @@ class ScrollbarPainter extends ChangeNotifier implements CustomPainter {
566582
case ScrollbarOrientation.left:
567583
thumbSize = Size(thickness, _thumbExtent);
568584
trackSize = Size(thickness + 2 * crossAxisMargin, _trackExtent);
569-
x = crossAxisMargin + padding.left;
585+
x = crossAxisMargin + _resolvedPadding!.left;
570586
y = _thumbOffset;
571587
trackOffset = Offset(x - crossAxisMargin, _leadingTrackMainAxisOffset);
572588
borderStart = trackOffset + Offset(trackSize.width, 0.0);
573589
borderEnd = Offset(trackOffset.dx + trackSize.width, trackOffset.dy + _trackExtent);
574590
case ScrollbarOrientation.right:
575591
thumbSize = Size(thickness, _thumbExtent);
576592
trackSize = Size(thickness + 2 * crossAxisMargin, _trackExtent);
577-
x = size.width - thickness - crossAxisMargin - padding.right;
593+
x = size.width - thickness - crossAxisMargin - _resolvedPadding!.right;
578594
y = _thumbOffset;
579595
trackOffset = Offset(x - crossAxisMargin, _leadingTrackMainAxisOffset);
580596
borderStart = trackOffset;
@@ -583,15 +599,15 @@ class ScrollbarPainter extends ChangeNotifier implements CustomPainter {
583599
thumbSize = Size(_thumbExtent, thickness);
584600
trackSize = Size(_trackExtent, thickness + 2 * crossAxisMargin);
585601
x = _thumbOffset;
586-
y = crossAxisMargin + padding.top;
602+
y = crossAxisMargin + _resolvedPadding!.top;
587603
trackOffset = Offset(_leadingTrackMainAxisOffset, y - crossAxisMargin);
588604
borderStart = trackOffset + Offset(0.0, trackSize.height);
589605
borderEnd = Offset(trackOffset.dx + _trackExtent, trackOffset.dy + trackSize.height);
590606
case ScrollbarOrientation.bottom:
591607
thumbSize = Size(_thumbExtent, thickness);
592608
trackSize = Size(_trackExtent, thickness + 2 * crossAxisMargin);
593609
x = _thumbOffset;
594-
y = size.height - thickness - crossAxisMargin - padding.bottom;
610+
y = size.height - thickness - crossAxisMargin - _resolvedPadding!.bottom;
595611
trackOffset = Offset(_leadingTrackMainAxisOffset, y - crossAxisMargin);
596612
borderStart = trackOffset;
597613
borderEnd = Offset(trackOffset.dx + _trackExtent, trackOffset.dy);
@@ -634,6 +650,7 @@ class ScrollbarPainter extends ChangeNotifier implements CustomPainter {
634650
if (_lastAxisDirection == null || !_needPaint(_lastMetrics)) {
635651
return;
636652
}
653+
637654
// Skip painting if there's not enough space.
638655
if (_traversableTrackExtent <= 0) {
639656
return;
@@ -1320,7 +1337,7 @@ class RawScrollbar extends StatefulWidget {
13201337
/// When null, the inherited [MediaQueryData.padding] is used.
13211338
///
13221339
/// Defaults to null.
1323-
final EdgeInsets? padding;
1340+
final EdgeInsetsGeometry? padding;
13241341

13251342
@override
13261343
RawScrollbarState<RawScrollbar> createState() => RawScrollbarState<RawScrollbar>();
@@ -1543,6 +1560,7 @@ class RawScrollbarState<T extends RawScrollbar> extends State<T> with TickerProv
15431560
/// Subclasses can override to configure the [scrollbarPainter].
15441561
@protected
15451562
void updateScrollbarPainter() {
1563+
final TextDirection textDirection = Directionality.of(context);
15461564
scrollbarPainter
15471565
..color = widget.thumbColor ?? const Color(0x66BCBCBC)
15481566
..trackRadius = widget.trackRadius
@@ -1552,10 +1570,10 @@ class RawScrollbarState<T extends RawScrollbar> extends State<T> with TickerProv
15521570
..trackBorderColor = _showTrack
15531571
? widget.trackBorderColor ?? const Color(0x1a000000)
15541572
: const Color(0x00000000)
1555-
..textDirection = Directionality.of(context)
1573+
..textDirection = textDirection
15561574
..thickness = widget.thickness ?? _kScrollbarThickness
15571575
..radius = widget.radius
1558-
..padding = widget.padding ?? MediaQuery.paddingOf(context)
1576+
..padding = (widget.padding ?? MediaQuery.paddingOf(context)).resolve(textDirection)
15591577
..scrollbarOrientation = widget.scrollbarOrientation
15601578
..mainAxisMargin = widget.mainAxisMargin
15611579
..shape = widget.shape

packages/flutter/test/widgets/scrollbar_test.dart

Lines changed: 65 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ const Duration _kScrollbarTimeToFade = Duration(milliseconds: 600);
1717

1818
ScrollbarPainter _buildPainter({
1919
TextDirection textDirection = TextDirection.ltr,
20-
EdgeInsets padding = EdgeInsets.zero,
20+
EdgeInsetsGeometry padding = EdgeInsets.zero,
2121
Color color = _kScrollbarColor,
2222
double thickness = _kThickness,
2323
double mainAxisMargin = 0.0,
@@ -3567,4 +3567,68 @@ The provided ScrollController cannot be shared by multiple ScrollView widgets.''
35673567
},
35683568
variant: TargetPlatformVariant.desktop(),
35693569
);
3570+
3571+
test('with EdgeInsetsDirectional', () {
3572+
const Size size = Size(60, 80);
3573+
final ScrollMetrics metrics = defaultMetrics.copyWith(
3574+
minScrollExtent: -100,
3575+
maxScrollExtent: 240,
3576+
axisDirection: AxisDirection.down,
3577+
);
3578+
3579+
final ScrollbarPainter ltrPainter = _buildPainter(
3580+
padding: const EdgeInsetsDirectional.fromSTEB(1, 2, 3, 4),
3581+
scrollMetrics: metrics,
3582+
);
3583+
3584+
ltrPainter.update(
3585+
metrics.copyWith(viewportDimension: size.height, pixels: double.negativeInfinity),
3586+
AxisDirection.down,
3587+
);
3588+
3589+
// Top overscroll.
3590+
ltrPainter.paint(testCanvas, size);
3591+
final Rect ltrRect0 = captureRect();
3592+
expect(ltrRect0.top, 2);
3593+
expect(size.width - ltrRect0.right, 3);
3594+
3595+
// Bottom overscroll.
3596+
ltrPainter.update(
3597+
metrics.copyWith(viewportDimension: size.height, pixels: double.infinity),
3598+
AxisDirection.down,
3599+
);
3600+
3601+
ltrPainter.paint(testCanvas, size);
3602+
final Rect ltrRect1 = captureRect();
3603+
expect(size.height - ltrRect1.bottom, 4);
3604+
expect(size.width - ltrRect1.right, 3);
3605+
3606+
final ScrollbarPainter rtlPainter = _buildPainter(
3607+
padding: const EdgeInsetsDirectional.fromSTEB(1, 2, 3, 4),
3608+
scrollMetrics: metrics,
3609+
textDirection: TextDirection.rtl,
3610+
);
3611+
3612+
rtlPainter.update(
3613+
metrics.copyWith(viewportDimension: size.height, pixels: double.negativeInfinity),
3614+
AxisDirection.down,
3615+
);
3616+
3617+
// Top overscroll.
3618+
rtlPainter.paint(testCanvas, size);
3619+
final Rect rtlRect0 = captureRect();
3620+
expect(rtlRect0.top, 2);
3621+
expect(rtlRect0.left, 3);
3622+
3623+
// Bottom overscroll.
3624+
rtlPainter.update(
3625+
metrics.copyWith(viewportDimension: size.height, pixels: double.infinity),
3626+
AxisDirection.down,
3627+
);
3628+
3629+
rtlPainter.paint(testCanvas, size);
3630+
final Rect rtlRect1 = captureRect();
3631+
expect(size.height - rtlRect1.bottom, 4);
3632+
expect(rtlRect1.left, 3);
3633+
});
35703634
}

0 commit comments

Comments
 (0)