Merge lp:~henninge/launchpad/bug-410864-undefined-in-picker into lp:launchpad
Status: | Merged |
---|---|
Approved by: | Curtis Hovey |
Approved revision: | no longer in the source branch. |
Merged at revision: | 13196 |
Proposed branch: | lp:~henninge/launchpad/bug-410864-undefined-in-picker |
Merge into: | lp:launchpad |
Diff against target: |
472 lines (+155/-94) 9 files modified
lib/canonical/launchpad/webapp/configure.zcml (+2/-2) lib/lp/app/javascript/picker.js (+21/-44) lib/lp/app/javascript/tests/test_personpicker.html (+1/-1) lib/lp/app/javascript/tests/test_personpicker.js (+4/-4) lib/lp/app/javascript/tests/test_picker.html (+1/-0) lib/lp/app/javascript/tests/test_picker.js (+36/-9) lib/lp/app/javascript/widgets.js (+75/-32) lib/lp/app/widgets/popup.py (+13/-1) lib/lp/app/widgets/templates/form-picker-macros.pt (+2/-1) |
To merge this branch: | bzr merge lp:~henninge/launchpad/bug-410864-undefined-in-picker |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Curtis Hovey (community) | code | Approve | |
Review via email: mp+63911@code.launchpad.net |
Commit message
[r=sinzui][bug=410864,787595] Work on the picker widget: Retargets the personpicker yui widget to extend the new lp-picker yui widget. Fixes a regression that let the result batches run out of proportion. Fixes the 'too many results' display.
Description of the change
= Summary =
Apart from fixing the linked bug this branch also piggybacks a fix for a
regression introduced by the fix for bug 778129 (my bad).
This regression caused the picker widget to display all the batches of
results even if a search box was available. Instead it should display
the "Too many search results" message and not display any results.
The reason for bug 410864 lies in lazr-js assuming that an empty result
list indicates that a search returned no results. But enforcing the
batch limit also creates empty result list in which case the
message "No items matched ..." produced by lazr-js is not fitting.
To prevent that message, a one-item result list with an empty item
was created which caused "undefined" to appear as the title of that
one item.
== Proposed fix ==
For the regression: The code used "config.
create() method but config might not always have such a member. So
create() needs to mimic the behavior of the createPickerPat
method and default show_search_box to true when it is undefined.
For the actual bug: The render method needs to be aware that it
could get an empty data as a result and produce an empty result
element accordingly. A better fix would be to make lazr-js's
behavior regarding empty result lists configurable but I went for the
least-impact solution. Another squad is doing work on the picker and
they may well touch this.
== Pre-implementation notes ==
Chatted a big with deryck, did not catch Curtis on IRC.
== Tests ==
lib/lp/
== Demo and Q/A ==
Go to a bug page, open the picker for the bug target and enter
"linux". You should get a "Too many results" message but no
"undefined" in the empty list below.
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/
lib/lp/
./lib/lp/
337: 'Picker' has not been fully defined yet.
Thank you for this change. I think the branch conflicts with https:/ /code.launchpad .net/~jcsackett /launchpad/ oh-oh-pick- me-pick- me-3 which moved modules and chunks of code. I recognise the area you made changes. You may need to make these same changes to lp.app. javascript. widgets