Permalink
Browse files

(Bug 4780) Update commentmanage to use new tooltips

- instead of using a tooltip for a dynamic form (what was I
  thinking???), use a dialog. Immediate plusses are that we don't have
  to worry about keeping the tooltip persistent. May also help with
  screen reader accessibility because the role is now more semantically
  correct (dialog). Also keyboard accessibility...

- various tweaks in commentmanage

- use self.error instead of trying to use the content option's callback

- include new files. Limit scope of iconselector CSS so it doesn't
  affect other dialog boxes

- fix tests
  • Loading branch information...
1 parent 3d1516c commit 1a99e7c71e79bfe15531f81600c0ed4645f51a3b @afuna afuna committed Oct 24, 2012
View
7 cgi-bin/LJ/S2/EntryPage.pm
@@ -412,12 +412,15 @@ sub EntryPage
LJ::need_res( { group => "jquery" }, qw(
js/jquery/jquery.ui.tooltip.js
js/jquery.ajaxtip.js
+ js/jquery/jquery.ui.button.js
+ js/jquery/jquery.ui.dialog.js
js/jquery.commentmanage.js
- js/tooltip.js
js/jquery/jquery.ui.position.js
stc/jquery/jquery.ui.tooltip.css
+ stc/jquery/jquery.ui.button.css
+ stc/jquery/jquery.ui.dialog.css
stc/ajaxtip.css
- stc/popup-form.css
+ stc/jquery.commentmanage.css
) );
LJ::need_res( LJ::S2::tracking_popup_js() );
View
9 htdocs/js/dw/dw-core.js
@@ -51,14 +51,7 @@ $.extractParams = function(url) {
$.throbber = {
src: Site.imgprefix + "/ajax-loader.gif",
- error: Site.imgprefix + "/silk/site/error.png",
- image: function() {
- return $("<img />", {
- src: $.throbber.src,
- alt: "Loading..."
- } )
- ;
- }
+ error: Site.imgprefix + "/silk/site/error.png"
};
$.endpoint = function(action){
View
23 htdocs/js/jquery.ajaxtip.js
@@ -80,18 +80,13 @@ $.widget("dw.ajaxtip", $.ui.tooltip, {
// now add the throbber. It will be removed automatically
$(self.element).throbber( deferred );
- self.option( "content", function(setContent) {
- deferred.done(function() {
- // setContent();
- });
-
- deferred.fail(function(jqxhr, status, error) {
- // "abort" status means we cancelled the ajax request
- if ( status !== "abort" ) {
- setContent( "Error contacting server: " + error );
- }
- });
+ deferred.fail(function(jqxhr, status, error) {
+ // "abort" status means we cancelled the ajax request
+ if ( status !== "abort" ) {
+ self.error( "Error contacting server: " + error );
+ }
});
+
}
});
@@ -134,11 +129,7 @@ $.widget("dw.ajaxtipold", $.ui.tooltip, {
self._reposition( tip );
}
}, self.options.tooltip));
- },
- _init: function() {
- if(this.options.content)
- this.element.data("tooltip").show()
- },
+ }
});
*/
// TODO: make this work
View
177 htdocs/js/jquery.commentmanage.js
@@ -23,7 +23,7 @@ $.widget("dw.moderate", {
journal: undefined,
form_auth: undefined,
- endpoint: "__rpc_talkscreen",
+ endpoint: "talkscreen"
},
_updateLink: function(newData) {
this.element.attr("href", newData.newurl);
@@ -50,13 +50,11 @@ $.widget("dw.moderate", {
_abort: function(reason, ditemid) {
ditemid = ditemid || this.linkdata.id;
- this.element.ajaxtip({namespace:"moderate"})
- .ajaxtip( "abort", "Error moderating comment #" + ditemid + ". " + reason);
+ this.element.ajaxtip().ajaxtip( "error", "Error moderating comment #" + ditemid + ". " + reason);
},
_create: function() {
var self = this;
-
var params = $.extractParams(this.element.attr("href"));
this.linkdata = {
id: params.talkid || "",
@@ -83,31 +81,36 @@ $.widget("dw.moderate", {
return;
}
- var posturl = "/" + self.options.journal + "/" + self.options.endpoint
- + "?jsmode=1&json=1&mode=" + self.linkdata.action;
+ var endpoint = self.options.endpoint +
+ "?jsmode=1&json=1&mode=" + self.linkdata.action;
self.element
- .ajaxtip({
- namespace: "moderate",
- })
- .ajaxtip("load", {
- url: posturl,
- context: self,
- data: {
- talkid : self.linkdata.id,
- journal : self.options.journal,
-
- confirm : "Y",
- lj_form_auth: self.options.form_auth
- },
- success: function( data, status, jqxhr ) {
- if ( data.error ) {
- self.element.ajaxtip( "error", "Error while trying to " + self.linkdata.action + ": " + data.error )
- } else {
- self.element.ajaxtip("success",data.msg);
- self._updateLink(data);
+ .ajaxtip() // init
+ .ajaxtip( "load", {
+ endpoint: endpoint,
+
+ ajax: {
+ type: "POST",
+
+ context: self,
+
+ data: {
+ talkid : self.linkdata.id,
+ journal : self.options.journal,
+
+ confirm : "Y",
+ lj_form_auth: self.options.form_auth
+ },
+
+ success: function( data, status, jqxhr ) {
+ if ( data.error ) {
+ this.element.ajaxtip( "error", "Error while trying to " + this.linkdata.action + ": " + data.error );
+ } else {
+ this.element.ajaxtip( "success", data.msg );
+ this._updateLink(data);
+ }
+ self._trigger( "complete" );
}
- self._trigger( "complete" );
}
});
});
@@ -120,13 +123,13 @@ $.widget("dw.delcomment", {
journal: undefined,
form_auth: undefined,
- endpoint: "__rpc_delcomment"
+ endpoint: "delcomment"
},
_abort: function(reason, ditemid) {
ditemid = ditemid || this.linkdata.id;
- this.element.ajaxtip({namespace:"delcomment"})
- .ajaxtip( "abort", "Error deleting comment #" + ditemid + ". " + reason);
+ this.element.ajaxtip() // init
+ .ajaxtip( "error", "Error deleting comment #" + ditemid + ". " + reason );
},
_create: function() {
@@ -149,30 +152,35 @@ $.widget("dw.delcomment", {
return;
}
- var posturl = "/" + self.options.journal + "/" + self.options.endpoint
- +"?"+$.param({ mode: "js", json: 1, journal: self.options.journal, id: self.linkdata.id});
+ var endpoint = self.options.endpoint +
+ "?"+$.param({ mode: "js", json: 1, journal: self.options.journal, id: self.linkdata.id});
var postdata = { confirm: 1 };
if($("#popdel"+self.linkdata.id+"ban").is(":checked")) postdata["ban"] = 1;
if($("#popdel"+self.linkdata.id+"spam").is(":checked")) postdata["spam"] = 1;
if($("#popdel"+self.linkdata.id+"thread").is(":checked")) postdata["delthread"] = 1;
if(self.options.form_auth) postdata["lj_form_auth"] = self.options.form_auth;
- self.element
- .ajaxtip("load", {
- url: posturl,
- data: postdata,
- context: self,
- success: function( data, status, jqxhr ) {
- if ( data.error ) {
- self.element.ajaxtip( "error", "Error while trying to delete comment: " + data.error )
- } else {
- self.element.ajaxtip("success",data.msg);
- removecomment(self.linkdata.id, postdata["delthread"]);
+ self.element.ajaxtip()
+ .ajaxtip( "load", {
+ endpoint: endpoint,
+ ajax: {
+ type: "POST",
+ context: self,
+
+ data: postdata,
+
+ success: function( data, status, jqxhr ) {
+ if ( data.error ) {
+ this.element.ajaxtip( "error", "Error while trying to delete comment: " + data.error );
+ } else {
+ this.element.ajaxtip( "success", data.msg );
+ removecomment(this.linkdata.id, postdata["delthread"]);
+ }
+ self._trigger( "complete" );
}
- self._trigger( "complete" );
}
- })
+ });
}
function removecomment(ditemid,killchildren) {
@@ -209,57 +217,58 @@ $.widget("dw.delcomment", {
}
if ( e.shiftKey ) {
- self.element.ajaxtip({ namespace: "delcomment" })
deletecomment();
return;
}
- self.element
- .ajaxtip({
- namespace: "delcomment",
- content: function() {
- var canAdmin = cmtinfo["canAdmin"];
- var canSpam = cmtinfo["canSpam"];
-
- var form = $("<form class='popup-form'><fieldset><legend>Delete comment?</legend></fieldset></form>");
- var ul = $("<ul>").appendTo(form.find("fieldset"));
-
- if(remote != "" && cmtdata.u != "" && cmtdata.u != remote && canAdmin) {
- var id = "popdel"+self.linkdata.id+"ban";
- ul.append($("<li>").append(
- $("<input>", { type: "checkbox", value: "ban", id: id}),
- $("<label>", { "for": id }).html("Ban <strong>"+cmtdata.u+"</strong> from commenting")
- ));
- }
- if(remote != "" && cmtdata.u != remote && canSpam) {
- var id = "popdel"+self.linkdata.id+"spam";
- ul.append($("<li>").append(
- $("<input>", { type: "checkbox", value: "spam", id: id}),
- $("<label>", { "for": id }).text("Mark this comment as spam")
- ));
- }
+ var $deleteDialog = function() {
+ var canAdmin = cmtinfo["canAdmin"];
+ var canSpam = cmtinfo["canSpam"];
- if(cmtdata.rc && cmtdata.rc.length && canAdmin){
- var id = "popdel"+self.linkdata.id+"thread";
- ul.append($("<li>").append(
- $("<input>", { type: "checkbox", value: "thread", id: id}),
- $("<label>", { "for": id }).text("Delete thread (all subcomments)")
- ));
- }
+ var _checkbox = function(action, label) {
+ var prefix = "popdel"+self.linkdata.id;
+ return "<li><input type='checkbox' value='"+action+"' id='"+prefix+action+"'>" +
+ "<label for='"+prefix+action+"'>"+label+"</label></li>";
+ };
- ul.append($("<li>", { "class": "submit" }).append(
- $("<input>", { type: "button", value: "Delete"})
- .click(deletecomment),
+ var form = $("<form title='Delete Comment'></form>");
+ var checkboxes = [];
- $("<input>", { type: "button", value: "Cancel" })
- .click(function(){self.element.ajaxtip("cancel")}),
+ if(remote !== "" && cmtdata.u !== "" && cmtdata.u !== remote && canAdmin) {
+ checkboxes.push(_checkbox( "ban", "Ban <strong>"+cmtdata.u+"</strong> from commenting" ));
+ }
- $("<div class='note'>shift-click to delete without options</div>")
- ));
+ if(remote !== "" && cmtdata.u !== remote && canSpam) {
+ checkboxes.push(_checkbox( "spam", "Mark this comment as spam" ));
+ }
- return form;
+ if(cmtdata.rc && cmtdata.rc.length && canAdmin){
+ checkboxes.push(_checkbox( "thread", "Delete thread (all subcomments)" ));
}
+
+ $("<ul>").append(checkboxes.join("")).appendTo(form);
+ $("<p class='detail'>shift-click to delete without options</p>").appendTo(form);
+
+ return form;
+ }();
+
+ $deleteDialog.dialog({
+ position: {
+ my: "right bottom+10",
+ at: "left center",
+ of: this,
+ collision: "flipfit flipfit"
+ },
+ buttons: {
+ "Delete": function() {
+ $(this).dialog( "close" );
+ deletecomment();
+ }
+ },
+ dialogClass: "popdel",
+ maxWidth: "80%",
+ width: 500
});
});
}
View
17 htdocs/stc/jquery.commentmanage.css
@@ -0,0 +1,17 @@
+/* delete dialog */
+.popdel ul {
+ list-style: none;
+ margin: 0;
+}
+
+.popdel li {
+ margin-bottom: 0.5em;
+}
+
+.popdel .detail {
+ margin: 1.2em 0 0.3em 0;
+}
+
+.popdel .ui-dialog-buttonpane {
+ padding: 0;
+}
View
14 htdocs/stc/jquery.iconselector.css
@@ -157,33 +157,33 @@
/* UI overrides */
-.ui-dialog {
+.iconselector .ui-dialog {
padding:0;
}
-.ui-dialog .ui-dialog-content {
+.iconselector .ui-dialog .ui-dialog-content {
padding:.5em;
}
-.ui-widget-content {
+.iconselector .ui-widget-content {
border-width:1px;
border-style:solid;
}
-.ui-widget-header {
+.iconselector .ui-widget-header {
border:0;
}
-.ui-dialog .ui-dialog-titlebar {
+.iconselector .ui-dialog .ui-dialog-titlebar {
padding:.3em;
}
-.ui-dialog .ui-dialog-buttonpane {
+.iconselector .ui-dialog .ui-dialog-buttonpane {
padding:.3em 0;
text-align:center;
margin:0;
}
-.ui-dialog .ui-dialog-buttonpane button {
+.iconselector .ui-dialog .ui-dialog-buttonpane button {
margin:0;
padding:0;
float:none;
View
7 htdocs/talkread.bml
@@ -844,14 +844,17 @@ body<=
if ($do_commentmanage_js) {
LJ::need_res('js/commentmanage.js');
LJ::need_res( { group => "jquery" }, qw(
- js/jquery/jquery.ui.widget.js
js/jquery/jquery.ui.tooltip.js
js/jquery.ajaxtip.js
+ js/jquery/jquery.ui.button.js
+ js/jquery/jquery.ui.dialog.js
js/jquery.commentmanage.js
js/jquery/jquery.ui.position.js
stc/jquery/jquery.ui.tooltip.css
+ stc/jquery/jquery.ui.button.css
+ stc/jquery/jquery.ui.dialog.css
stc/ajaxtip.css
- stc/popup-form.css
+ stc/jquery.commentmanage.css
) );
my $js_screen_color = "\"" . LJ::ejs(BML::get_template_def("screenedbarcolor") || BML::get_template_def("emcolor")) . "\"";
my $js_normal_color = "\"" . LJ::ejs(BML::get_template_def("emcolor")) . "\"";
View
91 views/dev/tests/commentmanage.js
@@ -4,6 +4,8 @@ old: js/6alib/commentmanage.js
jquery: js/jquery/jquery.ui.widget.js
jquery: js/jquery.ajaxtip.js
jquery: js/jquery.commentmanage.js
+jquery: js/jquery/jquery.ui.core.js
+jquery: js/jquery/jquery.ui.dialog.js
jquery: js/jquery/jquery.ui.tooltip.js
jquery: js/jquery/jquery.ui.position.js
*/
@@ -178,7 +180,7 @@ function _check_link(linkid, oldstate, newstate) {
equal($link.attr("href"), newstate.url, linkid + " - new url" );
equal($link.text(), newstate.text, linkid + " - new text" );
- equals($link.ajaxtip("widget").html(), newstate.msg, linkid + " - did action");
+ equals($link.ajaxtip("option", "content"), newstate.msg, linkid + " - did action");
})
.trigger("click");
this.server.respond();
@@ -188,7 +190,7 @@ function _check_link(linkid, oldstate, newstate) {
equal($link.attr("href"), oldstate.url, linkid + " - changed back to old url");
equal($link.text(), oldstate.text, linkid + " - changed back to old text");
- equals($link.ajaxtip("widget").html(), oldstate.msg, linkid + " - did action");
+ equals($link.ajaxtip("option", "content"), oldstate.msg, linkid + " - did action");
})
.trigger("click");
this.server.respond();
@@ -209,7 +211,7 @@ function _check_link_with_image(linkid, oldstate, newstate) {
equal($img.attr("alt"), newstate.text, linkid + " - new alt" );
equal($img.attr("title"), newstate.text, linkid + " - new title" );
- equals($link.ajaxtip("widget").html(), newstate.msg, linkid + " - did action");
+ equals($link.ajaxtip("option", "content"), newstate.msg, linkid + " - did action");
})
.trigger("click");
this.server.respond();
@@ -220,7 +222,7 @@ function _check_link_with_image(linkid, oldstate, newstate) {
equal($img.attr("alt"), oldstate.text, linkid + " - changed back to old alt");
equal($img.attr("title"), oldstate.text, linkid + " - changed back to old title" );
- equals($link.ajaxtip("widget").html(), oldstate.msg, linkid + " - did action");
+ equals($link.ajaxtip("option", "content"), oldstate.msg, linkid + " - did action");
})
.trigger("click");
this.server.respond();
@@ -279,13 +281,14 @@ test( "delete all children (has children)", 4, function() {
ok( ! parent.is(":visible"), "Parent comment successfully hidden after delete" );
ok( ! child.is(":visible"), "Child comment successfully hidden after delete" );
})
- .trigger("click")
- .ajaxtip("widget")
- .find("input[value='thread']")
- .attr("checked", "checked")
- .end()
- .find("input[value='Delete']")
- .click()
+ .trigger("click");
+
+ $(".popdel")
+ .find("input[value='thread']")
+ .attr("checked", "checked")
+ .end()
+ .find("button")
+ .click();
this.server.respond();
} );
@@ -306,13 +309,15 @@ test( "delete all children (has no children)", 4, function() {
ok( parent.is(":visible"), "Parent comment not deleted" );
ok( ! child.is(":visible"), "Child comment successfully hidden after delete" );
})
- .trigger("click")
- .ajaxtip("widget")
- .find("input[value='thread']")
- .attr("checked", "checked")
- .end()
- .find("input[value='Delete']")
- .click();
+ .trigger("click");
+
+ $(".popdel")
+ .find("input[value='thread']")
+ .attr("checked", "checked")
+ .end()
+ .find("button")
+ .click();
+
this.server.respond();
} );
@@ -332,10 +337,11 @@ test( "delete no children (has children)", 4, function() {
ok( ! parent.is(":visible"), "Parent comment successfully hidden after delete" );
ok( child.is(":visible"), "Child comment not deleted, still visible" );
})
- .trigger("click")
- .ajaxtip("widget")
- .find("input[value='Delete']")
- .click();
+ .trigger("click");
+
+ $(".popdel")
+ .find("button")
+ .click();
this.server.respond();
} );
@@ -356,10 +362,11 @@ test( "delete no children (has no children)", 4, function() {
ok( parent.is(":visible"), "Parent comment not deleted, still visible" );
ok( ! child.is(":visible"), "Child comment successfully hidden after delete" );
})
- .trigger("click")
- .ajaxtip("widget")
- .find("input[value='Delete']")
- .click();
+ .trigger("click");
+
+ $(".popdel")
+ .find("button")
+ .click();
this.server.respond();
} );
@@ -382,10 +389,12 @@ test( "failed delete: no hiding", 4, function() {
ok( parent.is(":visible"), "Parent comment not deleted, still visible" );
ok( child.is(":visible"), "Child comment not deleted, still visible" );
})
- .trigger("click")
- .ajaxtip("widget")
- .find("input[value='Delete']")
- .click();
+ .trigger("click");
+
+ $(".popdel")
+ .find("button")
+ .click();
+
this.server.respond();
@@ -398,7 +407,7 @@ test( "invalid moderate link", 1, function() {
.trigger("click")
this.server.respond()
- equals($("#invalid_moderate_link").ajaxtip("widget").text(),
+ equals($("#invalid_moderate_link").ajaxtip("option","content"),
"Error moderating comment #. Not enough context available.");
});
@@ -408,7 +417,7 @@ test( "invalid delete link", 1, function() {
.trigger({ type: "click", shiftKey: true })
this.server.respond();
- equals($("#invalid_delete_link").ajaxtip("widget").text(),
+ equals($("#invalid_delete_link").ajaxtip("option","content"),
"Error deleting comment #. Comment is not visible on this page.");
} );
@@ -419,7 +428,7 @@ test( "no such comment for moderation", 1, function() {
.trigger("click")
this.server.respond();
- equals($("#mismatched_moderate_link").ajaxtip("widget").text(),
+ equals($("#mismatched_moderate_link").ajaxtip("option","content"),
"Error moderating comment #999. Cannot moderate comment which is not visible on this page.")
} );
@@ -429,7 +438,7 @@ test( "mismatched journal for deletion", 1, function() {
.trigger({ type: "click", shiftKey: true })
this.server.respond();
- equals($("#mismatched_journal_delete_link").ajaxtip("widget").text(),
+ equals($("#mismatched_journal_delete_link").ajaxtip("option","content"),
"Error deleting comment #123. Journal in link does not match expected journal.")
} );
@@ -439,7 +448,7 @@ test( "no such comment for moderation", 1, function() {
.trigger("click")
this.server.respond();
- equals($("#mismatched_journal_moderate_link").ajaxtip("widget").text(),
+ equals($("#mismatched_journal_moderate_link").ajaxtip("option","content"),
"Error moderating comment #123. Journal in link does not match expected journal.")
} );
@@ -449,7 +458,7 @@ test( "no such comment for deletion", 1, function() {
.trigger({ type: "click", shiftKey: true })
this.server.respond();
- equals($("#mismatched_delete_link").ajaxtip("widget").text(),
+ equals($("#mismatched_delete_link").ajaxtip("option","content"),
"Error deleting comment #999. Comment is not visible on this page.")
} );
@@ -460,7 +469,7 @@ test( "lacking arguments for moderate: form_auth", 1, function() {
.trigger("click");
this.server.respond();
- equals($("#freeze_link").ajaxtip("widget").text(),
+ equals($("#freeze_link").ajaxtip("option","content"),
"Error moderating comment #123. Not enough context available.")
} );
@@ -471,7 +480,7 @@ test( "lacking arguments for moderate: journal", 1, function() {
.trigger("click");
this.server.respond();
- equals($("#freeze_link").ajaxtip("widget").text(),
+ equals($("#freeze_link").ajaxtip("option","content"),
"Error moderating comment #123. Not enough context available.")
} );
@@ -482,7 +491,7 @@ test( "lacking arguments for delete: cmtinfo", 1, function() {
.trigger({ type: "click", shiftKey: true })
this.server.respond();
- equals($("#delete_link").ajaxtip("widget").text(),
+ equals($("#delete_link").ajaxtip("option","content"),
"Error deleting comment #123. Not enough context available.")
} );
@@ -493,7 +502,7 @@ test( "lacking arguments for delete: journal", 1, function() {
.trigger({ type: "click", shiftKey: true })
this.server.respond();
- equals($("#delete_link").ajaxtip("widget").text(),
+ equals($("#delete_link").ajaxtip("option","content"),
"Error deleting comment #123. Not enough context available.")
} );
@@ -504,7 +513,7 @@ test( "lacking arguments for delete: form_auth", 1, function() {
.trigger({ type: "click", shiftKey: true })
this.server.respond();
- equals($("#delete_link").ajaxtip("widget").text(),
+ equals($("#delete_link").ajaxtip("option","content"),
"Error deleting comment #123. Not enough context available.")
} );

0 comments on commit 1a99e7c

Please sign in to comment.