Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove animation from ScrollBar #124120

Closed
wants to merge 2 commits into from

Conversation

mrdevdhingra
Copy link

Removes the animation from the ScrollBar and fixes the delay to settle to right color, when hovering over it. It was happening because there was animation delay of 200, which making giving a perception of delay while hovering over ScrollBar, by removing animation, it fixes the delay and color is changed immediately.
Other possible solution was to make animation delay 0, but that would lead to useless lines of code about delay, which wouldn't be happening, instead i removed the lines about animation.

fixes #116212

Pre-launch Checklist

  • [x ] 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, or this PR is test-exempt.
  • All existing and new tests are passing.

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

@flutter-dashboard flutter-dashboard bot added f: material design flutter/packages/flutter/material repository. f: scrolling Viewports, list views, slivers, etc. framework flutter/packages/flutter repository. See also f: labels. labels Apr 4, 2023
@HansMuller HansMuller requested a review from Piinks April 7, 2023 21:43
@@ -334,7 +333,7 @@ class _MaterialScrollbarState extends RawScrollbarState<_MaterialScrollbar> {
return Color.lerp(
_scrollbarTheme.thumbColor?.resolve(states) ?? idleColor,
_scrollbarTheme.thumbColor?.resolve(states) ?? hoverColor,
_hoverAnimationController.value,
0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would not allow users to disable it, it would just disable the animation entirely for everyone. This is likely not the way to fix #116212

Copy link
Contributor

@Piinks Piinks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hover animation is part of the material design specification. We cannot simply remove it, likely the correct solution allows the user to disable it if it is not desired.
I am going to close this pull request because we cannot accept this change. If you would like to propose a new solution to the linked issue, please feel welcome to re-open it with changes or open a new one.
Thank you!

TargetPlatform.windows,
}),
);
// testWidgets('Scrollbar thumb color completes a hover animation', (WidgetTester tester) async {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commenting out this test is also not a change we would want to make.

@Piinks Piinks closed this Apr 10, 2023
@mrdevdhingra
Copy link
Author

Perfect I got your point and have more clear picture. I'll make a new request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
f: material design flutter/packages/flutter/material repository. f: scrolling Viewports, list views, slivers, etc. framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Scrollbar should not execute fade in/out animation if it is always visible
2 participants