> > === modified file 'lib/canonical/launchpad/javascript/bugs/filebug-dupefinder.js' > > --- lib/canonical/launchpad/javascript/bugs/filebug-dupefinder.js 2009-12-08 10:57:44 +0000 > > +++ lib/canonical/launchpad/javascript/bugs/filebug-dupefinder.js 2009-12-09 12:13:43 +0000 > > @@ -81,13 +81,6 @@ > > * @param e The Event triggering this function. > > */ > > function show_bug_reporting_form(e) { > > - if (Y.Lang.isValue(bug_already_reported_expanders)) { > > - // Collapse any duplicate-details divs. > > - Y.each(bug_already_reported_expanders, function(expander) { > > - collapse_bug_details(expander); > > - }); > > - } > > - > > // If the bug reporting form is in a hidden container, as it is on > > // the AJAX dupe search, show it. > > var filebug_form_container = Y.get('#filebug-form-container'); > > Salgado, in his YUI() -> LPS branch, changed a lot of Y.get() calls to > Y.one(). I guess that means things break non-silently when the targets > don't exist. Might be a good idea here too. > Right. Fixed. > > @@ -117,8 +110,37 @@ > > * display them in-line. > > */ > > function search_for_and_display_dupes() { > > - function show_failure_message() { > > - Y.get('#possible-duplicates').set(INNER_HTML, 'FAIL'); > > + function show_failure_message(transaction_id, response, args) { > > + // If the request failed due to a timeout, display a message > > + // explaining how the user may be able to work around it. > > + var error_message = ''; > > + if (response.status == 503) { > > + // We treat 503 (service unavailable) as a timeout because > > + // that's what timeouts in LP return. > > + error_message = > > + "

Searching for your bug in Launchpad took too long. " + > > + "Try reducing the number of words in the summary " + > > + "field and click \"Check again\" to retry your search. " + > > + "Alternatively, you can enter the details of your bug " + > > + "below.

"; > > + } else { > > + // Any other error code gets a generic message. > > + error_message = > > + "

An error occured whilst trying to find bugs matching " + > > + "the summary you entered. Click \"Check again\" to retry " + > > + "your search. Alternatively, you can enter the " + > > + "details of your bug below.

"; > > + } > > + > > + Y.get('#possible-duplicates').set(INNER_HTML, error_message); > > It works, but I think using innerHTML is bad except for a few > cases. Consider having error_message as just a message, then create a >

node and .set('text', error_message). Don't stress too much > though. > I agree. Fixed. > > + > > + Y.get('#spinner').setStyle(DISPLAY, NONE); > > Consider using the "unseen" CSS class instead, and remove it to make > the spinner visible. Doesn't matter too much though. I'm sure I've > heard arguments that classes are better than manipulating style > directly, but I can't summon them right now. > o I've heard similar from other people. I've changed the setStyle() calls to (add|remove)Class() as appropriate. > > + show_bug_reporting_form(); > > + > > + Y.get(Y.DOM.byId('field.title')).set( > > + 'value', search_field.get('value')); > > I think this would be clearer and cleaner as: > > Y.one('#bug_reporting_form input[name=field.title]').set('value', ...) > > I wish Zope forms didn't define ids for fields. It makes it more > difficult to combine different forms into a page. > Agreed and fixed. > > + search_button.set('value', 'Check again'); > > + search_button.setStyle(DISPLAY, INLINE); > > Perhaps disable the sar (and re-enable here) the search button rather than > making it disappear altogether? Taking it out of the page might mean > the browser has to do layout again. > Hmm, possibly. The disappearance of the button was a UI request from beuno back in the day, so I think we should leave it as-is for now and see how testing on edge goes. > > } > > > > function on_success(transaction_id, response, args) { > > @@ -131,18 +153,11 @@ > > bug_already_reported_expanders = Y.all( > > 'img.bug-already-reported-expander'); > > if (Y.Lang.isValue(bug_already_reported_expanders)) { > > - // If there are duplicates shown, change the title of the page > > - // and set up the JavaScript of the duplicates that have been > > - // returned. > > + // If there are duplicates shown, set up the JavaScript of > > + // the duplicates that have been returned. > > Y.bugs.setup_dupes(); > > - Y.get('#page-title').set( > > - INNER_HTML, > > - 'Is the bug you’re reporting one of these?'); > > } else { > > - // Otherwise, set the title to one that doesn't suggest > > - // there were dupes returned and show the bug reporting > > - // form. > > - Y.get('#page-title').set(INNER_HTML, 'Report a bug'); > > + // Otherwise, show the bug reporting form. > > show_bug_reporting_form(); > > } > > > > @@ -242,7 +257,7 @@ > > var weight = radio_button.get('checked') ? 'bold' : 'normal'; > > radio_button.get('parentNode').setStyle('fontWeight', weight); > > }); > > - }); > > + } > > > > return subscribe_form_overlay; > > } > > @@ -271,18 +286,18 @@ > > search_field.set('name', 'field.search'); > > search_field.set('id', 'field.search'); > > > > - // Disable the form so that hitting "enter" in the Summary > > - // field no longer sends us through to the next page. > > - // Y.on('submit', function(e) { e.halt(); }, '#my-form') > > - > > // Update the label on the search button so that it no longer > > // says "Continue". > > search_button.set('value', 'Next'); > > - search_button.set('type', 'button'); > > > > - // Set up the handlers for the search button and the input > > - // field. > > - search_button.on('click', search_for_and_display_dupes); > > + // Set up the handler for the search form. > > + search_form = Y.get('#filebug-search-form'); > > + search_form.on('submit', function(e) { > > + // Prevent the event from propagating; we don't want to reload > > + // the page. > > + e.halt(); > > + search_for_and_display_dupes(); > > + }); > > } > > > > Y.bugs.setup_dupes = function() { > > @@ -352,11 +367,6 @@ > > > > image.set(SRC, EXPANDER_EXPANDED); > > } > > - > > - // If the bug reporting form is shown, hide it. > > - if (bug_reporting_form.getStyle(DISPLAY) == BLOCK) { > > - bug_reporting_form.setStyle(DISPLAY, NONE); > > - } > > }); > > }); > > > > @@ -386,6 +396,7 @@ > > }; > > > > Y.bugs.setup_dupe_finder = function() { > > + Y.log("In setup_dupe_finder"); > > Y.on('domready', function() { > > config = {on: {success: set_up_dupe_finder, > > failure: function() {}}}; > > @@ -404,4 +415,4 @@ > > }; > > > > }, "0.1", {"requires": [ > > - "base", "oop", "node", "event", "lazr.formoverlay", "lazr.effects"]}); > > + "base", "io", "oop", "node", "event", "lazr.formoverlay", "lazr.effects"]}); > >