Merge lp:~sinzui/launchpad/bug-tag-enhacement into lp:launchpad

Proposed by Curtis Hovey
Status: Merged
Approved by: Curtis Hovey
Approved revision: no longer in the source branch.
Merged at revision: 15478
Proposed branch: lp:~sinzui/launchpad/bug-tag-enhacement
Merge into: lp:launchpad
Diff against target: 0 lines
To merge this branch: bzr merge lp:~sinzui/launchpad/bug-tag-enhacement
Reviewer Review Type Date Requested Status
j.c.sackett (community) Approve
Richard Harding (community) Approve
Review via email: mp+111639@code.launchpad.net

Commit message

Bug tag completer generates its own markup.

Description of the change

The bug tag completer is not portable. We cannot put it on +filebug or
advanced search without adding a lot of markup. The script could generate
its markup instead of assuming the page provides it which will make it
easier to reuse the code.

--------------------------------------------------------------------

RULES

    Pre-implementation: jcsackett
    * Pull the bug tag completer markup into the script.
    * There are actually two different blocks that could be merged
      into a single block that the bug tag completer manages.

    Addendum
    * I discovered that the failure case flashed green and the cancel
      case flashed red. Deryck, Jon, and Rick agreed they colours should
      be switched

QA

    * Visit https://bugs.qastaging.launchpad.net/gdp/+bug/459328
    * Verify you can remove all tags.
    * Verify the link reads "(+) Add tags"
    * Verify you can add and save a tag.
    * Verify the page shows the linked tags and there is an edit icon.

LINT

    lib/canonical/launchpad/icing/css/modifiers.css
    lib/lp/bugs/javascript/bug_tags_entry.js
    lib/lp/bugs/javascript/tests/test_bug_tags_entry.html
    lib/lp/bugs/javascript/tests/test_bug_tags_entry.js
    lib/lp/bugs/templates/bugtask-index.pt

TEST

    ./bin/test -vvc -t bug_tags_entry lp.bugs.tests.test_yui

IMPLEMENTATION

Updated the .hidden rule to have more precedence than form.inline. I made
a previous CSS fix this week by changing the order of classes. I think this
change is final. We want .hidden to override any other rule that sets the
display property.
    lib/canonical/launchpad/icing/css/modifiers.css

Simplified the bug-tag markup into one block for textual and graphical
browsers. The markup and the javascript do not need to work with add and
edit elements as separate features. The bug tag completer creates the
form and completer as needed. The bug tag completer hides the form
instead of each element in the form. I reversed the colours for failure
and cancel; the former should be red and the later should be green.
    lib/lp/bugs/javascript/bug_tags_entry.js
    lib/lp/bugs/javascript/tests/test_bug_tags_entry.html
    lib/lp/bugs/javascript/tests/test_bug_tags_entry.js
    lib/lp/bugs/templates/bugtask-index.pt

To post a comment you must log in.
Revision history for this message
Richard Harding (rharding) wrote :

Thanks so much for dealing with hiding/showing at the form level instead of each element. Makes a ton more sense. As part of that I don't think you need the variable handles to all of the elements and you can simplify the html setup [see #174 below].

#125
You can take advantage of the setAttrs() to set multiple attributes in one swoop. This can help performance with fewer redraws/etc.

#174
You can pass a list of nodes to the append method to avoid the multiple calls and creating the variables if you don't need them.

Example fiddle: http://jsfiddle.net/f4uVD/6/

review: Approve
Revision history for this message
j.c.sackett (jcsackett) wrote :

Looks good!

Thanks for doing this; it's nice to move this tag editor to a more reusable widget.

review: Approve

Preview Diff

Empty