update physics simulation article based on review feedback #2890
Conversation
|
LGTM |
|
All nits. LGTM otherwise. |
| @@ -153,25 +152,26 @@ Widget build(BuildContext context) { | |||
|
|
|||
| When the widget is released, it should spring back to the center. | |||
|
|
|||
| Add an `Animation<Alignment>` field and an `_updateAnimation()` method. This | |||
| Add an `Animation<Alignment>` field and an `_runAnimation()` method. This | |||
RedBrogdon
Aug 8, 2019
Contributor
nit: I don't see the parens for parameters included on any of the other function names in the text. I could be missing one, though.
nit: I don't see the parens for parameters included on any of the other function names in the text. I could be missing one, though.
| end: Alignment.center, | ||
| ), | ||
| ); | ||
| _controller.reset(); |
RedBrogdon
Aug 8, 2019
Contributor
nit: indenting.
nit: indenting.
| @@ -184,7 +184,6 @@ void initState() { | |||
| }); | |||
| }); | |||
| _updateAnimation(); | |||
| super.initState(); | |||
RedBrogdon
Aug 8, 2019
Contributor
This was moved to the top, wasn't it?
This was moved to the top, wasn't it?
RedBrogdon
Aug 8, 2019
Contributor
(super.initState, I mean)
(super.initState, I mean)
| method already sets the direction by setting the animation's start and end | ||
| alignment.) | ||
| continues at that speed before being snapped back. (The `_runAnimation()` method | ||
| already sets the direction by setting the animation's start and end alignment.) |
RedBrogdon
Aug 8, 2019
Contributor
nit: extra space after "direction."
nit: extra space after "direction."
| var spring = SpringDescription( | ||
| const spring = SpringDescription( |
RedBrogdon
Aug 8, 2019
Contributor
It's slightly weird to have a constant declared in a function scope rather than in the scope of the file or Widget class itself. If this is the only way to make the article readable, then it's cool to leave it. If you can somehow tell people to add this constant in a more appropriate place without overly complicating the instructions, though, that'd be a good idea.
It's slightly weird to have a constant declared in a function scope rather than in the scope of the file or Widget class itself. If this is the only way to make the article readable, then it's cool to leave it. If you can somehow tell people to add this constant in a more appropriate place without overly complicating the instructions, though, that'd be a good idea.
johnpryan
Aug 8, 2019
Author
Collaborator
discussed in the other review, but I think we should leave it here for readability
discussed in the other review, but I think we should leave it here for readability
| } | ||
| ``` | ||
|
|
||
| Don't forget to call `_runAnimation()` with the velocity and size: |
RedBrogdon
Aug 8, 2019
Contributor
nit: Same note on the parens here as on L155.
nit: Same note on the parens here as on L155.
| /// Update the animation so that it runs from the dragged point back to the | ||
| /// center. | ||
| void _updateAnimation() { | ||
| /// Calculates and runs a [SpringSimulation] |
RedBrogdon
Aug 8, 2019
Contributor
nit: period on the end of the comment.
nit: period on the end of the comment.
review: flutter/samples#123