Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1926)

Issue 10916043: Added jujucharms and viewmode config flags

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 10 months ago by j.c.sackett
Modified:
10 years, 10 months ago
Reviewers:
curtis, matthew.scott, mp+172858
Visibility:
Public.

Description

Added jujucharms and viewmode config flags -Flag added for defaultViewmode; still defaults to sidebar. No other work needed, since that's passed to the browser subapp which already knows what to do with it. -Flag added for the jujucharms landing page deploy mode -"Hello World" placeholder jujucharms view and template added -Browser and routeDefault method now jujucharms aware; uses config flag to route * to the jujucharms view. https://code.launchpad.net/~jcsackett/juju-gui/jujucharms-config-flag/+merge/172858 Requires: https://code.launchpad.net/~jcsackett/juju-gui/abstract-default-viewmode/+merge/172560 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 1

Patch Set 2 : Added jujucharms and viewmode config flags #

Total comments: 1

Patch Set 3 : Added jujucharms and viewmode config flags #

Unified diffs Side-by-side diffs Delta from patch set Stats (+128 lines, -12 lines) Patch
A [revision details] View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M app/config-debug.js View 1 chunk +3 lines, -1 line 0 comments Download
M app/config-prod.js View 1 chunk +3 lines, -1 line 0 comments Download
M app/modules-debug.js View 1 2 2 chunks +12 lines, -8 lines 0 comments Download
M app/subapps/browser/browser.js View 1 2 4 chunks +25 lines, -1 line 0 comments Download
A app/subapps/browser/views/jujucharms.js View 1 2 1 chunk +68 lines, -0 lines 0 comments Download
A app/templates/jujucharms.handlebars View 1 chunk +1 line, -0 lines 0 comments Download
M test/test_browser_app.js View 1 2 2 chunks +14 lines, -1 line 0 comments Download

Messages

Total messages: 6
j.c.sackett
Please take a look.
10 years, 10 months ago (2013-07-03 17:03:07 UTC) #1
curtis
I have a question. https://codereview.appspot.com/10916043/diff/1/app/subapps/browser/browser.js File app/subapps/browser/browser.js (left): https://codereview.appspot.com/10916043/diff/1/app/subapps/browser/browser.js#oldcode701 app/subapps/browser/browser.js:701: next(); Why is next() removed?
10 years, 10 months ago (2013-07-03 18:15:16 UTC) #2
curtis
We discussed the next() issue on IRC and agreed that you will restore the line. ...
10 years, 10 months ago (2013-07-03 18:34:19 UTC) #3
j.c.sackett
Please take a look.
10 years, 10 months ago (2013-07-03 18:41:26 UTC) #4
matthew.scott
LGTM, thanks! https://codereview.appspot.com/10916043/diff/1009/app/subapps/browser/views/jujucharms.js File app/subapps/browser/views/jujucharms.js (right): https://codereview.appspot.com/10916043/diff/1009/app/subapps/browser/views/jujucharms.js#newcode23 app/subapps/browser/views/jujucharms.js:23: * Browser SubApp Sidebar View handler. s/Sidebar ...
10 years, 10 months ago (2013-07-03 19:59:58 UTC) #5
j.c.sackett
10 years, 10 months ago (2013-07-03 21:57:56 UTC) #6
*** Submitted:

Added jujucharms and viewmode config flags

-Flag added for defaultViewmode; still defaults to sidebar. No other work
needed, since that's passed to the browser subapp which already knows what to do
with it.
-Flag added for the jujucharms landing page deploy mode
-"Hello World" placeholder jujucharms view and template added
-Browser and routeDefault method now jujucharms aware; uses config flag to
route * to the jujucharms view.

R=curtis, matthew.scott
CC=
https://codereview.appspot.com/10916043
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b