fix(scrollbar): Update padding type to EdgeInsetsGeometry #172056

SalehTZ · 2025-07-12T07:38:46Z

This PR enhances RTL support for the RawScrollbar widget by updating its padding type and adding comprehensive tests.

The changes include:

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
Adding a test case that verifies scrollbar positioning with EdgeInsetsDirectional padding
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: #167922

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

- Change RawScrollbar padding parameter from EdgeInsets to EdgeInsetsGeometry - This allows support for both logical and physical padding - Enables proper RTL (Right-to-Left) layout support for scroll bars This change is part of Flutter's ongoing RTL support improvements, allowing the scrollbar to work correctly in both LTR and RTL layouts. Fixes flutter#167922

- Add test case for scrollbar with EdgeInsetsDirectional padding - Verify scrollbar positioning with directional padding values - Test both overscroll and regular scroll scenarios - Ensure proper RTL layout support This test complements the recent change to support EdgeInsetsGeometry in the scrollbar widget. Fixes: flutter#167922

victorsanni

LGTM! Thanks for this PR.

justinmc

Thank you again for helping Flutter have better RTL support!

justinmc · 2025-07-21T18:24:00Z

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

+    assert(
+      textDirection != null,
+      'A TextDirection must be provided before a Scrollbar can be painted.',
+    );
+    _resolvedPadding = padding.resolve(textDirection);


With this assert, it looks like textDirection is now effectively required?

Would it be possible to only resolve this if necessary, like if it's an EdgeInsetsGeometry and textDirection is non-null?

It was previously on the line 556.

Yeah that's a better idea.

What about setting default a value TextDirection.ltr to textDirection if no text direction was provided? Because right now textDirection is required anyways. see https://github.com/flutter/flutter/pull/172056/files#diff-7ea53e627d4729db3a54ab440e8f6d2610cbfc44ac4f9718daef49b2b4645f2dL556

What do you think about the developer experience for RTL users in the case where there is a default? If they pass an EdgeInsetsDirectional, do they want to be warned if there is no TextDirection or would they be ok with an LTR default?

I see this wouldn't be the only place where we default to LTR:

flutter/packages/flutter/lib/src/services/text_input.dart

Line 1445 in 053c8a5

this.direction = TextDirection.ltr,

I think current approach is good for RTL users as well, they can change position of almost anything just by defining the locale parameter in MaterialApp or CuppertinoApp. I explained a bit about it here.

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

packages/flutter/test/widgets/scrollbar_test.dart

…etry

justinmc

LGTM with nits 👍 . I like your _resolvedPadding approach, thanks for fixing that.

justinmc · 2025-07-28T18:35:30Z

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

    }

    _textDirection = value;
+    _resolvedPadding = _padding.resolve(_textDirection);


It's possible that _textDirection is null and ~~value~~ padding is EdgeInsetsDirectional. Do we need another assert here similar to the one in the constructor?

Hmm, I don't think _textDirection and textDirection could be null, and also the value here is a TextDirection?.

Do we need another assert here similar to the one in the constructor?

We can make sure of this:

set textDirection(TextDirection? value) { assert(value != null, 'A non-null textDirection must be provided.'); assert( padding is! EdgeInsetsDirectional || value != null, 'A non-null textDirection must be provided when using EdgeInsetsDirectional for padding.', ); if (textDirection == value) { return; } _textDirection = value; _resolvedPadding = _padding.resolve(_textDirection); notifyListeners(); }

But generally this textDirection is based on the Locale and/or Directionality of the App/Context. So it wouldn't be null and if the 'locale' is null then the system's locale value is used. by default.
See:

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

Lines 877 to 884 in dcae0c4

/// The initial locale for this app's [Localizations] widget is based

/// on this value.

///

/// If the 'locale' is null then the system's locale value is used.

///

/// The value of [Localizations.locale] will equal this locale if

/// it matches one of the [supportedLocales]. Otherwise it will be

/// the first element of [supportedLocales].

See below for both Material and Cupertino:

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

Line 351 in dcae0c4

..textDirection = Directionality.of(context)

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

Line 163 in dcae0c4

..textDirection = Directionality.of(context)

Sorry I meant padding not value. And that makes sense, thanks for the explanation.

justinmc · 2025-07-28T18:42:27Z

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

+    assert(
+      textDirection != null,
+      'A TextDirection must be provided before a Scrollbar can be painted.',
+    );
+    _resolvedPadding = padding.resolve(textDirection);


What do you think about the developer experience for RTL users in the case where there is a default? If they pass an EdgeInsetsDirectional, do they want to be warned if there is no TextDirection or would they be ok with an LTR default?

I see this wouldn't be the only place where we default to LTR:

flutter/packages/flutter/lib/src/services/text_input.dart

Line 1445 in 053c8a5

this.direction = TextDirection.ltr,

gemini-code-assist · 2025-07-29T04:36:44Z

Warning

There is an error in the Gemini Code Assist config file for this repository at .gemini/config.yaml. The configuration cannot be used until the file is fixed.

…etry

auto-submit · 2025-07-29T18:00:07Z

autosubmit label was removed for flutter/flutter/172056, because - The status or check suite Windows framework_tests_libraries has failed. Please fix the issues identified (or deflake) before re-applying this label.

justinmc · 2025-07-29T18:02:08Z

@SalehTZ The test is an isolates test failure that must be a flake, I've seen it on another PR this morning. If it doesn't go green, can you try rebasing?

…lutter/flutter#172056)

flutter/flutter@46b097a...c3279ca 2025-07-30 [email protected] Roll Skia from 00765f238077 to da5a6704f8a3 (1 revision) (flutter/flutter#172966) 2025-07-30 [email protected] Roll Skia from 62476cd444d2 to 00765f238077 (3 revisions) (flutter/flutter#172955) 2025-07-30 [email protected] Roll Skia from 34693354289f to 62476cd444d2 (1 revision) (flutter/flutter#172950) 2025-07-30 [email protected] Roll Skia from f72bd01a49b1 to 34693354289f (1 revision) (flutter/flutter#172946) 2025-07-30 [email protected] [a11y] add RangeSlider to a11y test app as additional use-case (flutter/flutter#172922) 2025-07-30 [email protected] Roll Skia from a42898e5d622 to f72bd01a49b1 (21 revisions) (flutter/flutter#172944) 2025-07-29 [email protected] Fix SegmentedButton border doesn't reflect states (flutter/flutter#172754) 2025-07-29 [email protected] Fix documentation for FlutterEngineRunTask (flutter/flutter#172889) 2025-07-29 [email protected] Roll Fuchsia Linux SDK from tK_PAaLeo0pUxi8hv... to bQVQlLssTxxLjoDU0... (flutter/flutter#172925) 2025-07-29 [email protected] fix(scrollbar): Update padding type to EdgeInsetsGeometry (flutter/flutter#172056) 2025-07-29 [email protected] Refactor Android platform view code in advance of enabling HCPP on existing PV widgets (behind a flag) (flutter/flutter#170553) 2025-07-29 [email protected] Roll Packages from 6b2e34e to ed235d1 (4 revisions) (flutter/flutter#172905) 2025-07-29 [email protected] Add package PR triage note (flutter/flutter#172898) 2025-07-29 [email protected] Roll Skia from 409e1c7ba09b to a42898e5d622 (29 revisions) (flutter/flutter#172906) 2025-07-29 [email protected] Made `android_gradle_print_build_variants_test.dart` more robust (flutter/flutter#172910) 2025-07-29 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Reland licenses cpp switch (#172671)" (flutter/flutter#172912) 2025-07-29 [email protected] Fix Gemini Code Assist for GitHub config yaml (flutter/flutter#172887) 2025-07-29 [email protected] Marks Linux_android_emu_unstable android_defines_test to be unflaky (flutter/flutter#171856) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…r#9699) flutter/flutter@46b097a...c3279ca 2025-07-30 [email protected] Roll Skia from 00765f238077 to da5a6704f8a3 (1 revision) (flutter/flutter#172966) 2025-07-30 [email protected] Roll Skia from 62476cd444d2 to 00765f238077 (3 revisions) (flutter/flutter#172955) 2025-07-30 [email protected] Roll Skia from 34693354289f to 62476cd444d2 (1 revision) (flutter/flutter#172950) 2025-07-30 [email protected] Roll Skia from f72bd01a49b1 to 34693354289f (1 revision) (flutter/flutter#172946) 2025-07-30 [email protected] [a11y] add RangeSlider to a11y test app as additional use-case (flutter/flutter#172922) 2025-07-30 [email protected] Roll Skia from a42898e5d622 to f72bd01a49b1 (21 revisions) (flutter/flutter#172944) 2025-07-29 [email protected] Fix SegmentedButton border doesn't reflect states (flutter/flutter#172754) 2025-07-29 [email protected] Fix documentation for FlutterEngineRunTask (flutter/flutter#172889) 2025-07-29 [email protected] Roll Fuchsia Linux SDK from tK_PAaLeo0pUxi8hv... to bQVQlLssTxxLjoDU0... (flutter/flutter#172925) 2025-07-29 [email protected] fix(scrollbar): Update padding type to EdgeInsetsGeometry (flutter/flutter#172056) 2025-07-29 [email protected] Refactor Android platform view code in advance of enabling HCPP on existing PV widgets (behind a flag) (flutter/flutter#170553) 2025-07-29 [email protected] Roll Packages from 6b2e34e to ed235d1 (4 revisions) (flutter/flutter#172905) 2025-07-29 [email protected] Add package PR triage note (flutter/flutter#172898) 2025-07-29 [email protected] Roll Skia from 409e1c7ba09b to a42898e5d622 (29 revisions) (flutter/flutter#172906) 2025-07-29 [email protected] Made `android_gradle_print_build_variants_test.dart` more robust (flutter/flutter#172910) 2025-07-29 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Reland licenses cpp switch (#172671)" (flutter/flutter#172912) 2025-07-29 [email protected] Fix Gemini Code Assist for GitHub config yaml (flutter/flutter#172887) 2025-07-29 [email protected] Marks Linux_android_emu_unstable android_defines_test to be unflaky (flutter/flutter#171856) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…lutter/flutter#172056)

SalehTZ added 3 commits July 12, 2025 10:36

Merge branch 'flutter:master' into master

ee127cd

github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: scrolling Viewports, list views, slivers, etc. labels Jul 12, 2025

SalehTZ marked this pull request as ready for review July 12, 2025 08:06

justinmc requested a review from victorsanni July 15, 2025 22:30

victorsanni approved these changes Jul 15, 2025

View reviewed changes

justinmc mentioned this pull request Jul 21, 2025

[Proposal]Change RawScrollbar's padding attribute to be EdgeInsetsGeometry instead of EdgeInsets #167922

Closed

justinmc reviewed Jul 21, 2025

View reviewed changes

SalehTZ added 6 commits July 22, 2025 13:27

added safeguard for padding and textDirection also added docs

16199f1

also added docs for padding

1e9052f

hmm maybe also add this.

66562d2

used test() instead of testWidgets()

0eaf625

ensure _resolvedPadding is initialized.

f5384b0

Merge branch 'master' into change-scrollbar-padding-to-EdgeInsetsGeom…

a1a6f5b

…etry

victorsanni requested a review from justinmc July 28, 2025 17:37

justinmc approved these changes Jul 28, 2025

View reviewed changes

Merge branch 'master' into change-scrollbar-padding-to-EdgeInsetsGeom…

9b8fe90

…etry

justinmc added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 29, 2025

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jul 29, 2025

victorsanni added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 29, 2025

auto-submit bot added this pull request to the merge queue Jul 29, 2025

Merged via the queue into flutter:master with commit a5934ee Jul 29, 2025
73 of 74 checks passed

flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jul 29, 2025

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 30, 2025

a5934ee34 fix(scrollbar): Update padding type to EdgeInsetsGeometry (f…

69c35f5

…lutter/flutter#172056)

engine-flutter-autoroll mentioned this pull request Jul 30, 2025

Roll Flutter from 46b097a3070d to c3279caa127d (18 revisions) flutter/packages#9699

Merged

Piinks mentioned this pull request Aug 1, 2025

Flutter Notable Commits - 2025 #161460

Open

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 12, 2025

a5934ee34 fix(scrollbar): Update padding type to EdgeInsetsGeometry (f…

d0d1e28

…lutter/flutter#172056)

	/// The initial locale for this app's [Localizations] widget is based
	/// on this value.
	///
	/// If the 'locale' is null then the system's locale value is used.
	///
	/// The value of [Localizations.locale] will equal this locale if
	/// it matches one of the [supportedLocales]. Otherwise it will be
	/// the first element of [supportedLocales].

fix(scrollbar): Update padding type to EdgeInsetsGeometry #172056

fix(scrollbar): Update padding type to EdgeInsetsGeometry #172056

Uh oh!

Conversation

SalehTZ commented Jul 12, 2025

Pre-launch Checklist

Uh oh!

victorsanni left a comment

Choose a reason for hiding this comment

Uh oh!

justinmc left a comment

Choose a reason for hiding this comment

Uh oh!

justinmc Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

SalehTZ Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

justinmc Jul 28, 2025

Choose a reason for hiding this comment

Uh oh!

SalehTZ Jul 29, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

justinmc left a comment

Choose a reason for hiding this comment

Uh oh!

justinmc Jul 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SalehTZ Jul 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

justinmc Jul 29, 2025

Choose a reason for hiding this comment

Uh oh!

justinmc Jul 28, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot commented Jul 29, 2025

Uh oh!

auto-submit bot commented Jul 29, 2025

Uh oh!

justinmc commented Jul 29, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

SalehTZ Jul 22, 2025 •

edited

Loading

justinmc Jul 28, 2025 •

edited

Loading

SalehTZ Jul 29, 2025 •

edited

Loading