Skip to content

Commit

Permalink
CLDR-17658 Improve Dashboard performance and UI; work in progress (#3752
Browse files Browse the repository at this point in the history
)
  • Loading branch information
btangmu authored May 29, 2024
1 parent c5140cf commit 8f99ed9
Show file tree
Hide file tree
Showing 5 changed files with 381 additions and 38 deletions.
170 changes: 145 additions & 25 deletions tools/cldr-apps/js/src/esm/cldrDash.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,13 @@ import * as cldrStatus from "./cldrStatus.mjs";
import * as cldrSurvey from "./cldrSurvey.mjs";
import * as XLSX from "xlsx";

/**
* Notifications in these categories are combined so that there is not more than one per page.
* These categories can have well over 10,000 notifications, causing performance problems on
* the front end.
*/
const CATS_ONE_PER_PAGE = ["Abstained" /* , "Missing" */];

class DashData {
/**
* Construct a new DashData object
Expand All @@ -24,6 +31,8 @@ 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.pageCombinedEntries = {}; // map page to array of notification xpstrid on that page
this.updatingPath = false;
}

addEntriesFromJson(notifications) {
Expand All @@ -44,32 +53,107 @@ class DashData {
* @param {Object} e (entry in old format, from json)
*/
addEntry(cat, group, e) {
this.addCategory(cat);
this.catSize[cat]++;
if (!this.catFirst[cat]) {
this.catFirst[cat] = e.xpstrid;
try {
this.addCategory(cat);
this.catSize[cat]++;
if (!this.catFirst[cat]) {
this.catFirst[cat] = e.xpstrid;
}
if (CATS_ONE_PER_PAGE.includes(cat)) {
this.addCombinedEntry(cat, group, e);
} else if (this.pathIndex[e.xpstrid]) {
this.updateEntry(cat, group, e);
} else {
this.addNewEntry(cat, group, e);
}
} catch (err) {
console.error("Error in addEntry: " + err);
}
if (this.pathIndex[e.xpstrid]) {
const dashEntry = this.pathIndex[e.xpstrid];
dashEntry.addCategory(cat);
dashEntry.setWinning(e.winning);
if (e.comment) {
dashEntry.setComment(e.comment);
}

addCombinedEntry(cat, group, e) {
try {
const page = group.page;
if (!this.pageCombinedEntries[cat]) {
this.pageCombinedEntries[cat] = {};
}
if (e.subtype) {
dashEntry.setSubtype(e.subtype);
if (!this.pageCombinedEntries[cat][page]?.length) {
this.pageCombinedEntries[cat][page] = new Array(e.xpstrid);
this.addNewEntry(cat, group, e);
} else {
// TODO: make this work. unshift instead of push may be appropriate
// during an update (following a vote), since the combined notification
// is temporarily removed then added back, and in this case it may belong
// at the start of the array, not the end.
// Reference: https://unicode-org.atlassian.net/browse/CLDR-17658
if (this.updatingPath) {
this.pageCombinedEntries[cat][page].unshift(e.xpstrid);
} else {
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 addCombinedEntries for cat = " +
cat +
", page = " +
page
);
return;
}
const dashEntry = this.pathIndex[xpstrid];
if (!dashEntry) {
console.error(
"Existing entry not found in addCombinedEntry for cat = " +
cat +
", page = " +
page
);
return;
}
this.setComment(dashEntry, cat, page, null);
}
} else {
const dashEntry = new DashEntry(e.xpstrid, e.code, e.english);
dashEntry.setSectionPageHeader(group.section, group.page, group.header);
dashEntry.addCategory(cat);
dashEntry.setWinning(e.winning);
dashEntry.setPreviousEnglish(e.previousEnglish);
dashEntry.setComment(e.comment);
} catch (err) {
console.error("Error in addCombinedEntry: " + err);
}
}

updateEntry(cat, group, e) {
const dashEntry = this.pathIndex[e.xpstrid];
dashEntry.addCategory(cat);
dashEntry.setWinning(e.winning);
this.setComment(dashEntry, cat, group.page, e.comment);
if (e.subtype) {
dashEntry.setSubtype(e.subtype);
dashEntry.setChecked(this.itemIsChecked(e));
this.entries.push(dashEntry);
this.pathIndex[e.xpstrid] = dashEntry;
}
}

addNewEntry(cat, group, e) {
const dashEntry = new DashEntry(e.xpstrid, e.code, e.english);
dashEntry.setSectionPageHeader(group.section, group.page, group.header);
dashEntry.addCategory(cat);
dashEntry.setWinning(e.winning);
dashEntry.setPreviousEnglish(e.previousEnglish);
this.setComment(dashEntry, cat, group.page, e.comment);
dashEntry.setSubtype(e.subtype);
dashEntry.setChecked(this.itemIsChecked(e));
this.entries.push(dashEntry);
this.pathIndex[e.xpstrid] = dashEntry;
return e.xpstrid;
}

setComment(dashEntry, cat, page, comment) {
if (CATS_ONE_PER_PAGE.includes(cat)) {
dashEntry.setComment(
"Total " +
cat +
" entries on this page: " +
this.pageCombinedEntries[cat][page].length
);
} else {
dashEntry.setComment(comment);
}
}

Expand Down Expand Up @@ -150,9 +234,13 @@ class DashData {
}

removeEntry(dashEntry) {
const xpstrid = dashEntry.xpstrid;
this.removeEntryCats(dashEntry);
const index = this.entries.indexOf(dashEntry);
this.entries.splice(index, 1);
// Changed xpstrid means the entry wasn't really removed but was kept as representative of combined category
if (xpstrid === dashEntry.xpstrid) {
const index = this.entries.indexOf(dashEntry);
this.entries.splice(index, 1);
}
}

removeEntryCats(dashEntry) {
Expand All @@ -162,12 +250,42 @@ class DashData {
this.cats.delete(cat);
delete this.catSize[cat];
}
if (this.catFirst[cat] === dashEntry.xpstrid) {
if (CATS_ONE_PER_PAGE.includes(cat)) {
this.removeCombinedEntryCat(dashEntry, cat);
} else if (this.catFirst[cat] === dashEntry.xpstrid) {
this.findCatFirst(cat);
}
});
}

removeCombinedEntryCat(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];
// If this is the only category for this entry, then we can revise the
// entry to use the next xpstrid for the page -- unless that next xpstrid
// is already present for a different category...
if (dashEntry.cats.size === 1 && !this.pathIndex[nextXpstrid]) {
dashEntry.xpstrid = nextXpstrid;
delete this.pathIndex.xpstrid;
this.pathIndex[nextXpstrid] = dashEntry;
this.catFirst[cat] = nextXpstrid;
this.setComment(dashEntry, cat, page, null);
} else {
// TODO: fix this; work in progress
// Reference: https://unicode-org.atlassian.net/browse/CLDR-17658
// Remove this cat from the existing entry
dashEntry.cats.delete(cat);
// if (dashEntry.cats.size === 0) {
// delete this.pathIndex.xpstrid;
// }
}
}
}
}

findCatFirst(cat) {
for (let dashEntry of this.entries) {
if (dashEntry.cat === cat) {
Expand Down Expand Up @@ -335,6 +453,7 @@ function convertData(json) {
* containing notifications for a single path (old format)
*/
function updatePath(dashData, json) {
this.updatingPath = true;
try {
if (json.xpstrid in dashData.pathIndex) {
// We already have an entry for this path
Expand All @@ -352,6 +471,7 @@ function updatePath(dashData, json) {
} catch (e) {
cldrNotify.exception(e, "updating path for Dashboard");
}
this.updatingPath = false;
return dashData; // for unit test
}

Expand Down
9 changes: 9 additions & 0 deletions tools/cldr-apps/js/src/views/AboutPanel.vue
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,19 @@
</span>
</td>
</tr>
<tr>
<td class="aboutKey">Vue version</td>
<td>
<span class="aboutValue">{{ vueVersion }}</span>
</td>
</tr>
</tbody>
</table>
</div>
</template>

<script>
import { version } from "vue";
import * as cldrText from "../esm/cldrText.mjs";
export default {
Expand All @@ -58,10 +65,12 @@ export default {
comparePrefix: null,
compareMain: null,
compareRelease: null,
vueVersion: null,
};
},
created() {
this.vueVersion = " " + version;
fetch("api/about")
.then((r) => r.json())
.then(this.setData);
Expand Down
53 changes: 43 additions & 10 deletions tools/cldr-apps/js/src/views/DashboardWidget.vue
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
type="checkbox"
:title="describeShow(cat)"
:id="'dash-cat-checkbox-' + cat"
:checked="!catIsHidden[cat]"
:checked="!catCheckboxIsUnchecked[cat]"
@change="
(event) => {
catCheckmarkChanged(event, cat);
Expand Down Expand Up @@ -75,7 +75,8 @@
</header>
<section id="DashboardScroller" class="sidebyside-scrollable">
<template v-if="updatingVisibility">
<a-spin delay="1000" size="large" />
<!-- for unknown reason, the a-spin fails to appear on current Chrome/Firefox if any :delay is specified here -->
<a-spin size="large" />
</template>
<template v-else>
<template
Expand Down Expand Up @@ -181,6 +182,7 @@ import * as cldrNotify from "../esm/cldrNotify.mjs";
import * as cldrReport from "../esm/cldrReport.mjs";
import * as cldrStatus from "../esm/cldrStatus.mjs";
import * as cldrText from "../esm/cldrText.mjs";
import { nextTick } from "vue";
export default {
props: [],
Expand All @@ -195,12 +197,18 @@ export default {
localeName: null,
level: null,
downloadMessage: null,
catIsHidden: {},
catCheckboxIsUnchecked: {}, // default unchecked = false, checked = true
catIsHidden: {}, // default hidden = false, visible = true
updatingVisibility: false,
};
},
created() {
if (cldrStatus.getPermissions()?.userIsTC) {
this.catIsHidden["Abstained"] = this.catCheckboxIsUnchecked[
"Abstained"
] = true;
}
this.fetchData();
},
Expand Down Expand Up @@ -367,22 +375,47 @@ export default {
},
catCheckmarkChanged(event, category) {
// setTimeout solves a weakness in the Vue implementation: if the number of
// notifications is large, the checkbox in the header can take a second or more
// setTimeout is intended to solve a weakness in the Vue implementation: if the number of
// notifications is large, the checkbox in the header can take a second or even a minute
// to change its visible state in response to the user's click, during which time
// the user may click again thinking the first click wasn't recognized. Postponing
// the DOM update of thousands of rows ensures that the header checkbox updates
// the DOM update of thousands of rows should help ensure that the header checkbox updates
// without delay.
this.updatingVisibility = true;
setTimeout(
() => this.updateVisibility(event.target.checked, category),
0
// Also the booleans catCheckboxIsUnchecked and catIsHidden are distinct in order for
// the checkbox itself to update immediately even if the rows for the corresponding
// category may take a long time to update.
// Unfortunately, neither of these mechanisms seems guaranteed to prevent a very very
// long delay between the time the user clicks the checkbox and the time that the checkbox
// changes its state.
this.catCheckboxIsUnchecked[category] = !event.target.checked; // redundant?
const USE_NEXT_TICK = true;
this.console.log(
"Starting catCheckmarkChanged; USE_NEXT_TICK = " + USE_NEXT_TICK
);
this.updatingVisibility = true;
this.console.log("updatingVisibility = true");
if (USE_NEXT_TICK) {
nextTick().then(() => {
this.updateVisibility(event.target.checked, category);
});
} else {
const DELAY_FOR_VISIBILITY_UPDATE = 100; // milliseconds
this.console.log(
"DELAY_FOR_VISIBILITY_UPDATE = " + DELAY_FOR_VISIBILITY_UPDATE
);
setTimeout(
() => this.updateVisibility(event.target.checked, category),
DELAY_FOR_VISIBILITY_UPDATE
);
}
},
updateVisibility(checked, category) {
this.console.log("Starting updateVisibility");
this.catIsHidden[category] = !checked;
this.updatingVisibility = false;
this.console.log("updatingVisibility = false");
this.console.log("Ending updateVisibility");
},
canBeHidden(cats) {
Expand Down
Loading

0 comments on commit 8f99ed9

Please sign in to comment.