launchpad-reviewers team mailing list archive

["j.c.sackett" <jonathan.sackett@xxxxxxxxxxxxx>]

j.c.sackett has proposed merging lp:~jcsackett/launchpad/fix-bottom-searchfield-684151 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #684151 Search field at the bottom of a search results page never works
  https://bugs.launchpad.net/bugs/684151


Summary
=======

On search result pages from +search, there is a secondary search form below the result set for large results. This second form doesn't work, because it generates a form with the text input element having a name of "text-secondary" as a result of it ape-ing the id, which must be text-secondary to avoid a collision with the primary form at the top of the page.

To fix this, the name should be switched from text-secondary to text.

Implementation
==============

In the template for the form, the section setting the name for the element postfixed it with the suffix -secondary.

Tests
=====

bin/test -t launchpad-search-pages

Demo & QA
=========

Go to the search page; enter a search in the secondary form. It should work.
-- 
https://code.launchpad.net/~jcsackett/launchpad/fix-bottom-searchfield-684151/+merge/43584
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/fix-bottom-searchfield-684151 into lp:launchpad.

=== modified file 'lib/lp/app/browser/tests/launchpad-search-pages.txt'
--- lib/lp/app/browser/tests/launchpad-search-pages.txt	2010-10-09 16:36:22 +0000
+++ lib/lp/app/browser/tests/launchpad-search-pages.txt	2010-12-13 22:31:21 +0000
@@ -1,4 +1,5 @@
-= Launchpad search page =
+Launchpad search page
+=====================
 
 Users can search for Launchpad objects and pages from the search form
 located on all pages. The search is performed and displayed by the
@@ -16,7 +17,8 @@
     <....SimpleViewClass from .../templates/launchpad-search.pt ...>
 
 
-== Page title and heading ==
+Page title and heading
+----------------------
 
 The page title and heading suggest to the user to search launchpad
 when there is no search text.
@@ -55,7 +57,8 @@
     u'Pages matching "albatross" in Launchpad'
 
 
-== No matches ==
+No matches
+----------
 
 There were no matches for 'albatross'.
 
@@ -75,7 +78,8 @@
     False
 
 
-== bug and question searches ==
+Bug and Question Searches
+-------------------------
 
 When a numeric token can be extracted from the submitted search text,
 the view tries to match a bug and question. Bugs and questions are
@@ -189,7 +193,8 @@
 
 
 
-== Projects and Persons and Teams searches ==
+Projects and Persons and Teams searches
+---------------------------------------
 
 When a Launchpad name can be made from the search text, the view tries
 to match the name to a pillar or person. a pillar is a distribution,
@@ -314,7 +319,8 @@
     None
 
 
-== Shipit CD searches ==
+Shipit CD searches
+------------------
 
 The has_shipit property will be True when the search looks like the user
 is searching for Shipit CDs. There is no correct object in Launchpad to
@@ -373,7 +379,8 @@
     'http://www.ubuntu.com/getubuntu/shipit-faq'
 
 
-== Page searches ==
+Page searches
+-------------
 
 The view uses the GoogleSearchService to locate pages that match the
 search terms.
@@ -493,7 +500,8 @@
 GoogleSearchService and PageMatch objects.
 
 
-== No page matches ==
+No page matches
+---------------
 
 When an empty PageMatches object is returned by the GoogleSearchService to
 the view, there are no matches to show.
@@ -504,7 +512,8 @@
     False
 
 
-== Bad Google response handling ==
+Bad Google response handling
+----------------------------
 
 Connectivity problems can cause missing or incomplete responses from
 Google. The LaunchpadSearchView will display the other searches and
@@ -536,7 +545,8 @@
     'The response was incomplete, no xml.'
 
 
-== SearchFormView and SearchFormPrimaryView ==
+SearchFormView and SearchFormPrimaryView
+----------------------------------------
 
 Two companion views are used to help render the global search form.
 They define the required attributes to render the form in the
@@ -560,7 +570,7 @@
       name="sitesearch-secondary">
       <div>
         <input class="textType" type="text" size="36"
-          id="field.text-secondary" name="field.text-secondary" />
+          id="field.text-secondary" name="field.text" />
         <input class="button" type="submit" value="Search"
           id="field.text-secondary" name="field.actions.search-secondary" />
       </div>
@@ -588,8 +598,8 @@
       </div>
     </form>
 
-
-== WindowedList and GoogleBatchNavigator ==
+WindowedList and GoogleBatchNavigator
+-------------------------------------
 
 The LaunchpadSearchView uses two helper classes to work with
 PageMatches.

=== modified file 'lib/lp/app/templates/launchpad-search-form.pt'
--- lib/lp/app/templates/launchpad-search-form.pt	2010-01-08 21:23:15 +0000
+++ lib/lp/app/templates/launchpad-search-form.pt	2010-12-13 22:31:21 +0000
@@ -37,7 +37,7 @@
           name=""
           tal:attributes="
             id string:field.text${view/id_suffix};
-            name string:field.text${view/id_suffix};
+            name string:field.text;
             value view/text;
           "
         />