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

Issue 8529044: Use charm container in sidebar

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

Description

Use charm container in sidebar -CharmContainer used to display new charms -Replaced iteration to render tokens with iteration gathering data, which is passed into CharmContainer -Removed "New Charms" from template, as that is provided by the charm container. https://code.launchpad.net/~jcsackett/juju-gui/use-container-in-sidebar/+merge/157747 Requires: https://code.launchpad.net/~jcsackett/juju-gui/charm-container-redesign/+merge/157745 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 6

Patch Set 2 : Use charm container in sidebar #

Total comments: 1

Patch Set 3 : Use charm container in sidebar #

Patch Set 4 : Use charm container in sidebar #

Unified diffs Side-by-side diffs Delta from patch set Stats (+22 lines, -11 lines) Patch
A [revision details] View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M app/subapps/browser/templates/sidebar.handlebars View 1 chunk +0 lines, -1 line 0 comments Download
M app/subapps/browser/views/sidebar.js View 1 2 3 chunks +20 lines, -10 lines 0 comments Download

Messages

Total messages: 7
j.c.sackett
Please take a look.
10 years, 11 months ago (2013-04-08 20:38:24 UTC) #1
rharding
LGTM with trivial. If you'd prefer I guess we can make the slider replacement another ...
10 years, 11 months ago (2013-04-08 20:49:32 UTC) #2
j.c.sackett
Replied; changes coming. https://codereview.appspot.com/8529044/diff/1/app/subapps/browser/views/sidebar.js File app/subapps/browser/views/sidebar.js (right): https://codereview.appspot.com/8529044/diff/1/app/subapps/browser/views/sidebar.js#newcode115 app/subapps/browser/views/sidebar.js:115: this.slider = this._generateSliderWidget(sliderCharms); On 2013/04/08 20:49:32, ...
10 years, 11 months ago (2013-04-08 21:25:31 UTC) #3
j.c.sackett
Please take a look.
10 years, 11 months ago (2013-04-08 21:31:16 UTC) #4
jeff.pihach
LGTM Just wondering about the cleanup on those instances. https://codereview.appspot.com/8529044/diff/4001/app/subapps/browser/views/sidebar.js File app/subapps/browser/views/sidebar.js (right): https://codereview.appspot.com/8529044/diff/4001/app/subapps/browser/views/sidebar.js#newcode133 app/subapps/browser/views/sidebar.js:133: ...
10 years, 11 months ago (2013-04-08 21:52:29 UTC) #5
j.c.sackett
On 2013/04/08 21:52:29, jeff.pihach wrote: > LGTM Just wondering about the cleanup on those instances. ...
10 years, 11 months ago (2013-04-08 22:06:29 UTC) #6
j.c.sackett
10 years, 11 months ago (2013-04-09 13:57:41 UTC) #7
*** Submitted:

Use charm container in sidebar

-CharmContainer used to display new charms
-Replaced iteration to render tokens with iteration gathering data, which is
passed into CharmContainer
-Removed "New Charms" from template, as that is provided by the charm
container.

R=rharding, jeff.pihach
CC=
https://codereview.appspot.com/8529044
Sign in to reply to this message.

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