Skip to content

Commit

Permalink
CLDR-17658 Improve Dashboard performance and UI; work in progress
Browse files Browse the repository at this point in the history
-For now, change only the front end, except for enabling Abstain for TC

-Abstain for TC is hidden (unchecked) by default

-Combine notifications in certain categories so that there is not more than one per page

-For now, apply this to both Missing and Abstained categories; easy to change which categories are affected

-Try to fix delayed visibility of checkbox click; alternatives and debugging for catCheckmarkChanged

-Remove delay from a-spin pending solution to delayed visibility of checkbox click

-Add Vue version to the About page; bugs may depend on Vue version
  • Loading branch information
btangmu committed May 28, 2024
1 parent 582b053 commit b2a988c
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 16 deletions.
42 changes: 29 additions & 13 deletions tools/cldr-apps/js/src/esm/cldrDash.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@ class DashData {
// An object whose keys are xpstrid (xpath hex IDs like "db7b4f2df0427e4"), and whose values are DashEntry objects
this.pathIndex = {};
this.hiddenObject = null;
this.pageCombinedCount = {}; // map page to number of notifications on that page
this.pageCombinedEntry = {}; // map page to representative notification entry xpstrid on that page
this.pageCombinedEntries = {}; // map page to array of notification xpstrid on that page
}

addEntriesFromJson(notifications) {
Expand Down Expand Up @@ -73,19 +72,20 @@ class DashData {
addCombinedEntry(cat, group, e) {
try {
const page = group.page;
if (!this.pageCombinedCount[cat]) {
this.pageCombinedCount[cat] = {};
this.pageCombinedEntry[cat] = {};
if (!this.pageCombinedEntries[cat]) {
this.pageCombinedEntries[cat] = {};
}
if (!this.pageCombinedCount[cat][page]) {
this.pageCombinedCount[cat][page] = 1;
this.pageCombinedEntry[cat][page] = this.addNewEntry(cat, group, e);
if (!this.pageCombinedEntries[cat][page]?.length) {
this.pageCombinedEntries[cat][page] = new Array(e.xpstrid);
this.addNewEntry(cat, group, e);
} else {
this.pageCombinedCount[cat][page]++;
const xpstrid = this.pageCombinedEntry[cat][page]; // not necessarily e.xpstrid
this.pageCombinedEntries[cat][page].push(e.xpstrid);
// Use the FIRST item in the array as the representative,
// with its comment indicating the size of the array
const xpstrid = this.pageCombinedEntries[cat][page][0];
if (!xpstrid) {
console.error(
"Existing xpstrid not found in addCombinedEntry for cat = " +
"Existing xpstrid not found in addCombinedEntries for cat = " +
cat +
", page = " +
page
Expand Down Expand Up @@ -139,7 +139,7 @@ class DashData {
"Total " +
cat +
" entries on this page: " +
this.pageCombinedCount[cat][page]
this.pageCombinedEntries[cat][page].length
);
} else {
dashEntry.setComment(comment);
Expand Down Expand Up @@ -235,12 +235,28 @@ class DashData {
this.cats.delete(cat);
delete this.catSize[cat];
}
if (this.catFirst[cat] === dashEntry.xpstrid) {
if (CATS_ONE_PER_PAGE.includes(cat)) {
this.removeCombinedEntryCats(dashEntry, cat);
} else if (this.catFirst[cat] === dashEntry.xpstrid) {
this.findCatFirst(cat);
}
});
}

removeCombinedEntryCats(dashEntry, cat) {
const page = dashEntry.page;
this.pageCombinedEntries[cat][page].shift();
if (this.pageCombinedEntries[cat][page].length > 0) {
if (this.catFirst[cat] === dashEntry.xpstrid) {
const nextXpstrid = this.pageCombinedEntries[cat][page][0];
dashEntry.xpstrid = nextXpstrid;
delete this.pathIndex.xpstrid;
this.pathIndex[nextXpstrid] = dashEntry;
this.catFirst[cat] = nextXpstrid;
}
}
}

findCatFirst(cat) {
for (let dashEntry of this.entries) {
if (dashEntry.cat === cat) {
Expand Down
5 changes: 5 additions & 0 deletions tools/cldr-apps/js/src/views/DashboardWidget.vue
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,11 @@ export default {
},
created() {
if (cldrStatus.getPermissions()?.userIsTC) {
this.catIsHidden["Abstained"] = this.catCheckboxIsUnchecked[
"Abstained"
] = true;
}
this.fetchData();
},
Expand Down
59 changes: 59 additions & 0 deletions tools/cldr-apps/js/test/TestCldrDash.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,65 @@ describe("cldrDash.updatePath", function () {
1
);
});

it("should update combined entries", function () {
// jsonAbstain0 has 2 Abstained notifications on the same page.
// After the user votes for the first of those rows (which "represents" the
// Abstain category on that page), there should still be a combined notification
// for the page, with the remaining row as representative
const jsonTwoAbstained = {
hidden: {},
notifications: [
{
category: "Abstained",
groups: [
{
entries: [
{
code: "codeA",
comment: "",
english: "a",
old: "",
subtype: "",
winning: "a",
xpstrid: "1234",
},
{
code: "codeB",
comment: "",
english: "b",
old: "",
subtype: "",
winning: "b",
xpstrid: "abcd",
},
],
header: "pint-metric",
page: "Volume",
section: "Units",
},
],
},
],
};

const jsonUpdate = {
xpstrid: "1234",
page: {
rows: {
_xctnmb: {
xpstrid: "1234",
},
},
},
notifications: [],
};

const dataBefore = cldrDash.setData(jsonTwoAbstained);
const dataAfter = cldrDash.updatePath(dataBefore, jsonUpdate);
assert.strictEqual(dataAfter.catSize["Abstained"], 1);
assert.strictEqual(dataAfter.catFirst["Abstained"], "abcd");
});
});

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -239,9 +239,6 @@ public ReviewOutput get(
sm.getSupplementalDataInfo(), sourceFactory, new STUsersChoice(sm));
EnumSet<NotificationCategory> choiceSet =
VettingViewer.getDashboardNotificationCategories(usersOrg);
if (UserRegistry.userIsTC(user)) {
choiceSet.remove(NotificationCategory.abstained);
}
VettingParameters args = new VettingParameters(choiceSet, locale, coverageLevel);
args.setUserAndOrganization(user.id, usersOrg);
args.setFiles(locale, sourceFactory, sm.getDiskFactory());
Expand Down

0 comments on commit b2a988c

Please sign in to comment.