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

Issue 8679045: Rearchitect the browser to use routes more

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 11 months ago by rharding
Modified:
10 years, 11 months ago
Reviewers:
mp+158713, jeff.pihach, j.c.sackett
Visibility:
Public.

Description

Rearchitect the browser to use routes more - Use nested routes and route handling to build the browser UI - Break up the Views in the browser subapp to be more singular per UX component. - Adjust tests to fit with this. https://code.launchpad.net/~rharding/juju-gui/update-urls/+merge/158713 (do not edit description out of merge proposal)

Patch Set 1 #

Patch Set 2 : - #

Total comments: 18

Patch Set 3 : Rearchitect the browser to use routes more #

Patch Set 4 : Rearchitect the browser to use routes more #

Total comments: 12

Patch Set 5 : Rearchitect the browser to use routes more #

Patch Set 6 : Rearchitect the browser to use routes more #

Unified diffs Side-by-side diffs Delta from patch set Stats (+531 lines, -416 lines) Patch
A [revision details] View 1 2 3 4 5 1 chunk +2 lines, -0 lines 0 comments Download
M app/modules-debug.js View 1 chunk +9 lines, -0 lines 0 comments Download
M app/subapps/browser/browser.js View 1 2 3 4 7 chunks +184 lines, -68 lines 0 comments Download
M app/subapps/browser/templates/browser_charm.handlebars View 1 2 3 4 5 3 chunks +3 lines, -7 lines 0 comments Download
A app/subapps/browser/templates/editorial.handlebars View 1 1 chunk +9 lines, -0 lines 0 comments Download
M app/subapps/browser/templates/sidebar.handlebars View 1 2 3 4 5 1 chunk +2 lines, -4 lines 0 comments Download
M app/subapps/browser/views/charm.js View 1 2 3 4 5 8 chunks +90 lines, -16 lines 0 comments Download
A app/subapps/browser/views/editorial.js View 1 chunk +166 lines, -0 lines 0 comments Download
M app/subapps/browser/views/fullscreen.js View 1 chunk +7 lines, -24 lines 0 comments Download
M app/subapps/browser/views/sidebar.js View 4 chunks +10 lines, -154 lines 0 comments Download
M app/subapps/browser/views/view.js View 3 chunks +0 lines, -59 lines 0 comments Download
M app/templates/charm-token.handlebars View 1 2 3 4 5 1 chunk +20 lines, -18 lines 0 comments Download
M test/test_browser_app.js View 1 chunk +0 lines, -47 lines 0 comments Download
M test/test_browser_charm_details.js View 1 2 3 4 5 11 chunks +29 lines, -19 lines 0 comments Download

Messages

Total messages: 12
rharding
Please take a look.
10 years, 11 months ago (2013-04-15 16:28:32 UTC) #1
rharding
Please take a look.
10 years, 11 months ago (2013-04-15 17:51:21 UTC) #2
rharding
Please take a look.
10 years, 11 months ago (2013-04-15 18:02:37 UTC) #3
jeff.pihach
Thanks for the refactor - I have a few comments below that I'd like some ...
10 years, 11 months ago (2013-04-15 18:21:21 UTC) #4
rharding
We had a chat about most of it. I've added a bunch of comment blocks ...
10 years, 11 months ago (2013-04-15 19:25:10 UTC) #5
rharding
Please take a look.
10 years, 11 months ago (2013-04-15 19:27:14 UTC) #6
j.c.sackett
Thanks for this refactor; I have some comments and questions below. https://codereview.appspot.com/8679045/diff/10002/app/subapps/browser/browser.js File app/subapps/browser/browser.js (left): ...
10 years, 11 months ago (2013-04-15 21:06:33 UTC) #7
rharding
Comments below. Will have an updated branch in a bit. https://codereview.appspot.com/8679045/diff/10002/app/subapps/browser/browser.js File app/subapps/browser/browser.js (left): https://codereview.appspot.com/8679045/diff/10002/app/subapps/browser/browser.js#oldcode191 ...
10 years, 11 months ago (2013-04-15 22:15:24 UTC) #8
j.c.sackett
On 2013/04/15 22:15:24, rharding wrote: > Short answer: yes > Longer: this functionality never worked ...
10 years, 11 months ago (2013-04-15 22:24:00 UTC) #9
j.c.sackett
LGTM from discussion and with promised fixes. Thanks!
10 years, 11 months ago (2013-04-15 22:24:18 UTC) #10
jeff.pihach
LGTM Thanks for all the work!
10 years, 11 months ago (2013-04-15 22:26:04 UTC) #11
rharding
10 years, 11 months ago (2013-04-16 00:09:31 UTC) #12
*** Submitted:

Rearchitect the browser to use routes more

- Use nested routes and route handling to build the browser UI
- Break up the Views in the browser subapp to be more singular per UX
component.
- Adjust tests to fit with this.

R=jeff.pihach, j.c.sackett
CC=
https://codereview.appspot.com/8679045
Sign in to reply to this message.

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