Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(39)

Issue 1191153002: Make back button control drawer in stocks app (Closed)

Created:
5 years, 6 months ago by jackson
Modified:
5 years, 6 months ago
Reviewers:
abarth-chromium
CC:
abarth-chromium, gregsimon, jackson_old, mojo-reviews_chromium.org, qsr+mojo_chromium.org
Base URL:
git@github.com:domokit/mojo.git@master
Target Ref:
refs/heads/master
Project:
mojo
Visibility:
Public.

Description

Make back button control drawer in stocks app Currently you lose your scroll and drawer state when coming back from the settings pane. I think we should solve this by having the Navigator maintain a Stack and keeping the StockHome alive underneath it. But this is good enough for a first iteration. R=abarth@chromium.org, abarth Committed: https://chromium.googlesource.com/external/mojo/+/0c8edcaef8f3714b7ddfade37290f00ba2fc43f3

Patch Set 1 #

Patch Set 2 : fix analyzer warnings #

Total comments: 3

Patch Set 3 : abarth cr feedback #

Patch Set 4 : rebase #

Patch Set 5 : abarth feedback #

Unified diffs Side-by-side diffs Delta from patch set Stats (+90 lines, -59 lines) Patch
M sky/examples/stocks2/lib/stock_app.dart View 1 2 1 chunk +27 lines, -11 lines 0 comments Download
M sky/examples/stocks2/lib/stock_home.dart View 1 2 3 4 5 chunks +21 lines, -11 lines 0 comments Download
M sky/examples/widgets/navigation.dart View 1 3 chunks +4 lines, -5 lines 0 comments Download
M sky/sdk/lib/widgets/drawer.dart View 2 chunks +5 lines, -5 lines 0 comments Download
M sky/sdk/lib/widgets/navigator.dart View 1 4 chunks +33 lines, -27 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
jackson
5 years, 6 months ago (2015-06-18 22:07:40 UTC) #1
abarth-chromium
lgtm https://codereview.chromium.org/1191153002/diff/20001/sky/examples/stocks2/lib/stock_app.dart File sky/examples/stocks2/lib/stock_app.dart (right): https://codereview.chromium.org/1191153002/diff/20001/sky/examples/stocks2/lib/stock_app.dart#newcode20 sky/examples/stocks2/lib/stock_app.dart:20: }); Technically we should do this stock fetcher ...
5 years, 6 months ago (2015-06-18 22:24:00 UTC) #2
jackson
5 years, 6 months ago (2015-06-18 22:36:10 UTC) #3
Message was sent while issue was closed.
Committed patchset #5 (id:80001) manually as
0c8edcaef8f3714b7ddfade37290f00ba2fc43f3 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698