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

Issue 8940043: Fixes several problems with search

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:
mp+160706, jeff.pihach, matthew.scott
Visibility:
Public.

Description

Fixes several problems with search -Querystring was eaten on viewmode changes or when a search url was pasted into browser. -Editorial and Search could have collisions in fullscreen. -Charm add button didn't work on search results charm tokens. Note: this reproduced (and now has merged) work in https://code.launchpad.net/~rharding/juju-gui/charm-details-1172050 https://code.launchpad.net/~jcsackett/juju-gui/search-fixes/+merge/160706 (do not edit description out of merge proposal)

Patch Set 1 #

Patch Set 2 : Fixes several problems with search #

Total comments: 3

Patch Set 3 : Fixes several problems with search #

Unified diffs Side-by-side diffs Delta from patch set Stats (+273 lines, -131 lines) Patch
A [revision details] View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M app/subapps/browser/browser.js View 1 2 29 chunks +135 lines, -122 lines 0 comments Download
M app/subapps/browser/views/editorial.js View 1 chunk +4 lines, -7 lines 0 comments Download
M app/subapps/browser/views/search.js View 2 chunks +48 lines, -0 lines 0 comments Download
M test/test_browser_app.js View 1 2 1 chunk +60 lines, -0 lines 0 comments Download
M test/test_browser_editorial.js View 2 chunks +0 lines, -2 lines 0 comments Download
M test/test_browser_search_view.js View 2 chunks +24 lines, -0 lines 0 comments Download

Messages

Total messages: 5
j.c.sackett
Please take a look.
10 years, 11 months ago (2013-04-24 16:46:52 UTC) #1
j.c.sackett
Please take a look.
10 years, 11 months ago (2013-04-24 16:49:07 UTC) #2
jeff.pihach
LGTM thanks, with trivial https://codereview.appspot.com/8940043/diff/3001/app/subapps/browser/browser.js File app/subapps/browser/browser.js (right): https://codereview.appspot.com/8940043/diff/3001/app/subapps/browser/browser.js#newcode24 app/subapps/browser/browser.js:24: * Show or hide the ...
10 years, 11 months ago (2013-04-24 17:05:57 UTC) #3
matthew.scott
LGTM with little comment change. Thanks for the branch. https://codereview.appspot.com/8940043/diff/3001/app/subapps/browser/browser.js File app/subapps/browser/browser.js (right): https://codereview.appspot.com/8940043/diff/3001/app/subapps/browser/browser.js#newcode496 app/subapps/browser/browser.js:496: ...
10 years, 11 months ago (2013-04-24 17:26:55 UTC) #4
j.c.sackett
10 years, 11 months ago (2013-04-24 18:30:49 UTC) #5
*** Submitted:

Fixes several problems with search

-Querystring was eaten on viewmode changes or when a search url was pasted
into browser.
-Editorial and Search could have collisions in fullscreen.
-Charm add button didn't work on search results charm tokens.

Note: this reproduced (and now has merged) work in
https://code.launchpad.net/~rharding/juju-gui/charm-details-1172050

R=jeff.pihach, matthew.scott
CC=
https://codereview.appspot.com/8940043
Sign in to reply to this message.

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