From 4f630b13b5cb100a617d1742a327b1f84d36d796 Mon Sep 17 00:00:00 2001 From: Tom Bishop Date: Wed, 31 Jul 2024 09:56:04 -0400 Subject: [PATCH] CLDR-17516 Revise flag interface (#3905) --- tools/cldr-apps/js/src/esm/cldrForum.mjs | 70 +++++++++++++------ tools/cldr-apps/js/src/esm/cldrInfo.mjs | 32 ++------- tools/cldr-apps/js/src/esm/cldrTable.mjs | 7 +- tools/cldr-apps/js/src/esm/cldrText.mjs | 7 +- .../cldr-apps/src/main/webapp/surveytool.css | 26 +------ 5 files changed, 63 insertions(+), 79 deletions(-) diff --git a/tools/cldr-apps/js/src/esm/cldrForum.mjs b/tools/cldr-apps/js/src/esm/cldrForum.mjs index 9ad0b77eeef..f3fff54a552 100644 --- a/tools/cldr-apps/js/src/esm/cldrForum.mjs +++ b/tools/cldr-apps/js/src/esm/cldrForum.mjs @@ -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); @@ -235,7 +236,8 @@ function openPostOrReply(params) { replyTo, root, open, - value + value, + willFlag ); const text = prefillPostText(postType, value); @@ -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( @@ -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 += '
' + subject + "
\n"; - html += "
" + cldrText.get("forum_remember_vote") + "
\n"; + html += "
" + cldrText.get(reminder) + "
\n"; html += '
' + typeLabel + "
\n"; html += '
\n'; html += '
\n'; @@ -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/ @@ -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(), @@ -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( @@ -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"; @@ -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({ @@ -907,6 +934,7 @@ function makeOneNewPostButton( subject: subj, postType: postType, value: value, + willFlag: willFlag, }); } ); diff --git a/tools/cldr-apps/js/src/esm/cldrInfo.mjs b/tools/cldr-apps/js/src/esm/cldrInfo.mjs index e1ffaf4bfb0..7f9012f9aa4 100644 --- a/tools/cldr-apps/js/src/esm/cldrInfo.mjs +++ b/tools/cldr-apps/js/src/esm/cldrInfo.mjs @@ -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, ...]. diff --git a/tools/cldr-apps/js/src/esm/cldrTable.mjs b/tools/cldr-apps/js/src/esm/cldrTable.mjs index fd6d59171ce..178b28a9cd6 100644 --- a/tools/cldr-apps/js/src/esm/cldrTable.mjs +++ b/tools/cldr-apps/js/src/esm/cldrTable.mjs @@ -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; @@ -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"); } } diff --git a/tools/cldr-apps/js/src/esm/cldrText.mjs b/tools/cldr-apps/js/src/esm/cldrText.mjs index d31e0d2be7f..1d00a5eed67 100644 --- a/tools/cldr-apps/js/src/esm/cldrText.mjs +++ b/tools/cldr-apps/js/src/esm/cldrText.mjs @@ -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.", diff --git a/tools/cldr-apps/src/main/webapp/surveytool.css b/tools/cldr-apps/src/main/webapp/surveytool.css index b5155254f95..efb70086474 100644 --- a/tools/cldr-apps/src/main/webapp/surveytool.css +++ b/tools/cldr-apps/src/main/webapp/surveytool.css @@ -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 { @@ -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; @@ -2685,12 +2674,6 @@ 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; @@ -2698,11 +2681,6 @@ transition: background-color 0.2s; 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; }