Skip to content

Conversation

@drown0315
Copy link
Contributor

@drown0315 drown0315 commented Apr 12, 2024

Add a code snippet demonstrating the usage of SpringSimulation in the docs.
Improve the descriptions of each argument in SpringDescription.

Pre-launch Checklist

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

@github-actions github-actions bot added the framework flutter/packages/flutter repository. See also f: labels. label Apr 12, 2024
Copy link
Contributor

@justinmc justinmc left a comment

Choose a reason for hiding this comment

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

Some minor suggestions here but looks like an improvement overall. Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I worry "speed" here is misleading, if I get myself back in the mindset of physics class. Maybe say "rate" instead? Or "the longer the time to return to the equilibrium position"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your reply, your concerns are very valid. I prefer the latter

Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like this paragraph belongs after the next one.

Copy link
Contributor

Choose a reason for hiding this comment

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

I also think that this paragraph would be better after the next paragraph.

Comment on lines 63 to 66
Copy link
Contributor

Choose a reason for hiding this comment

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

I would leave the original first sentence and keep it as its own paragraph, then make your additions a second paragraph (making sure your first sentence is a complete sentence). Something like:

The damping coefficient (c).

It is a pure number without physical meaning that describes the oscillation and decay of a system after being disturbed. The larger the damping...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I think this might make the structure clearer. I've reorganized the parameters for mass, stiffness, and damping accordingly.

Comment on lines 103 to 109
Copy link
Contributor

Choose a reason for hiding this comment

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

You have mismatched parentheses here.

Comment on lines 120 to 127
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: There should be an empty row of /// between these two lines I think.

@drown0315
Copy link
Contributor Author

drown0315 commented Apr 19, 2024

@justinmc Thanks for review, I've already made overall updates.

Copy link
Contributor

@justinmc justinmc left a comment

Choose a reason for hiding this comment

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

LGTM with nits. Thanks for the fixes here and for improving our docs!

Comment on lines 63 to 64
Copy link
Contributor

Choose a reason for hiding this comment

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

Another physics nit. Rephrasing the last sentence here:

A stiff spring applies more force to the object that is attached for some deviation from the rest position.

Comment on lines 69 to 71
Copy link
Contributor

Choose a reason for hiding this comment

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

"meaning,describes" => "meaning and describes"

Copy link
Contributor

Choose a reason for hiding this comment

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

Also space after the period: "disturbed. The"

Copy link
Contributor

Choose a reason for hiding this comment

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

Super nit: "as if it was" => "as if it were"

@drown0315
Copy link
Contributor Author

@justinmc Thanks for review👍, I've already made overall updates.

@bleroux bleroux added the autosubmit Merge PR when tree becomes green via auto submit App label May 3, 2024
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label May 3, 2024
@auto-submit
Copy link
Contributor

auto-submit bot commented May 3, 2024

auto label is removed for flutter/flutter/146674, due to This PR has not met approval requirements for merging. The PR author is not a member of flutter-hackers and needs 1 more review(s) in order to merge this PR.

  • Merge guidelines: A PR needs at least one approved review if the author is already part of flutter-hackers or two member reviews if the author is not a flutter-hacker before re-applying the autosubmit label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

Copy link
Contributor

@bleroux bleroux left a comment

Choose a reason for hiding this comment

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

LGTM!

@bleroux bleroux added the autosubmit Merge PR when tree becomes green via auto submit App label May 3, 2024
@auto-submit auto-submit bot merged commit cfeb76e into flutter:master May 3, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 3, 2024
auto-submit bot pushed a commit to flutter/packages that referenced this pull request May 3, 2024
flutter/flutter@bf7191f...f1037a0

2024-05-03 [email protected] Roll Flutter Engine from 445c3bfc0d5b to 8cce00433073 (1 revision) (flutter/flutter#147778)
2024-05-03 [email protected] Roll Flutter Engine from 1f9edbeeceb3 to 445c3bfc0d5b (1 revision) (flutter/flutter#147776)
2024-05-03 [email protected] Roll Flutter Engine from 75eb18090510 to 1f9edbeeceb3 (1 revision) (flutter/flutter#147769)
2024-05-03 [email protected] Improved documentation for SpringSimulation (flutter/flutter#146674)
2024-05-03 [email protected] Roll Flutter Engine from b8d81a4f6e5a to 75eb18090510 (1 revision) (flutter/flutter#147766)
2024-05-03 [email protected] Roll Flutter Engine from d8c8cf4384f0 to b8d81a4f6e5a (2 revisions) (flutter/flutter#147765)
2024-05-03 [email protected] Move snippets package back into flutter repo (flutter/flutter#147690)
2024-05-03 [email protected] Roll Flutter Engine from 98a800b00cfc to d8c8cf4384f0 (2 revisions) (flutter/flutter#147763)
2024-05-03 [email protected] Roll Flutter Engine from c380e9fd4122 to 98a800b00cfc (3 revisions) (flutter/flutter#147761)
2024-05-03 [email protected] Roll Flutter Engine from fc71b650a70a to c380e9fd4122 (4 revisions) (flutter/flutter#147759)
2024-05-03 [email protected] Roll pub packages (flutter/flutter#147741)
2024-05-03 [email protected] Roll Flutter Engine from 3f1f81915620 to fc71b650a70a (2 revisions) (flutter/flutter#147753)
2024-05-02 [email protected] Roll Flutter Engine from 5088f63ecee2 to 3f1f81915620 (3 revisions) (flutter/flutter#147748)
2024-05-02 [email protected] [web] skip debug mode CanvasKit e2e tests due to flakiness; unskip all other modes (flutter/flutter#147736)
2024-05-02 [email protected] Control flow collections: `flutter_tools/` (flutter/flutter#147450)
2024-05-02 [email protected] Add default arguments to `AnimatedPhysicalModel` (flutter/flutter#147424)
2024-05-02 [email protected] Roll Flutter Engine from 9982b5ffd913 to 5088f63ecee2 (3 revisions) (flutter/flutter#147740)
2024-05-02 [email protected] `_RenderDecorator.computeDryBaseline`  (flutter/flutter#146365)
2024-05-02 [email protected] Roll Flutter Engine from e1126e59b698 to 9982b5ffd913 (1 revision) (flutter/flutter#147727)
2024-05-02 [email protected] [web] increase chromedriver logging level (flutter/flutter#147687)
2024-05-02 [email protected] Roll Flutter Engine from 1fb36ac9d718 to e1126e59b698 (1 revision) (flutter/flutter#147720)

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],[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
vashworth pushed a commit to vashworth/packages that referenced this pull request May 6, 2024
flutter/flutter@bf7191f...f1037a0

2024-05-03 [email protected] Roll Flutter Engine from 445c3bfc0d5b to 8cce00433073 (1 revision) (flutter/flutter#147778)
2024-05-03 [email protected] Roll Flutter Engine from 1f9edbeeceb3 to 445c3bfc0d5b (1 revision) (flutter/flutter#147776)
2024-05-03 [email protected] Roll Flutter Engine from 75eb18090510 to 1f9edbeeceb3 (1 revision) (flutter/flutter#147769)
2024-05-03 [email protected] Improved documentation for SpringSimulation (flutter/flutter#146674)
2024-05-03 [email protected] Roll Flutter Engine from b8d81a4f6e5a to 75eb18090510 (1 revision) (flutter/flutter#147766)
2024-05-03 [email protected] Roll Flutter Engine from d8c8cf4384f0 to b8d81a4f6e5a (2 revisions) (flutter/flutter#147765)
2024-05-03 [email protected] Move snippets package back into flutter repo (flutter/flutter#147690)
2024-05-03 [email protected] Roll Flutter Engine from 98a800b00cfc to d8c8cf4384f0 (2 revisions) (flutter/flutter#147763)
2024-05-03 [email protected] Roll Flutter Engine from c380e9fd4122 to 98a800b00cfc (3 revisions) (flutter/flutter#147761)
2024-05-03 [email protected] Roll Flutter Engine from fc71b650a70a to c380e9fd4122 (4 revisions) (flutter/flutter#147759)
2024-05-03 [email protected] Roll pub packages (flutter/flutter#147741)
2024-05-03 [email protected] Roll Flutter Engine from 3f1f81915620 to fc71b650a70a (2 revisions) (flutter/flutter#147753)
2024-05-02 [email protected] Roll Flutter Engine from 5088f63ecee2 to 3f1f81915620 (3 revisions) (flutter/flutter#147748)
2024-05-02 [email protected] [web] skip debug mode CanvasKit e2e tests due to flakiness; unskip all other modes (flutter/flutter#147736)
2024-05-02 [email protected] Control flow collections: `flutter_tools/` (flutter/flutter#147450)
2024-05-02 [email protected] Add default arguments to `AnimatedPhysicalModel` (flutter/flutter#147424)
2024-05-02 [email protected] Roll Flutter Engine from 9982b5ffd913 to 5088f63ecee2 (3 revisions) (flutter/flutter#147740)
2024-05-02 [email protected] `_RenderDecorator.computeDryBaseline`  (flutter/flutter#146365)
2024-05-02 [email protected] Roll Flutter Engine from e1126e59b698 to 9982b5ffd913 (1 revision) (flutter/flutter#147727)
2024-05-02 [email protected] [web] increase chromedriver logging level (flutter/flutter#147687)
2024-05-02 [email protected] Roll Flutter Engine from 1fb36ac9d718 to e1126e59b698 (1 revision) (flutter/flutter#147720)

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],[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
TecHaxter pushed a commit to TecHaxter/flutter_packages that referenced this pull request May 22, 2024
flutter/flutter@bf7191f...f1037a0

2024-05-03 [email protected] Roll Flutter Engine from 445c3bfc0d5b to 8cce00433073 (1 revision) (flutter/flutter#147778)
2024-05-03 [email protected] Roll Flutter Engine from 1f9edbeeceb3 to 445c3bfc0d5b (1 revision) (flutter/flutter#147776)
2024-05-03 [email protected] Roll Flutter Engine from 75eb18090510 to 1f9edbeeceb3 (1 revision) (flutter/flutter#147769)
2024-05-03 [email protected] Improved documentation for SpringSimulation (flutter/flutter#146674)
2024-05-03 [email protected] Roll Flutter Engine from b8d81a4f6e5a to 75eb18090510 (1 revision) (flutter/flutter#147766)
2024-05-03 [email protected] Roll Flutter Engine from d8c8cf4384f0 to b8d81a4f6e5a (2 revisions) (flutter/flutter#147765)
2024-05-03 [email protected] Move snippets package back into flutter repo (flutter/flutter#147690)
2024-05-03 [email protected] Roll Flutter Engine from 98a800b00cfc to d8c8cf4384f0 (2 revisions) (flutter/flutter#147763)
2024-05-03 [email protected] Roll Flutter Engine from c380e9fd4122 to 98a800b00cfc (3 revisions) (flutter/flutter#147761)
2024-05-03 [email protected] Roll Flutter Engine from fc71b650a70a to c380e9fd4122 (4 revisions) (flutter/flutter#147759)
2024-05-03 [email protected] Roll pub packages (flutter/flutter#147741)
2024-05-03 [email protected] Roll Flutter Engine from 3f1f81915620 to fc71b650a70a (2 revisions) (flutter/flutter#147753)
2024-05-02 [email protected] Roll Flutter Engine from 5088f63ecee2 to 3f1f81915620 (3 revisions) (flutter/flutter#147748)
2024-05-02 [email protected] [web] skip debug mode CanvasKit e2e tests due to flakiness; unskip all other modes (flutter/flutter#147736)
2024-05-02 [email protected] Control flow collections: `flutter_tools/` (flutter/flutter#147450)
2024-05-02 [email protected] Add default arguments to `AnimatedPhysicalModel` (flutter/flutter#147424)
2024-05-02 [email protected] Roll Flutter Engine from 9982b5ffd913 to 5088f63ecee2 (3 revisions) (flutter/flutter#147740)
2024-05-02 [email protected] `_RenderDecorator.computeDryBaseline`  (flutter/flutter#146365)
2024-05-02 [email protected] Roll Flutter Engine from e1126e59b698 to 9982b5ffd913 (1 revision) (flutter/flutter#147727)
2024-05-02 [email protected] [web] increase chromedriver logging level (flutter/flutter#147687)
2024-05-02 [email protected] Roll Flutter Engine from 1fb36ac9d718 to e1126e59b698 (1 revision) (flutter/flutter#147720)

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],[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
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants