Skip to content

Improve documentation of frictionFactor function#184509

Merged
auto-submit[bot] merged 3 commits into
flutter:masterfrom
AbdeMohlbi:issue_95367
Apr 9, 2026
Merged

Improve documentation of frictionFactor function#184509
auto-submit[bot] merged 3 commits into
flutter:masterfrom
AbdeMohlbi:issue_95367

Conversation

@AbdeMohlbi

@AbdeMohlbi AbdeMohlbi commented Apr 2, 2026

Copy link
Copy Markdown
Member

Fixes #95367

Pre-launch Checklist

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-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.

@github-actions github-actions Bot added framework flutter/packages/flutter repository. See also f: labels. f: scrolling Viewports, list views, slivers, etc. labels Apr 2, 2026
@AbdeMohlbi

Copy link
Copy Markdown
Member Author

/gemini review

@AbdeMohlbi AbdeMohlbi added the CICD Run CI/CD label Apr 2, 2026
gemini-code-assist[bot]

This comment was marked as resolved.

@github-actions github-actions Bot removed the CICD Run CI/CD label Apr 2, 2026
@AbdeMohlbi

Copy link
Copy Markdown
Member Author

/gemini review

@AbdeMohlbi AbdeMohlbi added the CICD Run CI/CD label Apr 2, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request updates the documentation for BouncingScrollPhysics.frictionFactor to clarify the friction calculation and specify values for different deceleration rates. Feedback was provided to correct the definition of overscrollFraction, noting that it represents the current overscroll amount rather than the cumulative distance dragged.

Comment on lines +701 to +702
/// The [overscrollFraction] represents the distance the user has dragged past
/// the edge as a fraction of the [ScrollMetrics.viewportDimension], where 0.0

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The documentation states that overscrollFraction represents the "distance the user has dragged past the edge". However, in the implementation of applyPhysicsToUserOffset, this value is calculated as the current distance the scroll position is beyond the scroll extent (the overscroll amount) divided by the viewport dimension. It does not represent the cumulative distance the user has moved their finger during the gesture. Using "amount of overscroll" or "overscroll distance" would be more accurate.

  /// The [overscrollFraction] represents the current amount of overscroll as a
  /// fraction of the [ScrollMetrics.viewportDimension], where 0.0
References
  1. Documentation should be useful and accurate. Explaining the parameter correctly ensures developers understand how the friction factor is calculated. (link)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

the original change has a better documentation IMO, let's wait for a review.

@AbdeMohlbi AbdeMohlbi added CICD Run CI/CD and removed CICD Run CI/CD labels Apr 2, 2026
@AbdeMohlbi AbdeMohlbi changed the title fix documnetation of frictionFactor Improve documentation of frictionFactor function Apr 5, 2026
@AbdeMohlbi AbdeMohlbi marked this pull request as ready for review April 5, 2026 07:58

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request updates the documentation for BouncingScrollPhysics.frictionFactor to clarify overscroll behavior and specify initial friction factors for normal and fast deceleration rates. Feedback suggests refining the definition of overscrollFraction to specify it as a fraction of the viewport dimension and adding a link to ScrollMetrics.viewportDimension to improve clarity and follow documentation guidelines.

Comment on lines +701 to +703
/// The `overscrollFraction` represents how far past the edge the user has
/// dragged, where 0.0 means no overscroll. As this value increases, the
/// friction factor decreases quadratically, making further overscroll harder.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The description of overscrollFraction could be more precise. It represents the overscroll distance as a fraction of the viewport dimension, not just "how far" (which might be interpreted as pixels). Additionally, the quadratic decrease only applies as the fraction approaches 1.0 (at which point friction becomes infinite). Providing a link to ScrollMetrics.viewportDimension also helps clarify the term.

  /// The `overscrollFraction` represents the overscroll distance as a fraction
  /// of the [ScrollMetrics.viewportDimension], where 0.0 means no overscroll.
  /// As this value increases towards 1.0, the friction factor decreases
  /// quadratically, making further overscroll harder.
References
  1. Documentation should be useful and explain the 'how' and 'why'. Clarifying the scale of parameters and linking to related definitions makes the documentation more useful for developers. (link)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

this is the same comment from before

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Well, I guess it is a good thing Gemini is consistent! 😆

@justinmc justinmc requested review from Piinks and justinmc April 7, 2026 22:30

@Piinks Piinks left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@AbdeMohlbi AbdeMohlbi added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 9, 2026
@auto-submit auto-submit Bot added this pull request to the merge queue Apr 9, 2026
Merged via the queue into flutter:master with commit 39e21ef Apr 9, 2026
81 checks passed
@flutter-dashboard flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Apr 9, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 9, 2026
auto-submit Bot pushed a commit to flutter/packages that referenced this pull request Apr 9, 2026
flutter/flutter@05e0ae0...81c87ea

2026-04-09 [email protected] Remove last material dependency from cupertino tests (flutter/flutter#184781)
2026-04-09 [email protected] Roll Skia from 7c46cb639dba to 4d0f5389e131 (7 revisions) (flutter/flutter#184812)
2026-04-09 [email protected] Make `windowing_test` follow repo analyzer rules (flutter/flutter#184752)
2026-04-09 [email protected] Improve documentation of `frictionFactor` function (flutter/flutter#184509)
2026-04-09 [email protected] Roll Skia from d2b0bd12576a to 7c46cb639dba (1 revision) (flutter/flutter#184796)
2026-04-09 [email protected] Roll Fuchsia GN SDK from JLBh4Z9PKsjIJcqDU... to SEfYx3xgueX3aFAY3... (flutter/flutter#184797)
2026-04-09 [email protected] Fixed freeze flow (flutter/flutter#184793)
2026-04-09 [email protected] Roll pub packages (flutter/flutter#184795)
2026-04-09 [email protected] Roll Skia from e9ed4fc9f154 to d2b0bd12576a (36 revisions) (flutter/flutter#184791)
2026-04-08 [email protected] [Android] Allow sensitive content to gracefully fail when unregistering host before registering (flutter/flutter#184789)
2026-04-08 [email protected] Refactor: remove material from autocomplete_test, scrollable_restoration_test, semantics_tester_generate_test_semantics_expression_for_current_semantics_tree_test (flutter/flutter#184615)
2026-04-08 [email protected] Warn about the use of TestSemantics (flutter/flutter#184369)
2026-04-08 [email protected] Change freeze flow to pull_request_target (flutter/flutter#184785)
2026-04-08 [email protected] Update to the beta dart version for 3.44 branch cut.  (flutter/flutter#184770)
2026-04-08 [email protected] [Dot shorthands] Migrate examples/api/test (flutter/flutter#183966)
2026-04-08 [email protected] [fuchsia] Give AOT runners the ability to copy FFI callback thunks. (flutter/flutter#184696)
2026-04-08 [email protected] Add await or ignore lint to invokeMethod callsites (flutter/flutter#182870)
2026-04-08 [email protected] Correctly handle failure to read /proc/self/exe link (flutter/flutter#184700)
2026-04-08 [email protected] Roll Skia from e264d870a380 to e9ed4fc9f154 (11 revisions) (flutter/flutter#184713)
2026-04-08 [email protected] Roll Packages from 5299279 to 0e0a032 (5 revisions) (flutter/flutter#184720)
2026-04-08 [email protected] Roll pub packages (flutter/flutter#184772)
2026-04-08 [email protected] Roll Fuchsia Linux SDK from 1rcChbOv4nSTVkUxs... to pDXMXRIjEHTw7B0sk... (flutter/flutter#184722)
2026-04-08 [email protected] Remove navigator_utils cross-imports from cupertino tests (flutter/flutter#184282)
2026-04-08 [email protected] Even more awaits v2 (flutter/flutter#184552)
2026-04-08 [email protected] Allow personal skills to be gitignored (flutter/flutter#184727)
2026-04-08 [email protected] [ci] mac_arm64 build_test re-enable shard 1 presubmit (flutter/flutter#184751)
2026-04-08 [email protected] Fix repo check on code freeze (flutter/flutter#184771)

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
mbcorona pushed a commit to mbcorona/flutter that referenced this pull request Apr 15, 2026
Fixes flutter#95367
## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [AI contribution guidelines] and understand my
responsibilities, or I am not using AI tools.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [x] I signed the [CLA].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] 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].
- [x] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [x] All existing and new tests are passing.

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](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.

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[AI contribution guidelines]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CICD Run CI/CD 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.

BouncingScrollPhysics.frictionFactor documentation is wrong

2 participants