Code review comment for lp:~gmb/launchpad/ajax-dupefinder-has-risen-from-the-grave

Revision history for this message
Michael Nelson (michael.nelson) wrote :

> This branch does the final connecting up of JS to make the Asynchronous
> Dupefinder stuff work.

This is great! I've got three points below that I think are worth addressing - see what you think.

>
> How to test this branch (UI):
>
> 1. Go to launchpad.dev/firefox/+filebug
> 2. Enter a simple title (say, 'a') and hit "next"
> 3. You should get a list of possible duplicates for that bug.
> 4. You should be able to interact with those dupes in the same way as you
> always do (subscribe, mark as affecting you, etc).
> 5. You should also be able to file a new bug if you need to in the same way
> as you do currently.
>
> Issues that I'm aware of at this stage:
>
> 1. The animations seem to happen at the wrong time, so slide_ins get played
> after the element has already been rendered as closed. I'm looking into this.

OK - at first I thought this was a feature :-) (as I try not to read the MP before trying out the feature).

> 2. The way that the dupes appear in the page isn't exactly pretty. I'm open
> to suggestions about how this should work.

It would be nice to see them slide in... once you have the results, add them to the dom, but hidden, and then slide out the div containing them all?

My first thought is that the phrase: 'Is "a" one of these bugs?' seems strange. Even with a proper summary, 'Is "A bunch of soyuz source pages are missing headings" one of these bugs?' seems strange. Do you know the history of that decision? My guess would be that, since the summary entered by the user *wasn't* displayed anywhere else once they had searched, it needed to be included on the page somewhere. But now with your changes here, that the summary and input box remain on the page (at least for the JS version), I think we could just say something like 'Do any of the following bugs describe the bug you are trying to report?'. What do you think? It's great that they can now still see their search and update it if desired!

Secondly, currently in your branch it seems that you cannot have the 'No, I need to report a new bug' section and one of the other potential duplicates open at the same time. For example, as a person I:

1. enter a summary and hit Next,
2. briefly read the titles, but nothing rings a bell, so I click 'No, I need to report a new bug",
3. I start adding the further information, and then realise, hey I want to check one of those bugs above, as it actually could be the same,
4. I click on the expander and zap, my new bug description disappears.

Of course, the info I entered is still there, and I'll see it again if I click again on "No, I need to report a new bug", but I don't see any reason for it disappearing in the first place? (with less justification, the same goes in reverse, closing the expanded dups when I click on 'No, I need to report a new bug.').

Third, just an implementation detail, but hitting Enter while the focus in the summary field doesn't filter - it re-loads the page, which I'm assuming is not what you want.

Thanks for resurrecting this work! It'll be a great improvement :)

review: Needs Information (ui)

« Back to merge proposal