Merge lp://staging/~beuno/loggerhead/bzr-search_integration into lp://staging/loggerhead

Proposed by Robert Collins
Status: Merged
Merge reported by: Martin Albisetti
Merged at revision: 182
Proposed branch: lp://staging/~beuno/loggerhead/bzr-search_integration
Merge into: lp://staging/loggerhead
To merge this branch: bzr merge lp://staging/~beuno/loggerhead/bzr-search_integration
Reviewer Review Type Date Requested Status
Martin Albisetti Approve
Review via email: mp+480@code.staging.launchpad.net
To post a comment you must log in.
273. By Martin Albisetti

Merge from trunk, resolved conflicts

274. By Martin Albisetti

 * Fixed Copyright assignment

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

So, sorry for taking such a very long time to get to this.

I have a few comments, mostly cosmetic:

The change in size/layout of the suggestions div when it goes from "Loading..." to the suggestions is a bit disconcerting.

The padding on the suggestions box seems excessive -- what I think would be ideal is the left edge of the suggestions lined up with the left edge of the text in the box you are typing in.

Things get a little silly when the list of suggestions is very long -- the suggestions div is way way longer than the page.

Shouldn't the selection box disappear when the search field is un-focused?

Is it easily possible to allow the use of the keyboard for selecting suggestions?

It's pretty hard to see why the results that are returned are returned and also the order is a bit incomprehensible.

Some of these (particularly the last two) can be done after the code has landed. But let's have a talk about the various points before that happens.

The code is all fine at least :)

Revision history for this message
Martin Albisetti (beuno) wrote :

Addressed all comments :)

review: Approve
275. By Martin Albisetti

Merge from trunk

276. By Martin Albisetti

 * fixed padding
 * hide results when removing the focus from input
 * make results narrower and shorter

277. By Martin Albisetti

Delay hiding the div so you can actually click on the results

278. By Martin Albisetti

Tell the user when no results where found

Subscribers

People subscribed via source and target branches