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

Issue 8910043: Adds search functionality

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

Description

Adds search functionality This adds text search from the search widget -Search widget propogates search text to browser subapp -Subapp routes data to search view -Search view does query from text -Search view can render search results As a driveby, this moves repeated apiFailure code out to an extension. There is an issue outstanding: -The querystring can be eaten by the routing code in some cases. A follow up branch is in progress for this problem. https://code.launchpad.net/~jcsackett/juju-gui/search-routing-and-view/+merge/160110 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 31

Patch Set 2 : Adds search functionality #

Patch Set 3 : Adds search functionality #

Patch Set 4 : Adds search functionality #

Patch Set 5 : Adds search functionality #

Unified diffs Side-by-side diffs Delta from patch set Stats (+367 lines, -116 lines) Patch
A [revision details] View 1 2 3 4 1 chunk +2 lines, -0 lines 0 comments Download
M app/modules-debug.js View 1 2 chunks +5 lines, -2 lines 0 comments Download
M app/subapps/browser/browser.js View 1 2 6 chunks +41 lines, -11 lines 0 comments Download
A app/subapps/browser/templates/search.handlebars View 1 1 chunk +4 lines, -0 lines 0 comments Download
M app/subapps/browser/views/charm.js View 1 2 2 chunks +3 lines, -14 lines 0 comments Download
M app/subapps/browser/views/editorial.js View 1 5 chunks +20 lines, -24 lines 0 comments Download
A app/subapps/browser/views/search.js View 1 2 3 1 chunk +110 lines, -0 lines 0 comments Download
M app/subapps/browser/views/view.js View 1 3 chunks +12 lines, -26 lines 0 comments Download
M app/templates/browser-search.handlebars View 1 chunk +1 line, -1 line 0 comments Download
M app/views/utils.js View 1 2 3 4 2 chunks +56 lines, -9 lines 0 comments Download
M app/widgets/charm-search.js View 1 4 chunks +21 lines, -5 lines 0 comments Download
M test/index.html View 1 chunk +10 lines, -8 lines 0 comments Download
M test/test_browser_charm_details.js View 1 2 1 chunk +0 lines, -1 line 0 comments Download
A test/test_browser_search_view.js View 1 2 1 chunk +78 lines, -0 lines 0 comments Download
M test/test_browser_search_widget.js View 1 3 chunks +4 lines, -15 lines 0 comments Download

Messages

Total messages: 7
j.c.sackett
Please take a look.
9 years, 7 months ago (2013-04-22 13:39:38 UTC) #1
rharding
LGTM With some comments - I really think the apiFailure as extension makes a lot ...
9 years, 7 months ago (2013-04-22 14:08:14 UTC) #2
jeff.pihach
LGTM - I'd like to see that apiFailure util moved into an extension as it's ...
9 years, 7 months ago (2013-04-22 14:20:55 UTC) #3
curtis
Hi Jon. Thank you for the branch. I have a question about a test. https://codereview.appspot.com/8910043/diff/1/app/subapps/browser/templates/search.handlebars ...
9 years, 7 months ago (2013-04-22 14:33:07 UTC) #4
j.c.sackett
Comments addressed; changes coming in submission. https://codereview.appspot.com/8910043/diff/1/app/modules-debug.js File app/modules-debug.js (right): https://codereview.appspot.com/8910043/diff/1/app/modules-debug.js#newcode310 app/modules-debug.js:310: fullpath: '/juju-ui/subapps/browser/views/utils.js' On ...
9 years, 7 months ago (2013-04-22 20:54:45 UTC) #5
curtis
LGTM, thanks.
9 years, 7 months ago (2013-04-22 21:01:35 UTC) #6
j.c.sackett
9 years, 7 months ago (2013-04-23 13:50:51 UTC) #7
*** Submitted:

Adds search functionality

This adds text search from the search widget
-Search widget propogates search text to browser subapp
-Subapp routes data to search view
-Search view does query from text
-Search view can render search results

As a driveby, this moves repeated apiFailure code out to an extension.

There is an issue outstanding:
-The querystring can be eaten by the routing code in some cases. A follow up
branch is in progress for this problem.

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

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