Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CLDR-17516 Revise flag interface #3905

Merged
merged 3 commits into from
Jul 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading