Delete drawerController and make drawer handle back behavior natively

Delete drawerController and make drawer handle back behavior natively

R=abarth@chromium.org, abarth

Committed: https://chromium.googlesource.com/external/mojo/+/e1d5ff4506667e7b747317a358aadb82d9c5a5f6

[abarth-chromium, 2015-07-11 02:00:53]

abarth@chromium.org changed reviewers:
+ abarth@chromium.org

[abarth-chromium, 2015-07-11 02:00:54]

lgtm

https://codereview.chromium.org/1232673003/diff/40001/sky/sdk/lib/widgets/dra...
File sky/sdk/lib/widgets/drawer.dart (right):

https://codereview.chromium.org/1232673003/diff/40001/sky/sdk/lib/widgets/dra...
sky/sdk/lib/widgets/drawer.dart:70: const String kDrawerRouteName = "[open
drawer]";
I wonder if the navigation should let you use opaque objects as route names so
that we don't get string collisions.

https://codereview.chromium.org/1232673003/diff/40001/sky/sdk/lib/widgets/dra...
sky/sdk/lib/widgets/drawer.dart:98: }
We should make an |Empty| widget so that this can just be:

return new Empty();

https://codereview.chromium.org/1232673003/diff/40001/sky/sdk/lib/widgets/dra...
sky/sdk/lib/widgets/drawer.dart:144: void close() => _performance.reverse();
We can delete most of these functions.  They existed to give the controller a
nice API, which isn't needed anymore.

https://codereview.chromium.org/1232673003/diff/40001/sky/sdk/lib/widgets/dra...
sky/sdk/lib/widgets/drawer.dart:148: void handleMaskTap(_) => close();
e.g., this function can just call _performance.reverse() directly.

[abarth-chromium, 2015-07-14 16:55:10]

https://codereview.chromium.org/1232673003/diff/40001/sky/sdk/example/stocks/...
File sky/sdk/example/stocks/lib/stock_home.dart (left):

https://codereview.chromium.org/1232673003/diff/40001/sky/sdk/example/stocks/...
sky/sdk/example/stocks/lib/stock_home.dart:324: drawer: _drawerShowing ?
buildDrawer() : null
You're building the drawer unconditionally....  It would be better not to have
to do this work if we're not going to show the drawer.

[jackson, 2015-07-14 18:25:50]

https://codereview.chromium.org/1232673003/diff/40001/sky/sdk/lib/widgets/dra...
File sky/sdk/lib/widgets/drawer.dart (right):

https://codereview.chromium.org/1232673003/diff/40001/sky/sdk/lib/widgets/dra...
sky/sdk/lib/widgets/drawer.dart:70: const String kDrawerRouteName = "[open
drawer]";
On 2015/07/11 02:00:54, abarth-chromium wrote:
> I wonder if the navigation should let you use opaque objects as route names so
> that we don't get string collisions.

I agree, I'll do this in a new CL.

https://codereview.chromium.org/1232673003/diff/40001/sky/sdk/lib/widgets/dra...
sky/sdk/lib/widgets/drawer.dart:98: }
On 2015/07/11 02:00:54, abarth-chromium wrote:
> We should make an |Empty| widget so that this can just be:
> 
> return new Empty();

This isn't needed now that we aren't building when closed.

https://codereview.chromium.org/1232673003/diff/40001/sky/sdk/lib/widgets/dra...
sky/sdk/lib/widgets/drawer.dart:144: void close() => _performance.reverse();
On 2015/07/11 02:00:54, abarth-chromium wrote:
> We can delete most of these functions.  They existed to give the controller a
> nice API, which isn't needed anymore.

Done.

https://codereview.chromium.org/1232673003/diff/40001/sky/sdk/lib/widgets/dra...
sky/sdk/lib/widgets/drawer.dart:148: void handleMaskTap(_) => close();
On 2015/07/11 02:00:54, abarth-chromium wrote:
> e.g., this function can just call _performance.reverse() directly.

Done.

[abarth-chromium, 2015-07-14 18:28:21]

lgtm

[jackson, 2015-07-14 18:51:48]

Committed patchset #5 (id:80001) manually as
e1d5ff4506667e7b747317a358aadb82d9c5a5f6 (presubmit successful).