Animate the user account drawer header arrow by dudeofawesome · Pull Request #14315 · flutter/flutter

dudeofawesome · 2018-01-27T21:01:10Z

When the user account drawer header is tapped, the arrow in the bottom right will now rotate instead of just switching to the upward facing arrow.

Hixie · 2018-01-29T22:38:19Z

packages/flutter/lib/src/material/user_accounts_drawer_header.dart

+        curve: Curves.easeInOut,
+      )
+      ..addListener(() {
+        setState(() {});


always put a comment inside the block of a setState() if it's otherwise empty, explaining what it is that the build method depends on that has changed

Hixie · 2018-01-29T22:38:38Z

packages/flutter/lib/src/material/user_accounts_drawer_header.dart

+
+    _controller = new AnimationController(
+      value: widget.isOpen ? 1.0 : 0.0,
+      duration: const Duration(milliseconds: 200),


I picked 200 ms because that's the duration of the account switcher animations in the gallery's drawer example.

https://github.com/flutter/flutter/blob/master/examples/flutter_gallery/lib/demo/material/drawer_demo.dart#L36

Hixie · 2018-01-29T22:40:03Z

packages/flutter/lib/src/material/user_accounts_drawer_header.dart

+    );
+    _animation = new CurvedAnimation(
+        parent: _controller,
+        curve: Curves.easeInOut,


why this curve?

I picked easeInOut because that's what I think the Material guidelines recommend. Should this be a different curve?

Ah, I see. That's actually Curves.fastOutSlowIn (the name matches Android's name for the same).

Hixie · 2018-01-29T22:43:34Z

packages/flutter/lib/src/material/user_accounts_drawer_header.dart

+
+  void _onTap() {
+    if (widget.isOpen) {
+      _controller.reverse();


this doesn't seem right. Consider the case where the user taps the widget, onTap is invoked, and does not change isOpen. Now the arrow will be animating the wrong direction.

I think it would make more sense to trigger the animation during didUpdateWidget based on the new value of isOpen.

See data_table.dart in the material directory, there's a _SortArrow widget there that you may wish to crib from. (It also does other cool things, like always going clockwise unless you change it mid-turn, IIRC.)

Hixie · 2018-01-29T22:43:42Z

this will need tests

dudeofawesome · 2018-01-30T07:18:49Z

@Hixie What would you recommend I test? I'm not very familiar with UI testing in Flutter.

Hixie · 2018-02-05T20:51:38Z

@dudeofawesome Anything you don't want us to regress should be tested. Imagine that we have a bot which randomly changes a line of code, then runs the tests, and if the tests pass, it checks the change in.
So for example, you could test that if the user taps on the header, the arrow animates correctly.

Hixie · 2018-02-05T20:54:02Z

packages/flutter/lib/src/material/user_accounts_drawer_header.dart

+    super.didUpdateWidget(oldWidget);
+
+    if (widget.isOpen) {
+      _controller.forward();


This will cause the animation to repeatedly restart each time the widget is rebuilt, if the direction didn't change. You probably want to check if the animation is already running, and only start it if it's not running in the right direction. So e.g. in this case you'd want to call forward() if the animation status was dismissed or reverse, and where you call reverse you only want to call it if the status is completed or forward.

Hixie · 2018-03-09T21:47:02Z

@dudeofawesome I'm going to close this PR since you're not waiting for review right now, but please don't hesitate to reopen it, or submit a new one, once you are ready for further review. Thanks!

If you don't want to work further on this PR that's fine too, I've tagged it so that when we have some free time we can fix whatever remains to be fixed and land it for you.

dudeofawesome · 2018-04-02T01:38:18Z

@Hixie I've been trying to get this updated with the latest version of Flutter, but have been stuck on a an odd pub issue. It won't get the gallery demo's packages because it says it can't find version 0.3.0 of the connectivity package.

$ flutter packages get
Running "flutter packages get" in flutter_gallery...
Package connectivity has no versions that match 0.3.0 derived from:
- flutter_gallery depends on version 0.3.0
pub get failed (1)

I've upgraded my system Dart install to the latest (2.0.0-dev.43.0 (Wed Mar 28 01:04:15 2018 +0200) on "macos_x64"), and am using the bundled flutter command:

Flutter 0.0.21-pre.336 • channel feat/user-account-arrow-anim • https://github.com/dudeofawesome/flutter.git
Framework • revision 53810b82d1 (50 minutes ago) • 2018-04-01 17:43:56 -0700
Engine • revision 6280adbfb1
Tools • Dart 2.0.0-dev.39.0.flutter-06949dc985

I've merged in the latest changes from upstream/master.

flutter run attempts to upgrade before it does anything else, so I can't really do anything until I get this fixed. Do you have any suggestions on how I might fix that?

Hixie · 2018-04-02T16:11:45Z

Hm, I wonder if what's going on is that your local changes are confusing our version code, and so pub is refusing to give you packages that relate to more recent changes.

Can you try git fetch, to make sure you have the new git tags downloaded, and if that doesn't help, try rebasing so that your branch is up to date with your commits on top rather than having a separate branch with a very old merge point? (I don't know if what I just said makes sense, git mystifies me a bit.)

* Create TabBarTheme of(...) constructor * First round comments * fix imports

dudeofawesome · 2018-10-30T23:46:23Z

I'd love to be able to finish this PR up, but I've been unable to get my dev environment set up properly.

I'm trying to use the Flutter executable in this repository, which I've forked and then cloned on to my machine at ~/Github/flutter, with the executable at ~/Github/flutter/bin/flutter. If I run $ ~/Github/flutter/bin/flutter --version I get the following:

Flutter 0.0.21-pre.331 • channel master • https://github.com/dudeofawesome/flutter.git
Framework • revision a3074304b9 (4 days ago) • 2018-10-26 22:39:08 -0700
Engine • revision 6c2a0b3a6d
Tools • Dart 2.1.0 (build 2.1.0-dev.8.0 bf26f760b1)

which is clearly a problem given Flutter is at 0.10.1 or something now.

I'm not really sure how to get the bundled version of Flutter updated. I've tried merging in the most recent changes from upstream/master, but that didn't change the version that $ ~/Github/flutter/bin/flutter --version reports.

jslavitz · 2018-10-31T00:43:51Z

@dudeofawesome Awesome! And I've had that exact problem. I've found the best way to resolve it is to get the diff file for the current PR and then use git apply to merge it into the current version of flutter.

To get the diff file (if you don't already know), you have to click the "files changed" link at the top of the pr, delete the "/files" and replace it with ".diff". Press enter and you'll get the diff file! (Also, you'll probably need to add a new line to the end of the file you make.) If you then copy that text into a text file, you can use diff apply <file> to patch it into a new branch.

If git apply fails, you'll have to manually copy your changes into a new version of flutter.

Let me know if that helps!

…eofawesome/flutter into feat/user-account-arrow-anim

googlebot · 2018-10-31T02:22:10Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of all the commit author(s), set the cla label to yes (if enabled on your project), and then merge this pull request when appropriate.

dudeofawesome · 2018-10-31T02:52:04Z

@jslavitz I ended up trying a rebase (which I've tried before) and it worked this time! I've now got flutter 0.10.2-pre.131!

Edit: And apparently that added 83 files to the diff. Not sure exactly what happened there.

jslavitz · 2018-10-31T20:36:32Z

@dudeofawesome Great! Though, adding those files isn't good and we won't be able to merge this commit unless it only has the changes from the files you were working on. It would probably make the most sense to strip out your changed file and then merge that into a fresh master. (You also might have to make a new PR because you unfortunately can't un-commit files from a PR :( ).

jslavitz · 2018-11-02T22:05:12Z

@dudeofawesome If you're busy right now, I can take this one over! Just wondering if you modified any other files than user_accounts_drawer_header.dart?

dudeofawesome · 2018-11-02T23:48:49Z

@jslavitz I'm a little bit stuck as far as trying to figure out exactly how I should go about writing tests for this. The only user-facing change here is that the arrow now animates. Is there any prior art that you know of as far as testing animations?

I've only modified the user_accounts_drawer_header.dart file.

jslavitz · 2018-11-03T02:35:54Z

@dudeofawesome So the purpose of tests are just to verify that the code you've written works the way you expect it to. So in this case, you probably want to test that:

Before the header drawer is tapped, the arrow is fully down
In the middle of the animation, the arrow is neither up or down
At the end of the animation, the arrow is fully up.
And then have these three tests in reverse direction for when the arrow goes from up to down.

You'll want to add the tests to the user_accounts_drawer_header_test.dart file. For examples on how to write this test, you'll need to look in other test files. To find those files, you could grep for AnimationController in the framework to find files that have animations in them. Then you could find the test files relating to these files and see how the animations are tested there.

If you want help, feel free to comment or if you want me to take over let me know! Good luck!

jslavitz · 2018-11-05T21:24:12Z

@dudeofawesome I'm in charge of getting this PR landed, so I'm going to take this over from here. If you still want to write a test though, we can totally add it to the testing framework, more tests are always better! Thanks so much for your contribution! 😊

dudeofawesome added 4 commits January 27, 2018 12:11

🚧 convert _AccountDetails to stateful widget

e52aae9

✨💄 animate account switcher arrow

0b462bc

🐛 set initial arrow position by isOpen

9b035a5

🎨 prefer declare const constructors

9e5e9c7

googlebot added the cla: yes label Jan 27, 2018

Hixie reviewed Jan 29, 2018

View reviewed changes

dudeofawesome added 2 commits January 29, 2018 19:43

👌💬 add comment to setState call

3723a5d

👌💄 use fastOutSlowIn curve

637b06c

dudeofawesome added 3 commits January 29, 2018 23:22

🎨 simplify animation listener structure

02ac56b

👌🐛 fix animation playing backwards if onTap doesn't act as expected

234c703

🐛💄 use flipped curve for arrow return

f1eba91

Hixie reviewed Feb 5, 2018

View reviewed changes

Hixie added the needs love label Mar 9, 2018

Hixie closed this Mar 9, 2018

dudeofawesome added 2 commits March 21, 2018 15:52

🔀 Merge remote-tracking branch 'flutter/master'

bb96f99

🔀 Merge remote-tracking branch 'upstream/master'

d20af84

dudeofawesome added 5 commits April 2, 2018 18:25

Merge remote-tracking branch 'upstream/master'

761eb80

🚧 convert _AccountDetails to stateful widget

3c4dd8e

✨💄 animate account switcher arrow

4b82f23

🐛 set initial arrow position by isOpen

78ba378

🎨 prefer declare const constructors

b941c18

[Material] Create TabBarTheme.of constructor (flutter#23702)

b8c3e0d

* Create TabBarTheme of(...) constructor * First round comments * fix imports

dudeofawesome added 14 commits October 30, 2018 18:33

Merge branch 'master' of https://github.com/dudeofawesome/flutter

b9106d4

🚧 convert _AccountDetails to stateful widget

8686c46

✨💄 animate account switcher arrow

c2133ed

🐛 set initial arrow position by isOpen

dc5f9bc

🎨 prefer declare const constructors

0af0cb9

👌💬 add comment to setState call

39a930d

👌💄 use fastOutSlowIn curve

c479223

🎨 simplify animation listener structure

c7b08b4

👌🐛 fix animation playing backwards if onTap doesn't act as expected

6ff3584

🐛💄 use flipped curve for arrow return

779693f

✨💄 animate account switcher arrow

0361a0e

👌🐛 fix animation playing backwards if onTap doesn't act as expected

44ab282

🐛 fix rebase issues

f56799e

Merge branch 'feat/user-account-arrow-anim' of https://github.com/dud…

6d4dd62

…eofawesome/flutter into feat/user-account-arrow-anim

googlebot added cla: no and removed cla: yes labels Oct 31, 2018

🎨 remove unnecessary new keywords

857e950

jslavitz mentioned this pull request Nov 6, 2018

Animate user account drawer header arrow #24023

Merged

jslavitz closed this in #24023 Nov 8, 2018

github-actions bot locked as resolved and limited conversation to collaborators Aug 9, 2021

Conversation

dudeofawesome commented Jan 27, 2018

Uh oh!

Hixie Jan 29, 2018

Choose a reason for hiding this comment

Uh oh!

Hixie Jan 29, 2018

Choose a reason for hiding this comment

Uh oh!

dudeofawesome Jan 30, 2018

Choose a reason for hiding this comment

Uh oh!

Hixie Jan 29, 2018

Choose a reason for hiding this comment

Uh oh!

dudeofawesome Jan 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Hixie Jan 30, 2018

Choose a reason for hiding this comment

Uh oh!

Hixie Jan 29, 2018

Choose a reason for hiding this comment

Uh oh!

Hixie commented Jan 29, 2018

Uh oh!

dudeofawesome commented Jan 30, 2018

Uh oh!

Hixie commented Feb 5, 2018

Uh oh!

Hixie Feb 5, 2018

Choose a reason for hiding this comment

Uh oh!

Hixie commented Mar 9, 2018

Uh oh!

dudeofawesome commented Apr 2, 2018

Uh oh!

Hixie commented Apr 2, 2018

Uh oh!

dudeofawesome commented Oct 30, 2018

Uh oh!

jslavitz commented Oct 31, 2018

Uh oh!

googlebot commented Oct 31, 2018

Uh oh!

dudeofawesome commented Oct 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jslavitz commented Oct 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jslavitz commented Nov 2, 2018

Uh oh!

dudeofawesome commented Nov 2, 2018

Uh oh!

jslavitz commented Nov 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jslavitz commented Nov 5, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

19 participants

dudeofawesome Jan 30, 2018 •

edited

Loading

dudeofawesome commented Oct 31, 2018 •

edited

Loading

jslavitz commented Oct 31, 2018 •

edited

Loading

jslavitz commented Nov 3, 2018 •

edited

Loading