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

Issue 6842119: Add the required charm filter dropdown.

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 years ago by bac
Modified:
8 years ago
Reviewers:
mp+137050
Visibility:
Public.

Description

Add the required charm filter dropdown. The original design for the charm search results panel had a dropdown that was to be used to filter the results. After some consideration the three sets were chosen to be 'all', 'deployed', and 'subordinate'. This branch implements the dropdown and filter mechanism. The widget was based on the graph/list picker on the bottom of the page. It was refactored into a generic LESS function and supports both uses. The design for the existing picker had a 1px wide design element to provide the background. It was not tall enough for a picker with three items. As a work-around an 'outset' mitred HTML border is used. It will be easy to replace with the proper asset when it is produced. If the picker is open and a click outside closes the charm panel then the picker remains open. That is a wart that needs to be fixed. https://code.launchpad.net/~bac/juju-gui/cs-filter/+merge/137050 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 28

Patch Set 2 : Add the required charm filter dropdown. #

Total comments: 8

Patch Set 3 : Add the required charm filter dropdown. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+482 lines, -111 lines) Patch
A [revision details] View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
A app/assets/images/picker_view_right_button_down.png View 0 chunks +-1 lines, --1 lines 0 comments Download
M app/models/charm.js View 1 chunk +1 line, -1 line 0 comments Download
M app/store/charm.js View 1 chunk +2 lines, -1 line 0 comments Download
M app/templates/charm-search-result.handlebars View 1 1 chunk +20 lines, -0 lines 0 comments Download
M app/templates/overview.handlebars View 1 chunk +5 lines, -5 lines 0 comments Download
M app/views/charm-panel.js View 1 2 11 chunks +173 lines, -19 lines 0 comments Download
M lib/views/stylesheet.less View 1 3 chunks +90 lines, -86 lines 0 comments Download
M test/test_charm_panel.js View 1 2 1 chunk +190 lines, -0 lines 0 comments Download

Messages

Total messages: 9
bac
Please take a look.
8 years ago (2012-11-29 21:08:41 UTC) #1
benji
This branch looks good. Tests are especially appreciated. I had many small comments. Given the ...
8 years ago (2012-11-30 20:37:47 UTC) #2
frankban
This branch looks good Brad, thanks. Approved with changes... well, with answers at least. I ...
8 years ago (2012-12-03 14:09:13 UTC) #3
bac
Thanks for the review Benji. I think I've addressed all of your concerns. After I ...
8 years ago (2012-12-03 15:05:22 UTC) #4
bac
On 2012/12/03 14:09:13, frankban wrote: > This branch looks good Brad, thanks. > Approved with ...
8 years ago (2012-12-03 20:10:05 UTC) #5
bac
Please take a look.
8 years ago (2012-12-03 20:43:29 UTC) #6
benji
Looks good. I had a couple of small questions, mostly for my own edification. https://codereview.appspot.com/6842119/diff/12001/app/views/charm-panel.js ...
8 years ago (2012-12-03 20:58:15 UTC) #7
bac
Thanks for the re-review Benji. https://codereview.appspot.com/6842119/diff/12001/app/views/charm-panel.js File app/views/charm-panel.js (right): https://codereview.appspot.com/6842119/diff/12001/app/views/charm-panel.js#newcode82 app/views/charm-panel.js:82: * @return {Array} A ...
8 years ago (2012-12-03 22:24:00 UTC) #8
bac
8 years ago (2012-12-03 22:28:07 UTC) #9
*** Submitted:

Add the required charm filter dropdown.

The original design for the charm search results panel had a dropdown
that was to be used to filter the results.  After some consideration
the three sets were chosen to be 'all', 'deployed', and 'subordinate'.

This branch implements the dropdown and filter mechanism.  The widget
was based on the graph/list picker on the bottom of the page.  It was
refactored into a generic LESS function and supports both uses.

The design for the existing picker had a 1px wide design element to
provide the background.  It was not tall enough for a picker with
three items.  As a work-around an 'outset' mitred HTML border is
used.  It will be easy to replace with the proper asset when it is
produced.

If the picker is open and a click outside closes the charm panel then
the picker remains open.  That is a wart that needs to be fixed.

R=benji, frankban
CC=
https://codereview.appspot.com/6842119
Sign in to reply to this message.

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