Repository navigation
Fix RangeSlider thumb overlay remains visible after touch interaction… - #189610
KarthikManikuttan wants to merge 17 commits into
Conversation
github-actions
Bot
added
framework
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the RangeSlider component to correctly handle focus highlights for its start and end thumbs by replacing the existing Focus widgets with FocusableActionDetector and managing focus states via new state variables. The changes include updates to the render object to support these focus states and the addition of new test cases to verify the focus behavior. The reviewer suggests refactoring the duplicated FocusableActionDetector logic into a private helper method to improve maintainability.
QuncCccccc
left a comment
QuncCccccc
left a comment
There was a problem hiding this comment.
The fix looks really good! Thanks for your contribution! Just left a few comments:)
KarthikManikuttan
left a comment
KarthikManikuttan
left a comment
There was a problem hiding this comment.
@QuncCccccc Thanks for the review comments, I updated the PR
waiting for your comments 😊
QuncCccccc
left a comment
QuncCccccc
left a comment
There was a problem hiding this comment.
Thanks a lot for your quick response! Overall LGTM with just one more test comment!
KarthikManikuttan
left a comment
KarthikManikuttan
left a comment
There was a problem hiding this comment.
@QuncCccccc Thank you for the review, I added a focused-state customization test
If there is anything else , please comment 😊
QuncCccccc
left a comment
QuncCccccc
left a comment
There was a problem hiding this comment.
LGTM! Thanks for your quick update!
|
Google testing failures look expected to me, image diffs where the reference image shows one thumb with a lingering highlight and the test image shows the highlight gone. |
|
@QuncCccccc @victorsanni Thanks for confirming. Since the Google testing failures are expected golden/image diffs from removing the lingering RangeSlider focus highlight, could you please approve/triage the Google testing result or add the PR to the Google testing queue? |
|
This PR is good to go but we are still in code freeze for Material and Cupertino library. I'll mark this as Draft and add "wait for code freeze" for now. Later when |
QuncCccccc
added
the
waiting for code freeze
|
This pull request contains changes to Material or Cupertino, which are currently frozen in this repository. Changes should be made in Please refer to #188444 for instructions. |
|
@QuncCccccc Hi! I saw the new instructions for porting Material/Cupertino PRs to flutter/packages. Is #189610 ready to be ported to material_ui now, or should I wait for any further guidance? |
Hi @KarthikManikuttan, yes, I just LGTMed the ported one. So I'm closing this one. Thanks for the help:)! |
… ends (#12560) Ports flutter/flutter#189610 to material_ui, per the porting instructions in flutter/flutter#188444. Fixes flutter/flutter#189205 After a RangeSlider thumb was touched or dragged, the selected thumb could retain focus and continue painting the focused overlay after the touch gesture ended. This happened because the focused thumb requested keyboard focus during pointer interaction, and RangeSlider painted its focused overlay directly from FocusNode.hasFocus. This PR separates actual thumb focus from the visual focus highlight. Touch interaction can still move focus to the selected thumb for semantics and keyboard continuation, but the focused overlay is only painted when Flutter's focus highlight mode says it should be shown, such as keyboard/traditional focus. This PR also fixes the active drag overlay disappearing while a touch gesture is still in progress. Previously, if the finger moved outside the thumb's overlay bounds during a drag, hover tracking could reverse the shared overlay animation even though the slider was still active. The overlay now remains visible for the active drag and dismisses when the interaction ends. Original PR: flutter/flutter#189610 ## Pre-Review Checklist
… ends (flutter#12560) Ports flutter/flutter#189610 to material_ui, per the porting instructions in flutter/flutter#188444. Fixes flutter/flutter#189205 After a RangeSlider thumb was touched or dragged, the selected thumb could retain focus and continue painting the focused overlay after the touch gesture ended. This happened because the focused thumb requested keyboard focus during pointer interaction, and RangeSlider painted its focused overlay directly from FocusNode.hasFocus. This PR separates actual thumb focus from the visual focus highlight. Touch interaction can still move focus to the selected thumb for semantics and keyboard continuation, but the focused overlay is only painted when Flutter's focus highlight mode says it should be shown, such as keyboard/traditional focus. This PR also fixes the active drag overlay disappearing while a touch gesture is still in progress. Previously, if the finger moved outside the thumb's overlay bounds during a drag, hover tracking could reverse the shared overlay animation even though the slider was still active. The overlay now remains visible for the active drag and dismisses when the interaction ends. Original PR: flutter/flutter#189610 ## Pre-Review Checklist
Fixes #189205
This PR fixes
RangeSlideroverlay behavior for touch interactions.After a
RangeSliderthumb was touched or dragged, the selected thumb could retain focus and continue painting the focused overlay after the touch gesture ended. This happened because the focused thumb requested keyboard focus during pointer interaction, andRangeSliderpainted its focused overlay directly fromFocusNode.hasFocus.This PR separates actual thumb focus from the visual focus highlight. Touch interaction can still move focus to the selected thumb for semantics and keyboard continuation, but the focused overlay is only painted when Flutter’s focus highlight mode says it should be shown, such as keyboard/traditional focus.
This PR also fixes the active drag overlay disappearing while a touch gesture is still in progress. Previously, if the finger moved outside the thumb’s overlay bounds during a drag, hover tracking could reverse the shared overlay animation even though the slider was still active. The overlay now remains visible for the active drag and dismisses when the interaction ends.
Changes:
FocusableActionDetectorfor the start and end thumb focus nodes._RenderRangeSlideris actively interacting.Tests added/updated:
RangeSlider keeps drag overlay when touch moves outside thumbRangeSlider touch focus does not show persistent overlayRangeSlider keyboard focus shows focused overlayValidation:
dart format packages/flutter/lib/src/material/range_slider.dart packages/flutter/test/material/range_slider_test.dartflutter test packages/flutter/test/material/range_slider_test.dartThe targeted
range_slider_test.dartsuite passes locally.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.