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

Issue 8529043: Updates charm container design

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

Description

Updates charm container design -Updates template to use new expand/collapse design (arrows, no text) and show total charm count in title -renderUI now assembles template data, rather than adding more ATTRS and using a one off template helper -hasExtra template helper is removed -Updated tests https://code.launchpad.net/~jcsackett/juju-gui/charm-container-redesign/+merge/157745 Requires: https://code.launchpad.net/~jcsackett/juju-gui/helpers-and-friends/+merge/157560 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 2

Patch Set 2 : Updates charm container design #

Patch Set 3 : Updates charm container design #

Unified diffs Side-by-side diffs Delta from patch set Stats (+17 lines, -21 lines) Patch
A [revision details] View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M app/templates/charm-container.handlebars View 1 chunk +5 lines, -5 lines 0 comments Download
M app/widgets/charm-container.js View 1 2 2 chunks +7 lines, -12 lines 0 comments Download
M test/test_charm_container.js View 1 2 4 chunks +3 lines, -4 lines 0 comments Download

Messages

Total messages: 6
j.c.sackett
Please take a look.
11 years ago (2013-04-08 20:34:24 UTC) #1
rharding
LGTM with one trivial. https://codereview.appspot.com/8529043/diff/1/app/widgets/charm-container.js File app/widgets/charm-container.js (right): https://codereview.appspot.com/8529043/diff/1/app/widgets/charm-container.js#newcode130 app/widgets/charm-container.js:130: content = this.TEMPLATE(data), this alignment ...
11 years ago (2013-04-08 20:46:04 UTC) #2
j.c.sackett
https://codereview.appspot.com/8529043/diff/1/app/widgets/charm-container.js File app/widgets/charm-container.js (right): https://codereview.appspot.com/8529043/diff/1/app/widgets/charm-container.js#newcode130 app/widgets/charm-container.js:130: content = this.TEMPLATE(data), On 2013/04/08 20:46:04, rharding wrote: > ...
11 years ago (2013-04-08 20:48:05 UTC) #3
j.c.sackett
Please take a look.
11 years ago (2013-04-08 21:33:44 UTC) #4
jeff.pihach
LGTM thanks
11 years ago (2013-04-08 21:50:44 UTC) #5
j.c.sackett
11 years ago (2013-04-09 13:35:01 UTC) #6
*** Submitted:

Updates charm container design

-Updates template to use new expand/collapse design (arrows, no text) and show
total charm count in title
-renderUI now assembles template data, rather than adding more ATTRS and using
a one off template helper
-hasExtra template helper is removed
-Updated tests

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

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