launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #09064
[Merge] lp:~sinzui/launchpad/bug-tags-edit-0 into lp:launchpad
Curtis Hovey has proposed merging lp:~sinzui/launchpad/bug-tags-edit-0 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1015315 in Launchpad itself: ""Edit" link text appears after editing bug tags"
https://bugs.launchpad.net/launchpad/+bug/1015315
For more details, see:
https://code.launchpad.net/~sinzui/launchpad/bug-tags-edit-0/+merge/111410
Graphical browsers show the edit icon next to the bug tags, after an
edit is performed, the edit action is redisplayed, except the text
"Edit" is now shown with the icon.
This is caused by the bug tag edit widget (well widget is a bit
presumptuous, more like a collective of behaviour embedded in a page)
which directly manipulates the anchors style instead of changing
classes. The code changes the anchors display from non to inline
(wrong). Instead the code should add or remove the hidden class.
--------------------------------------------------------------------
RULES
Pre-implementation: no one
* Update the code to add/remove the hidden class instead of changing
styles.
* setStyle() is called on several nodes. Maybe all calls to setStyle()
should instead be add/removeClass().
QA
* Visit https://bugs.qastaging.launchpad.net/gdp/+bug/459328
* Edit the bug tags
* Verify the edit icon is restored...there is no green "Edit" text
after it.
LINT
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
TEST
./bin/test -vvc -t test_bug_tags_entry lp.bugs.tests.test_yui
IMPLEMENTATION
I first removed lint from the module, and changed save_tag() so that it
could be instrumented by mockio. I added a YUI test for the setStyle()
behaviour. I then changed the module to add and remove the .hidden class.
I discovered that when there is a failure, the ok/cancel buttons are not
visible so you cannot resubmit. I fix this issue.
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.
--
https://code.launchpad.net/~sinzui/launchpad/bug-tags-edit-0/+merge/111410
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/bug-tags-edit-0 into lp:launchpad.
=== modified file 'lib/lp/bugs/javascript/bug_tags_entry.js'
--- lib/lp/bugs/javascript/bug_tags_entry.js 2012-03-15 05:40:30 +0000
+++ lib/lp/bugs/javascript/bug_tags_entry.js 2012-06-21 14:22:19 +0000
@@ -33,6 +33,7 @@
ESCAPE = 27,
TAGS_SHOW = 'tags-show',
TAGS_HIDE = 'tags-hide';
+ HIDDEN = 'hidden';
/**
* Grab all existing tags and insert them into the input
@@ -50,7 +51,7 @@
var tag_list = tags.join(' ');
/* If there are tags then add a space to the end of the string so the user
doesn't have to type one. */
- if (tag_list != "") {
+ if (tag_list !== "") {
tag_list += ' ';
}
tag_input.set(VALUE, tag_list);
@@ -68,21 +69,25 @@
return tags;
};
+
+namespace.lp_config = {};
+
+
/**
* Save the currently entered tags and switch inline editing off.
*
* @method save_tags
*/
var save_tags = function() {
- var lp_client = new Y.lp.client.Launchpad();
+ var lp_client = new Y.lp.client.Launchpad(namespace.lp_config);
var tags = namespace.parse_tags(tag_input.get(VALUE));
var bug = new Y.lp.client.Entry(
lp_client, LP.cache[BUG], LP.cache[BUG].self_link);
bug.removeAttr('http_etag');
bug.set('tags', tags);
- tags_edit_spinner.setStyle(DISPLAY, INLINE);
- ok_button.setStyle(DISPLAY, NONE);
- cancel_button.setStyle(DISPLAY, NONE);
+ tags_edit_spinner.removeClass(HIDDEN);
+ ok_button.addClass(HIDDEN);
+ cancel_button.addClass(HIDDEN);
bug.lp_save({on : {
success: function(updated_entry) {
var official_tags = [];
@@ -106,12 +111,12 @@
{tag_url: base_url + tag, tag: tag});
}).join(' ');
tag_list_span.set(INNER_HTML, tags_html);
- tag_input.setStyle(DISPLAY, NONE);
- tag_list_span.setStyle(DISPLAY, INLINE);
- ok_button.setStyle(DISPLAY, NONE);
- cancel_button.setStyle(DISPLAY, NONE);
- edit_tags_trigger.setStyle(DISPLAY, INLINE);
- tags_edit_spinner.setStyle(DISPLAY, NONE);
+ tag_input.addClass(HIDDEN);
+ tag_list_span.removeClass(HIDDEN);
+ ok_button.addClass(HIDDEN);
+ cancel_button.addClass(HIDDEN);
+ edit_tags_trigger.removeClass(HIDDEN);
+ tags_edit_spinner.addClass(HIDDEN);
Y.lp.anim.green_flash({ node: tag_list_span }).run();
if (Y.Lang.trim(tags_html) === '') {
Y.one('#bug-tags').removeClass(TAGS_SHOW);
@@ -121,7 +126,9 @@
}
},
failure: function(id, request) {
- tags_edit_spinner.setStyle(DISPLAY, NONE);
+ tags_edit_spinner.addClass(HIDDEN);
+ ok_button.removeClass(HIDDEN);
+ cancel_button.removeClass(HIDDEN);
Y.lp.anim.green_flash({ node: tag_list_span }).run();
}
}});
@@ -133,12 +140,12 @@
* @method cancel
*/
var cancel = function() {
- tag_input.setStyle(DISPLAY, NONE);
- tag_list_span.setStyle(DISPLAY, INLINE);
- ok_button.setStyle(DISPLAY, NONE);
- cancel_button.setStyle(DISPLAY, NONE);
- edit_tags_trigger.setStyle(DISPLAY, INLINE);
- tags_edit_spinner.setStyle(DISPLAY, NONE);
+ tag_input.addClass(HIDDEN);
+ tag_list_span.removeClass(HIDDEN);
+ ok_button.addClass(HIDDEN);
+ cancel_button.addClass(HIDDEN);
+ edit_tags_trigger.removeClass(HIDDEN);
+ tags_edit_spinner.addClass(HIDDEN);
autocomplete.hide();
Y.lp.anim.red_flash({ node: tag_list_span }).run();
if (Y.Lang.trim(tag_list_span.get('innerHTML')) === '') {
@@ -156,12 +163,12 @@
*/
var edit = function() {
populate_tags_input();
- tag_list_span.setStyle(DISPLAY, NONE);
- tag_input.setStyle(DISPLAY, INLINE);
+ tag_list_span.addClass(HIDDEN);
+ tag_input.removeClass(HIDDEN);
tag_input.focus();
- edit_tags_trigger.setStyle(DISPLAY, NONE);
- ok_button.setStyle(DISPLAY, INLINE);
- cancel_button.setStyle(DISPLAY, INLINE);
+ edit_tags_trigger.addClass(HIDDEN);
+ ok_button.removeClass(HIDDEN);
+ cancel_button.removeClass(HIDDEN);
autocomplete.render();
};
@@ -214,7 +221,7 @@
});
tag_input.on('keydown', function(e) {
- if (e.keyCode == ESCAPE) {
+ if (e.keyCode === ESCAPE) {
e.halt();
cancel();
}
=== modified file 'lib/lp/bugs/javascript/tests/test_bug_tags_entry.html'
--- lib/lp/bugs/javascript/tests/test_bug_tags_entry.html 2012-03-14 04:41:36 +0000
+++ lib/lp/bugs/javascript/tests/test_bug_tags_entry.html 2012-06-21 14:22:19 +0000
@@ -31,21 +31,61 @@
src="../../../../../build/js/lp/app/client.js"></script>
<script type="text/javascript"
src="../../../../../build/js/lp/app/testing/mockio.js"></script>
+ <script type="text/javascript"
+ src="../../../../../build/js/lp/app/anim/anim.js"></script>
+ <script type="text/javascript"
+ src="../../../../../build/js/lp/app/extras/extras.js"></script>
+ <script type="text/javascript"
+ src="../../../../../build/js/lp/app/autocomplete/autocomplete.js"></script>
<!-- The module under test. -->
<script type="text/javascript" src="../bug_tags_entry.js"></script>
- <!-- Any css assert for this module. -->
- <!-- <link rel="stylesheet" href="../assets/bug_tags_entry-core.css" /> -->
-
<!-- The test suite. -->
<script type="text/javascript" src="test_bug_tags_entry.js"></script>
</head>
<body class="yui3-skin-sam">
<ul id="suites">
- <!-- <li>lp.large_indicator.test</li> -->
<li>lp.bug_tags_entry.test</li>
</ul>
+
+ <div id="fixture"></div>
+
+ <script type="text/x-template" id="bug-tag-form">
+ <!-- Has tags -->
+ <div id="bug-tags" class="tags-show">
+ Tags:
+ <span id="tag-list">
+ <a class="official-tag"
+ href="/fnord/+bugs?field.tag=project-tag">project-tag</a>
+ <a class="unofficial-tag"
+ href="/fnord/+bugs?field.tag=user-tag">user-tag</a>
+ </span>
+
+ <form id="tags-form" style="display: inline">
+ <input type="text" id="tag-input" class="hidden" />
+ <img src="/@@/spinner" id="tags-edit-spinner" class="hidden"/>
+ <a href="+edit" title="Edit tags" id="edit-tags-trigger"
+ class="sprite edit action-icon">Edit</a>
+
+ <button class="lazr-pos lazr-btn yui-ieditor-submit_button hidden"
+ id="edit-tags-ok" type="button">Ok</button>
+ <button class="lazr-neg lazr-btn yui-ieditor-cancel_button hidden"
+ id="edit-tags-cancel" type="button">Cancel</button>
+ </form>
+
+ <a href="/+help-bugs/tag-help.html" target="help"
+ class="sprite maybe action-icon">Tag help</a>
+ </div>
+
+ <!-- No tags -->
+ <div id="add-bug-tags" class="tags-hide">
+ <a href="+edit" title="Add tags" id="add-tags-trigger"
+ class="sprite add">Add tags</a>
+ <a href="/+help-bugs/tag-help.html" target="help"
+ class="sprite maybe action-icon">Tag help</a>
+ </div>
+ </script>
</body>
</html>
=== modified file 'lib/lp/bugs/javascript/tests/test_bug_tags_entry.js'
--- lib/lp/bugs/javascript/tests/test_bug_tags_entry.js 2012-01-05 18:25:04 +0000
+++ lib/lp/bugs/javascript/tests/test_bug_tags_entry.js 2012-06-21 14:22:19 +0000
@@ -40,5 +40,101 @@
}));
+ suite.add(new Y.Test.Case({
+ name: 'Actions',
+
+ setUp: function() {
+ this.fixture = Y.one("#fixture");
+ var template = Y.one('#bug-tag-form').getContent();
+ this.fixture.append(template);
+ this.edit_tags_trigger = Y.one('#edit-tags-trigger');
+ this.add_tags_trigger = Y.one('#add-tags-trigger');
+ this.tag_list_span = Y.one('#tag-list');
+ this.tag_input = Y.one('#tag-input');
+ this.ok_button = Y.one('#edit-tags-ok');
+ this.cancel_button = Y.one('#edit-tags-cancel');
+ this.tags_edit_spinner = Y.one('#tags-edit-spinner');
+ window.LP = {
+ links: {me : "/~user"},
+ cache: {
+ bug: {
+ resource_type_link: 'Bug',
+ self_link: '/bug/1',
+ tags: ['project-tag']}
+ }
+ };
+ },
+
+ tearDown: function () {
+ if (this.fixture !== null) {
+ this.fixture.empty();
+ }
+ delete this.fixture;
+ delete window.LP;
+ },
+
+ test_edit: function () {
+ module.setup_tag_entry(['project-tag']);
+ this.edit_tags_trigger.simulate('click');
+ Y.Assert.isTrue(this.tag_list_span.hasClass('hidden'));
+ Y.Assert.isTrue(this.edit_tags_trigger.hasClass('hidden'));
+ Y.Assert.isFalse(this.tag_input.hasClass('hidden'));
+ Y.Assert.isFalse(this.ok_button.hasClass('hidden'));
+ Y.Assert.isFalse(this.cancel_button.hasClass('hidden'));
+ },
+
+ test_cancel: function () {
+ module.setup_tag_entry(['project-tag']);
+ this.edit_tags_trigger.simulate('click');
+ this.cancel_button.simulate('click');
+ Y.Assert.isFalse(this.tag_list_span.hasClass('hidden'));
+ Y.Assert.isFalse(this.edit_tags_trigger.hasClass('hidden'));
+ Y.Assert.isTrue(this.tag_input.hasClass('hidden'));
+ Y.Assert.isTrue(this.ok_button.hasClass('hidden'));
+ Y.Assert.isTrue(this.cancel_button.hasClass('hidden'));
+ Y.Assert.isTrue(this.tags_edit_spinner.hasClass('hidden'));
+ },
+
+ test_save_tags_success: function () {
+ module.setup_tag_entry(['project-tag']);
+ this.edit_tags_trigger.simulate('click');
+ var mockio = new Y.lp.testing.mockio.MockIo();
+ module.lp_config = {io_provider: mockio};
+ this.ok_button.simulate('click');
+ Y.Assert.isFalse(this.tags_edit_spinner.hasClass('hidden'));
+ Y.Assert.areEqual(
+ '/api/devel/bug/1', mockio.last_request.url);
+ mockio.success({
+ responseText: Y.JSON.stringify({
+ resource_type_link: 'Bug',
+ self_link: '/bug/1',
+ tags: ['project-tag']}),
+ responseHeaders: {'Content-Type': 'application/json'}});
+ Y.Assert.isFalse(this.tag_list_span.hasClass('hidden'));
+ Y.Assert.isFalse(this.edit_tags_trigger.hasClass('hidden'));
+ Y.Assert.isTrue(this.tag_input.hasClass('hidden'));
+ Y.Assert.isTrue(this.ok_button.hasClass('hidden'));
+ Y.Assert.isTrue(this.cancel_button.hasClass('hidden'));
+ Y.Assert.isTrue(this.tags_edit_spinner.hasClass('hidden'));
+ },
+
+ test_save_tags_failure: function () {
+ module.setup_tag_entry(['project-tag']);
+ this.edit_tags_trigger.simulate('click');
+ var mockio = new Y.lp.testing.mockio.MockIo();
+ module.lp_config = {io_provider: mockio};
+ this.ok_button.simulate('click');
+ mockio.failure({
+ responseText: Y.JSON.stringify({
+ resource_type_link: 'Bug',
+ self_link: '/bug/1',
+ tags: ['project-tag']}),
+ responseHeaders: {'Content-Type': 'application/json'}});
+ Y.Assert.isFalse(this.ok_button.hasClass('hidden'));
+ Y.Assert.isFalse(this.cancel_button.hasClass('hidden'));
+ Y.Assert.isTrue(this.tags_edit_spinner.hasClass('hidden'));
+ }
+ }));
+
Y.lp.testing.Runner.run(suite);
});
Follow ups
