Skip to content

Commit

Permalink
CLDR-17516 Revise flag interface (#3905)
Browse files Browse the repository at this point in the history
  • Loading branch information
btangmu authored Jul 31, 2024
1 parent 5ab2653 commit 4f630b1
Show file tree
Hide file tree
Showing 5 changed files with 63 additions and 79 deletions.
70 changes: 49 additions & 21 deletions tools/cldr-apps/js/src/esm/cldrForum.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ function openPostOrReply(params) {
: rootPost && rootPost.value
? rootPost.value
: null;
const willFlag = params?.willFlag || false;
const root = isReply ? rootPost.id : -1;
const open = isReply ? rootPost.open : true;
const typeLabel = makePostTypeLabel(postType, isReply);
Expand All @@ -235,7 +236,8 @@ function openPostOrReply(params) {
replyTo,
root,
open,
value
value,
willFlag
);
const text = prefillPostText(postType, value);

Expand All @@ -254,6 +256,7 @@ function openPostOrReply(params) {
* @param root the post id of the original post in the thread, or -1
* @param open true or false, is this thread open
* @param value the value that was requested in the root post, or null
* @param willFlag {Boolean} if true, this post will cause the item to be flagged
* @return the html
*/
function makePostHtml(
Expand All @@ -265,12 +268,14 @@ function makePostHtml(
replyTo,
root,
open,
value
value,
willFlag
) {
const reminder = willFlag ? "flag_must_have_reason" : "forum_remember_vote";
let html = "";

html += '<div id="postSubject" class="topicSubject">' + subject + "</div>\n";
html += "<div>" + cldrText.get("forum_remember_vote") + "</div>\n";
html += "<div>" + cldrText.get(reminder) + "</div>\n";
html += '<div class="postTypeLabel">' + typeLabel + "</div>\n";
html += '<form role="form" id="post-form">\n';
html += '<div class="form-group">\n';
Expand Down Expand Up @@ -334,6 +339,7 @@ function prefillPostText(postType, value) {
* Open a window displaying the form for creating a post
*
* @param html the main html for the form
* @param text the pre-filled user-editable text for the form
* @param parentPost the post object, if any, to which this is a reply, for display at the bottom of the window
*
* Reference: Bootstrap.js post-modal: https://getbootstrap.com/docs/4.1/components/modal/
Expand Down Expand Up @@ -363,29 +369,43 @@ function openPostWindow(html, text, parentPost) {
* @param event
*/
function submitPost(event) {
const text = $("#post-form textarea[name=text]").val();
if (text) {
reallySubmitPost(text);
}
event.preventDefault();
event.stopPropagation();
const form = getFormValues();
if (formIsAcceptable(form)) {
$("#post-form button").fadeOut();
cldrForumPanel.clearCache();
sendPostRequest(form);
} else {
// Call the user's attention to the bogus text area by winking it
$("#post-form textarea").fadeTo(1000, 0).fadeTo(1000, 1);
}
}

/**
* Submit a forum post
* Is the given form data acceptable?
*
* @param text the non-empty body of the message
* @param {Object} form
* @returns {Boolean} true if acceptable
*/
function reallySubmitPost(text) {
$("#post-form button").fadeOut();
cldrForumPanel.clearCache();
const form = getFormValues(text);
sendPostRequest(form);
function formIsAcceptable(form) {
if (!form.text.trim()) {
// the text field is empty or all whitespace
return false;
}
if (form.postType === cldrForumType.REQUEST) {
const prefill = cldrText.sub("forum_prefill_request", [form.value]);
if (form.text.trim() === prefill.trim()) {
// the text field for a Request matches the pre-fill
return false;
}
}
return true;
}

function getFormValues(text) {
function getFormValues() {
return {
text: text,
text: $("#post-form textarea[name=text]").val(),
locale: $("#post-form input[name=_]").val(),
open: $("#post-form input[name=open]").val(),
postType: $("#post-form input[name=postType]").val(),
Expand Down Expand Up @@ -827,7 +847,15 @@ function addNewPostButtons(el, locale, couldFlag, xpstrid, code, value) {
null /* rootPost */,
value
);

// Only an enabled REQUEST button can really cause a path to be flagged.
const reallyCanFlag =
couldFlag && value && Object.keys(options).includes(cldrForumType.REQUEST);
if (reallyCanFlag) {
const message = cldrText.get("mustflag_explain_msg");
cldrSurvey.addIcon(el, "i-stop");
el.appendChild(cldrDom.createChunk(message, "span", ""));
el.appendChild(document.createElement("p"));
}
Object.keys(options).forEach(function (postType) {
el.appendChild(
makeOneNewPostButton(
Expand Down Expand Up @@ -877,9 +905,8 @@ function makeOneNewPostButton(
// REQUEST is only enabled if there is a non-null value (which the user voted for).
const disabled = postType === cldrForumType.REQUEST && value === null;
// Only an enabled REQUEST button can really cause a path to be flagged.
const reallyCanFlag =
couldFlag && postType === cldrForumType.REQUEST && !disabled;
const buttonClass = reallyCanFlag
const willFlag = couldFlag && postType === cldrForumType.REQUEST && !disabled;
const buttonClass = willFlag
? "addPostButton forumNewPostFlagButton btn btn-default btn-sm"
: "addPostButton forumNewButton btn btn-default btn-sm";

Expand All @@ -898,7 +925,7 @@ function makeOneNewPostButton(
if (o.result && o.result.ph) {
subj = xpathMap.formatPathHeader(o.result.ph);
}
if (reallyCanFlag) {
if (willFlag) {
subj += " (Flag for review)";
}
openPostOrReply({
Expand All @@ -907,6 +934,7 @@ function makeOneNewPostButton(
subject: subj,
postType: postType,
value: value,
willFlag: willFlag,
});
}
);
Expand Down
32 changes: 6 additions & 26 deletions tools/cldr-apps/js/src/esm/cldrInfo.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -537,34 +537,14 @@ function updateRowVoteInfo(tr, theRow) {
)
);
}
if (theRow.voteVhash !== theRow.winningVhash && theRow.canFlagOnLosing) {
if (!theRow.rowFlagged) {
cldrSurvey.addIcon(tr.voteDiv, "i-stop");
tr.voteDiv.appendChild(
cldrDom.createChunk(
cldrText.sub("mustflag_explain_msg", {}),
"p",
"helpContent"
)
);
} else {
cldrSurvey.addIcon(tr.voteDiv, "i-flag");
tr.voteDiv.appendChild(
cldrDom.createChunk(cldrText.get("flag_desc", "p", "helpContent"))
);
}
if (theRow.rowFlagged) {
cldrSurvey.addIcon(tr.voteDiv, "i-flag");
tr.voteDiv.appendChild(
cldrDom.createChunk(cldrText.get("flag_desc", "p", "helpContent"))
);
}
}
if (!theRow.rowFlagged && theRow.canFlagOnLosing) {
// TODO: display this message and the actual "Flag for Review" button in the same place; see forumNewPostFlagButton.
// Change to: ⚐ [for committee approval|Ask]
// and don't show unless it can be, of course.
// Reference: https://unicode-org.atlassian.net/browse/CLDR-7536
cldrSurvey.addIcon(tr.voteDiv, "i-flag-d");
tr.voteDiv.appendChild(
cldrDom.createChunk(cldrText.get("flag_d_desc", "p", "helpContent"))
);
}

/*
* The value_vote array has an even number of elements,
* like [value0, vote0, value1, vote1, value2, vote2, ...].
Expand Down
7 changes: 3 additions & 4 deletions tools/cldr-apps/js/src/esm/cldrTable.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -811,9 +811,6 @@ function updateRowProposedWinningCell(tr, theRow, cell, protoButton) {
if (theRow.rowFlagged) {
const flagIcon = cldrSurvey.addIcon(cell, "s-flag");
flagIcon.title = cldrText.get("flag_desc");
} else if (theRow.canFlagOnLosing) {
const flagIcon = cldrSurvey.addIcon(cell, "s-flag-d");
flagIcon.title = cldrText.get("flag_d_desc");
}
cldrSurvey.setLang(cell);
tr.proposedcell = cell;
Expand Down Expand Up @@ -1050,7 +1047,9 @@ function addVitem(td, tr, theRow, item, newButton) {
theRow.canFlagOnLosing &&
!theRow.rowFlagged
) {
cldrSurvey.addIcon(choiceField, "i-stop"); // DEBUG
const stopIcon = cldrSurvey.addIcon(choiceField, "i-stop");
stopIcon.setAttribute("dir", "ltr");
stopIcon.title = cldrText.get("mustflag_explain_msg");
}
}

Expand Down
7 changes: 3 additions & 4 deletions tools/cldr-apps/js/src/esm/cldrText.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -178,13 +178,12 @@ const strings = {

override_explain_msg:
"You have voted for this item with ${overrideVotes} votes instead of the usual ${votes}",
voteInfo_overrideExplain_desc: "",
mustflag_explain_msg:
"The item you voted for is not winning. However, you may post a forum entry to flag the item for Committee review.",
voteInfo_mustflag_explain_desc: "",
"The item you voted for requires TC approval to win. If you post a forum request, the item will be flagged for review by the CLDR Technical Committee.",
flag_must_have_reason:
"‼️ Your request will cause this item to be flagged for review by the CLDR Technical Committee. Your message MUST provide justification, e.g., examples of common usage‼️",
flag_desc:
"This item has been flagged for review by the CLDR Technical Committee.",
flag_d_desc: "Losing items may be flagged for CLDR Committee review.",
explainRequiredVotes: "Changes to this item require ${requiredVotes} votes.",
valueIsLocked:
"This item has been locked by the CLDR Technical Committee. See the forum entry.",
Expand Down
26 changes: 2 additions & 24 deletions tools/cldr-apps/src/main/webapp/surveytool.css
Original file line number Diff line number Diff line change
Expand Up @@ -1286,17 +1286,14 @@ div.forumDiv div.subpost .postHeader {
display: table-cell;
}

button.forumNewButton {

}

button.forumNewPostFlagButton {
padding-left: 2em;
background: url(flag-d.png) no-repeat left center;
}

button.forumNewPostFlagButton:hover {
padding-left: 2em;
background: url(flag.png) no-repeat;
background: url(flag.png) no-repeat left center;
}

div.forumDiv .adminUserUser {
Expand Down Expand Up @@ -2256,14 +2253,6 @@ h2.v-oldvotes-sub {
border-top: 2px solid silver;
}

/*
Unknown status
*/

tr.tr_unknown {

}

div.helpHtml, .helpContent {
background: #eed;
font: medium Georgia, "Times New Roman", Times, serif;
Expand Down Expand Up @@ -2685,24 +2674,13 @@ transition: background-color 0.2s;
height: 17px;
background: url(flag.png) no-repeat center center;
}
.i-flag-d {
padding: 2px;
width: 17px;
height: 17px;
background: url(flag-d.png) no-repeat center center;
}

.s-flag {
width: 17px;
height: 30px;
background: url(flag.png) no-repeat center center ;
}

.s-flag-d {
width: 17px;
height: 30px;
background: url(flag-d.png) no-repeat center center ;
}
td.d-change,td.d-change-err,td.d-change-warn {
white-space: nowrap;
}
Expand Down

0 comments on commit 4f630b1

Please sign in to comment.