Skip to content

Commit

Permalink
CLDR-17759 Highlight cell when vote for a candidate item
Browse files Browse the repository at this point in the history
-Previously the cell did not get highlighted unless the Info Panel was open

-New function setDivClassSelected calls setLastShown to add thick border after vote

-Refactor: new function setDivClassSelected combines updateCurrentId and setLastShown

-Refactor: move setDivClass from cldrVote to cldrTable

-Refactor: move parentOfType from cldrTable to cldrDom

-Comments
  • Loading branch information
btangmu committed Jul 10, 2024
1 parent 265767d commit 12aac2e
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 42 deletions.
11 changes: 11 additions & 0 deletions tools/cldr-apps/js/src/esm/cldrDom.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,16 @@ function appendIcon(toElement, className, title) {
return e;
}

function parentOfType(tag, obj) {
if (!obj) {
return null;
}
if (obj.nodeName === tag) {
return obj;
}
return parentOfType(tag, obj.parentElement);
}

export {
addClass,
appendIcon,
Expand All @@ -235,6 +245,7 @@ export {
createChunk,
createLinkToFn,
listenFor,
parentOfType,
removeAllChildNodes,
removeClass,
setDisplayed,
Expand Down
9 changes: 2 additions & 7 deletions tools/cldr-apps/js/src/esm/cldrInfo.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ function panelShouldBeShown() {
* @param {String} str the string to show at the top
* @param {Node} tr the <TR> of the row
* @param {Object} hideIfLast mysterious parameter, a.k.a. theObj
* @param {Function} fn the draw function, a.k.a. showFn, sometimes constructed by cldrInfo.showItemInfoFn,
* @param {Function} fn the draw function, a.k.a. showFn, sometimes constructed by cldrTable.showItemInfoFn,
* sometimes ourShowFn in cldrVote.showProposedItem
*/
function show(str, tr, hideIfLast, fn) {
Expand All @@ -212,12 +212,7 @@ function show(str, tr, hideIfLast, fn) {
unShow();
unShow = null;
}
// Ideally, updateCurrentId and setLastShown should be called from cldrTable, not cldrInfo,
// however it's not clear whether they need to be called after openPanel and unShow
if (tr?.sethash) {
cldrLoad.updateCurrentId(tr.sethash);
}
cldrTable.setLastShown(hideIfLast);
cldrTable.updateSelectedRowAndCell(tr, hideIfLast);
addDeferredHelp(tr?.theRow); // if !tr.theRow, erase (as when click Next/Previous)
addPlaceholderHelp(tr?.theRow); // ditto
addInfoMessage(str);
Expand Down
55 changes: 34 additions & 21 deletions tools/cldr-apps/js/src/esm/cldrTable.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ const NO_WINNING_VALUE = "no-winning-value";
*/
const EMPTY_ELEMENT_VALUE = "❮EMPTY❯";

/**
* Remember the element (HTMLTableCellElement or HTMLDivElement) that was most recently
* shown as "selected" (displayed with a thick border). When a new element gets selected,
* this is used for removing the border from the old one
*/
let lastShown = null;

/**
Expand Down Expand Up @@ -1013,7 +1018,7 @@ function addVitem(td, tr, theRow, item, newButton) {
const div = document.createElement("div");
const isWinner = td == tr.proposedcell;
const testKind = cldrVote.getTestKind(item.tests);
cldrVote.setDivClass(div, testKind);
setDivClass(div, testKind);
item.div = div; // back link

const choiceField = document.createElement("div");
Expand Down Expand Up @@ -1089,6 +1094,21 @@ function addVitem(td, tr, theRow, item, newButton) {
}
}

function setDivClassSelected(div, testKind) {
setDivClass(div, testKind);
setLastShown(div); // add thick border and remove it from previous selected element
}

function setDivClass(div, testKind) {
if (testKind == "Warning") {
div.className = "d-item-warn";
} else if (testKind == "Error") {
div.className = "d-item-err";
} else {
div.className = "d-item";
}
}

/**
* Return a function that will set theRow.selectedItem, which will result in
* showing info for the given candidate item in the Info Panel.
Expand Down Expand Up @@ -1369,31 +1389,32 @@ function listen(str, tr, theObj, fn) {
cldrDom.listenFor(theObj, "click", function (e) {
if (cldrInfo.panelShouldBeShown()) {
cldrInfo.show(str, tr, theObj /* hideIfLast */, fn);
} else if (tr?.sethash) {
// These methods, updateCurrentId and setLastShown may be called from cldrInfo.show, if
// panelShouldBeShown() returned true. If the Info Panel is hidden then they still
// need to be called. Since they don't involve the Info Panel, the implementation
// should be changed to make them independent of the Info Panel and they should be
// called from a different module, not cldrInfo.
cldrLoad.updateCurrentId(tr.sethash);
setLastShown(theObj);
} else {
updateSelectedRowAndCell(tr, theObj);
}
cldrEvent.stopPropagation(e);
return false;
});
}

function updateSelectedRowAndCell(tr, obj) {
if (tr?.sethash) {
cldrLoad.updateCurrentId(tr.sethash);
}
setLastShown(obj);
}

function setLastShown(obj) {
if (lastShown && obj != lastShown) {
cldrDom.removeClass(lastShown, "pu-select");
const partr = parentOfType("TR", lastShown);
const partr = cldrDom.parentOfType("TR", lastShown);
if (partr) {
cldrDom.removeClass(partr, "selectShow");
}
}
if (obj) {
cldrDom.addClass(obj, "pu-select");
const partr = parentOfType("TR", obj);
const partr = cldrDom.parentOfType("TR", obj);
if (partr) {
cldrDom.addClass(partr, "selectShow");
}
Expand All @@ -1405,16 +1426,6 @@ function resetLastShown() {
lastShown = null;
}

function parentOfType(tag, obj) {
if (!obj) {
return null;
}
if (obj.nodeName === tag) {
return obj;
}
return parentOfType(tag, obj.parentElement);
}

export {
NO_WINNING_VALUE,
EMPTY_ELEMENT_VALUE,
Expand All @@ -1429,8 +1440,10 @@ export {
makeRowId,
refreshSingleRow,
resetLastShown,
setDivClassSelected,
setLastShown,
updateRow,
updateSelectedRowAndCell,
/*
* The following are meant to be accessible for unit testing only:
*/
Expand Down
15 changes: 1 addition & 14 deletions tools/cldr-apps/js/src/esm/cldrVote.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ function showProposedItem(inTd, tr, theRow, value, tests, json) {
cldrNotify.error("Not submitted", description);
return;
} else {
setDivClass(ourDiv, testKind);
cldrTable.setDivClassSelected(ourDiv, testKind);
}

if (testKind || !ourItem) {
Expand Down Expand Up @@ -430,18 +430,6 @@ function showProposedItem(inTd, tr, theRow, value, tests, json) {
return false;
}

function setDivClass(div, testKind) {
if (!testKind) {
div.className = "d-item";
} else if (testKind == "Warning") {
div.className = "d-item-warn";
} else if (testKind == "Error") {
div.className = "d-item-err";
} else {
div.className = "d-item";
}
}

/**
* Determine whether a JSONified array of CheckCLDR.CheckStatus is overall a warning or an error.
*
Expand Down Expand Up @@ -607,7 +595,6 @@ export {
getTestKind,
handleWiredClick,
isBusy,
setDivClass,
setVoteLevelChanged,
wireUpButton,
wrapRadio,
Expand Down

0 comments on commit 12aac2e

Please sign in to comment.