Bug 26950 (BugzillaSummaryEdit) - Make the summary and alias fields support click-to-edit
Summary: Make the summary and alias fields support click-to-edit
Alias: BugzillaSummaryEdit
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Maciej Stachowiak
URL: https://bugzilla.mozilla.org/show_bug...
Depends on: 17457
  Show dependency treegraph
Reported: 2009-07-03 00:59 PDT by Maciej Stachowiak
Modified: 2009-07-03 17:22 PDT (History)
1 user (show)

See Also:

patch (3.79 KB, patch)
2009-07-03 01:01 PDT, Maciej Stachowiak
no flags Details | Formatted Diff | Diff
patch (3.79 KB, patch)
2009-07-03 01:19 PDT, Maciej Stachowiak
ddkilzer: review-
Details | Formatted Diff | Diff
Patch v3 (4.41 KB, patch)
2009-07-03 10:23 PDT, David Kilzer (:ddkilzer)
mjs: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Maciej Stachowiak 2009-07-03 00:59:31 PDT
Make the summary and alias fields support click-to-edit, so Bugzilla can be cool like Flickr.
Comment 1 Maciej Stachowiak 2009-07-03 01:01:22 PDT
Created attachment 32224 [details]
Comment 2 Maciej Stachowiak 2009-07-03 01:01:59 PDT
My patch is completely untested - I could not figure out how to set up a bugzilla test server to try it out.
Comment 3 Maciej Stachowiak 2009-07-03 01:19:35 PDT
Created attachment 32225 [details]
Comment 4 David Kilzer (:ddkilzer) 2009-07-03 10:20:43 PDT
Comment on attachment 32225 [details]

>-function hideEditableField( container, input, action, field_id, original_value ) {
>+function hideEditableField( container, input, action, field1_text, field2_text, field_id, original_value ) {
>     YAHOO.util.Dom.setStyle(container, 'display', 'inline');
>     YAHOO.util.Dom.setStyle(input, 'display', 'none');
>-    YAHOO.util.Event.addListener(action, 'click', showEditableField,
>+    YAHOO.util.Event.addListener(action, 'click', showEditableFieldFocusLast,
>+                                 new Array(container, input));
>+    YAHOO.util.Event.addListener(field2_text, 'click', showEditableFieldFocusLast,
>+                                 new Array(container, input));
>+    YAHOO.util.Event.addListener(field1_text, 'click', showEditableField,
>                                  new Array(container, input));

Since field2_text is field_id with "_nonedit_display" appended and field1_text is "alias_nonedit_display", I don't think you need to add two parameters here.

Also, adding a new function, showEditableFieldFocusLast(), instead of adding arguments to the array object goes against the design of the JavaScript used elsewhere in the file.

>-function showEditableField (e, ContainerInputArray) {
>+function showEditableField (e, ContainerInputArray, focusLast) {

The "focusLast" argument should be passed in with the ContainerInputArray argument.

>     }
>     YAHOO.util.Event.preventDefault(e);
> }

Gratuitous whitespace change.

>+function showEditableFieldFocusLast (e, ContainerInputArray) {
>+    showEditableField(e, ContainerInputArray, true);

This function won't be needed if you add the "focusLast" argument to the ContainerInputArray.

>     hideEditableField( 'summary_alias_container','summary_alias_input',
>-                       'editme_action','short_desc', short_desc_value);  
>+                       'editme_action', 'alias_nonedit_display', 'short_desc_nonedit_display', 'short_desc', short_desc_value);  

Again, I think we only need to add one argument to hideEditableField().

r- for the above issues.  (Patches to Bugzilla should try to match the existing design to make them easier to merge upstream.)

FWIW, this patch did work using a local copy of the original page (saved as HTML) with a <base> tag added and the URL to the field.js adjusted to use a local copy.

I have an updated patch that I'll post next.
Comment 5 David Kilzer (:ddkilzer) 2009-07-03 10:23:01 PDT
Created attachment 32240 [details]
Patch v3

Patch with issues in Comment #4 addressed.
Comment 6 David Kilzer (:ddkilzer) 2009-07-03 10:34:57 PDT
Filed a Bugzilla bug for this enhancement request:

Bug 502249: Make the short description and alias fields support click-to-edit
Comment 7 Maciej Stachowiak 2009-07-03 14:02:56 PDT
Comment on attachment 32240 [details]
Patch v3

r=me on your changes. Not sure which one of us should land the patch now. :-)
Comment 8 David Kilzer (:ddkilzer) 2009-07-03 16:43:03 PDT
(In reply to comment #7)
> (From update of attachment 32240 [details])
> r=me on your changes. Not sure which one of us should land the patch now. :-)

I got it.  :)
Comment 9 David Kilzer (:ddkilzer) 2009-07-03 17:21:24 PDT
$ git svn dcommit
Committing to http://svn.webkit.org/repository/webkit/trunk ...
        M       BugsSite/ChangeLog
        M       BugsSite/js/field.js
Committed r45538