Repository navigation
Conversation
github-actions
Bot
added
framework
navaronbracke
requested changes
Jun 11, 2026
navaronbracke
left a comment
navaronbracke
left a comment
Contributor
There was a problem hiding this comment.
Some remarks on the tests and a gotcha with the shortcuts map (I think)
| result = Padding(padding: padding, child: result); | ||
| } | ||
|
|
||
| final Map<ShortcutActivator, Intent> shortcutMap = switch (MediaQuery.navigationModeOf( |
Contributor
There was a problem hiding this comment.
Should this go in didChangeDependencies() ?
I do recall the docs for shortcuts saying that you should avoid recreating the shortcut map during build(),
since build() is called many times
| ), | ||
| ), | ||
| ); | ||
| return () => values; |
Contributor
There was a problem hiding this comment.
I find this a bit of an anti pattern? Could we make the method return Future and only pump the frame, while using a GlobalKey to retrieve the value?
| WidgetTester tester, { | ||
| required NavigationMode navigationMode, | ||
| }) async { | ||
| var values = const RangeValues(40, 80); |
Contributor
There was a problem hiding this comment.
Could we put the values in the test itself? Now it is a bit of a hidden value of this method
| await tester.pumpAndSettle(); | ||
|
|
||
| // Now the arrow keys adjust the focused (start) thumb. | ||
| await tester.sendKeyEvent(LogicalKeyboardKey.arrowRight); |
Contributor
There was a problem hiding this comment.
Can we test both left and right?
| expect(values().end, 80); | ||
| }); | ||
|
|
||
| testWidgets('in editing mode, arrow keys change the value', (WidgetTester tester) async { |
Contributor
There was a problem hiding this comment.
I find the distinction between this test and "pressing enter enters editing mode so arrow keys change the value" a bit dubious?
Piinks
added
the
Decoupling: Not ready to port yet
Contributor
|
I've marked this PR as not ready to port to flutter/packages yet. |
11 tasks done
Contributor
|
Closing as replaced by flutter/packages#12630 |
auto-submit Bot
pushed a commit
to flutter/packages
that referenced
this pull request
Sep 18, 2026
…gationMode.directional` (#12630) Ports flutter/flutter#187816 to `material_ui`, following the instructions in flutter/flutter#188444. The commits were cherry-picked from the original PR, with only file paths and imports adjusted for this repo. In `NavigationMode.directional` (TV remotes, D-pads) the arrow keys are needed to move focus between views, so a focused `RangeSlider` thumb had no way to adjust its value. This adds the pattern discussed in flutter/flutter#181968: - Arrow keys only move focus. - Enter on a focused thumb enters a value adjustment mode, where Left and Right adjust the value. - Enter again, or losing focus, exits that mode. `NavigationMode.traditional` is unchanged: arrow keys adjust the value directly. This mirrors the keyboard handling `Slider` already has in this package. ## Tests New test group `RangeSlider keyboard with NavigationMode.directional` in `packages/material_ui/test/range_slider_test.dart`, covering each case above plus traditional navigation. Fixes flutter/flutter#181968 Supersedes flutter/flutter#187816, which will be closed with a pointer to this PR. ## Pre-Review Checklist **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot 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. [^1]: Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR implements the alternative input method for
RangeSliderinNavigationMode.directionalthat was discussed in #181968, following the UX pattern @chunhtai and the Material team suggested there: in directional mode the arrow keys only move focus, pressing Enter on a focused thumb enters a value-adjustment mode where Left/Right adjust the value, and pressing Enter again (or losing focus) exits it. Traditional navigation is unchanged. I've added a dedicated test group covering each of these cases.Dependency / sequencing note: This builds directly on top of the keyboard support added in #181525, which is currently a draft held behind the Material/Cupertino code freeze (#184093). Because that work isn't in
masteryet, I'd appreciate guidance on how you'd like to sequence this — e.g. rebasing this onto #181525's branch as a stacked PR, or holding it until #181525 (or its post-decoupling port) lands. Happy to adjust the base or re-target to the new package once the decoupling path is settled.Fixes #181968
If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance.
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.