Uh oh!
There was an error while loading. Please reload this page.
[material_ui] Port flutter/flutter #185149 "Slider label clips the screen" - #12572
Conversation
* Updated documentation for `_kValueIndicatorHorizontalBuffer` to clarify the 64.0 heuristic. * Added unit tests to `slider_test.dart` and `range_slider_test.dart` utilizing custom value indicator shapes to capture and verify `TextPainter` layout constraints. * Refactored brittle `Path` offset expectations in `slider_test.dart` to use robust `paints` matchers. * Updated `MediaQuery` mocks in `slider_test.dart` to use `.copyWith()` so screen size defaults are properly inherited by the new `safeMaxWidth` calculation.
There was a problem hiding this comment.
Code Review
This pull request introduces a horizontal buffer of 64.0 logical pixels for the value indicators of both Slider and RangeSlider to prevent them from bleeding off the screen edges. The label painters are updated to apply this buffer as a maxWidth constraint, restrict the text to a single line with an ellipsis on overflow, and migrate to textScaler. Corresponding tests are added to verify this behavior. The review feedback suggests explicitly setting a smaller physical size for the test views in the new tests to ensure that the layout constraints and ellipsis behavior are actually triggered and verified.
| testWidgets('RangeSlider labels respect horizontal buffer and avoid screen overflow', ( | ||
| WidgetTester tester, | ||
| ) async { | ||
| final logPainters = <TextPainter>[]; | ||
| final shape = LoggingRangeSliderValueIndicatorShape(<InlineSpan>[], logPainters); |
There was a problem hiding this comment.
The test might pass even if the maxWidth constraint is not applied. The default screen width in WidgetTester is 800.0 logical pixels, and the test strings are likely shorter than 800.0 - 64.0 = 736.0 logical pixels. To ensure the constraint and ellipsis behavior are actually triggered and verified, consider setting a smaller physical size for the test view.
| testWidgets('RangeSlider labels respect horizontal buffer and avoid screen overflow', ( | |
| WidgetTester tester, | |
| ) async { | |
| final logPainters =<TextPainter>[]; | |
| final shape =LoggingRangeSliderValueIndicatorShape(<InlineSpan>[], logPainters); | |
| testWidgets('RangeSlider labels respect horizontal buffer and avoid screen overflow', ( | |
| WidgetTester tester, | |
| ) async { | |
| tester.view.physicalSize =constSize(300.0, 600.0); | |
| tester.view.devicePixelRatio =1.0; | |
| addTearDown(() { | |
| tester.view.resetPhysicalSize(); | |
| tester.view.resetDevicePixelRatio(); | |
| }); | |
| final logPainters =<TextPainter>[]; | |
| final shape =LoggingRangeSliderValueIndicatorShape(<InlineSpan>[], logPainters); |
| testWidgets('Slider label respects horizontal buffer and avoids screen overflow', ( | ||
| WidgetTester tester, | ||
| ) async { | ||
| final logPainters = <TextPainter>[]; | ||
| final shape = LoggingValueIndicatorShape(<InlineSpan>[], logPainters); |
There was a problem hiding this comment.
The test might pass even if the maxWidth constraint is not applied. The default screen width in WidgetTester is 800.0 logical pixels, and the test string is likely shorter than 800.0 - 64.0 = 736.0 logical pixels. To ensure the constraint and ellipsis behavior are actually triggered and verified, consider setting a smaller physical size for the test view.
| testWidgets('Slider label respects horizontal buffer and avoids screen overflow', ( | |
| WidgetTester tester, | |
| ) async { | |
| final logPainters =<TextPainter>[]; | |
| final shape =LoggingValueIndicatorShape(<InlineSpan>[], logPainters); | |
| testWidgets('Slider label respects horizontal buffer and avoid screen overflow', ( | |
| WidgetTester tester, | |
| ) async { | |
| tester.view.physicalSize =constSize(300.0, 600.0); | |
| tester.view.devicePixelRatio =1.0; | |
| addTearDown(() { | |
| tester.view.resetPhysicalSize(); | |
| tester.view.resetDevicePixelRatio(); | |
| }); | |
| final logPainters =<TextPainter>[]; | |
| final shape =LoggingValueIndicatorShape(<InlineSpan>[], logPainters); |
There was a problem hiding this comment.
Not applicable: flutter_test uses the fixed-advance FlutterTest font
(1 em per glyph), so this 86-character label at textTheme.bodyLarge (16px)
lays out at ~1376 logical pixels, well past the 736 limit at the default
800px viewport. The constraint is exercised — removing the maxWidth fails
this assertion.
Test unchanged; this PR is a straight port of flutter/flutter#185149.
mbcorona
commented
Aug 24, 2026
@QuncCccccc this is the port of flutter/flutter#185149 that you approve. |
Uh oh!
There was an error while loading. Please reload this page.
…er#191734) flutter/packages@df2ba94...740f093 2026-08-25 srawlins@google.com [cupertino_ui] Remove unused parameters from constructors of generic classes. (flutter/packages#12457) 2026-08-25 srawlins@google.com [material_ui] Remove unused parameters from constructors of generic classes. (flutter/packages#12458) 2026-08-25 6655696+guidezpl@users.noreply.github.com Ignore shared code for iOS platform implementation of Google Maps plugin (flutter/packages#12529) 2026-08-25 136096126+glitchfl@users.noreply.github.com [cross_file] fixed `readAsString` decoding in-memory bytes as UTF-16 (flutter/packages#12479) 2026-08-25 lozhkovoi@gmail.com [cupertino_ui] Remove two items assert to allow CupertinoTabBar to have one tab (flutter/packages#12546) 2026-08-25 huahua8893@sina.cn [cupertino_ui] Fix covered sheet revealing root route through top gap (flutter/packages#12530) 2026-08-25 fluttergithubbot@gmail.com Sync release-go_router-18.0.0 to main (flutter/packages#12575) 2026-08-25 fluttergithubbot@gmail.com Sync release-material_ui-1.1.0 to main (flutter/packages#12577) 2026-08-25 fluttergithubbot@gmail.com Sync release-cupertino_ui-1.0.1 to main (flutter/packages#12576) 2026-08-24 41930132+hellohuanlin@users.noreply.github.com [quick_actions_ios]unskip XCUITests (flutter/packages#12436) 2026-08-24 karthimanikuttan001@gmail.com Fix RangeSlider thumb overlay remains visible after touch interaction ends (flutter/packages#12560) 2026-08-24 victor.orozco@cloudsufi.com [google_sign_in] Increase iOS coverage tests (flutter/packages#12484) 2026-08-24 269567208+reidbaker-agent@users.noreply.github.com [camera_android_camerax] Migrate from dart_skills_lint to skills_lint (flutter/packages#12543) 2026-08-24 74037732+developerashkan@users.noreply.github.com [go_router] Clarify onEnter/redirect ordering, add regression test (flutter/packages#12337) 2026-08-24 brunocorona.alcantar@gmail.com [material_ui] Port flutter/flutter flutter#185149 "Slider label clips the screen" (flutter/packages#12572) 2026-08-24 engine-flutter-autoroll@skia.org Roll Flutter from 65c9a8d to 9a82789 (17 revisions) (flutter/packages#12578) 2026-08-24 stuartmorgan@google.com [tool] Fix dart_test.yaml parsing (flutter/packages#12574) 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-flutter-autoroll Please CC flutter-ecosystem@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: 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
Ports flutter/flutter#185149 from flutter/flutter to flutter/packages, following the porting instructions in flutter/flutter#188444.
Long text in a
SliderorRangeSliderlabel clipped off the edges of the screen. This limits the label'smaxWidthto the screen width minus a calculated horizontal buffer (_kValueIndicatorHorizontalBuffer), forcesmaxLines = 1, and applies the Unicode ellipsis (\u2026), so the text no longer expands past the screen boundary and the bubble shape no longer breaks vertically. AscreenSize.width.isFinite && screenSize.width > 0guard keeps the constraint inert when no valid size is available.The original PR was reviewed and approved by @QuncCccccc. No merge commits were included; the five commits cherry-picked onto
material_uiwith no conflicts.Validation:
dart format --set-exit-if-changedon the four changed filesgit diff --checkflutter test test/slider_test.dart test/range_slider_test.dartFixesflutter/flutter#63293
Pre-Review Checklist
[shared_preferences]///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.Footnotes
Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩↩2