CLDR-17658 Improve Dashboard performance and UI; work in progress #3752

btangmu · 2024-05-24T21:17:22Z

-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 only the Abstained category (not Missing); easy to change which categories are affected

-Add unit test

-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

CLDR-17658

This PR completes the ticket.

ALLOW_MANY_COMMITS=true

-For now, change only the front end -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

macchiati · 2024-05-26T19:01:02Z

I pushed to staging to try it out. Good progress!

MUCH more responsive when there are many abstained!
Logged in as a TC, I still don't see Abstained label (minor, but we wanted to fix this now that TCs wouldn't be overwhelmed by Abstained items)
When I voted on an item, the Abstained dashboard row disappeared. But there were still Abstaineds on that page, and the Dashboard row it wasn't replaced by another Abstained dashboard row on that page, even after closing and opening the dashboard. [I'm guessing that that is a WIP. We were thinking of having a Dashboard row with just Section-Page that always goes to the first Abstained on the Page, but it is simpler to always just replace the one voted on by the next in the page (keeping the details of header and code, that would be fine.]

-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

-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 -Add unit test -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

btangmu · 2024-05-28T19:40:46Z

As discussed in infra meeting, I'm making this ready for review in spite of remaining problems. The dashboard doesn't update properly after voting. A work-around may be to refresh the dashboard frequently using the "↻" button.

srl295 · 2024-05-28T21:30:58Z

tools/cldr-apps/js/src/esm/cldrDash.mjs

+        // Use the FIRST item in the array as the representative,
+        // with its comment indicating the size of the array


might be able to sort by section+code (if it's currently unsorted?) to get the lowest code. which should correspond to the 'highest' (visually) item. For future

Yes, there's a ticket already: https://unicode-org.atlassian.net/browse/CLDR-15648

In the meantime, the complete (all-path) dashboard json response is sorted, however, after voting on a path, there's a miniature dash response for that path alone, and then the front end doesn't always know where to insert it...

srl295 · 2024-05-28T22:17:43Z

tools/cldr-apps/js/src/views/AboutPanel.vue

+        <tr>
+          <td class="aboutKey">Vue version</td>
+          <td>
+            <span class="aboutValue">{{ vueVersion }}</span>


i wonder if there's a way to get antdv versions

srl295 · 2024-05-28T22:18:39Z

tools/cldr-apps/js/src/views/DashboardWidget.vue

@@ -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" />


still couldn't repro this. but, the logistics of this update are pretty odd..

srl295 · 2024-05-28T22:19:29Z

tools/cldr-apps/js/src/views/DashboardWidget.vue

+      // 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.


I wouldn't say it's a weakness in Vue itself but a weakness in our usage of the order of 10k DOM objects.

srl295 · 2024-05-28T22:21:11Z

tools/cldr-apps/src/main/java/org/unicode/cldr/web/Dashboard.java

-        if (UserRegistry.userIsTC(user)) {
-            choiceSet.remove(NotificationCategory.abstained);
-        }


macchiati · 2024-05-29T01:02:04Z

Just got back to this; merging since Steven reviewed

btangmu · 2024-05-30T15:21:50Z

tools/cldr-apps/js/src/esm/cldrDash.mjs

+        // 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) {


Bug: this.updatingPath is always false here, because here "this" = dashData (as intended), but below when this.updatingPath is set to true, "this" = cldrDash

btangmu · 2024-05-30T15:22:56Z

tools/cldr-apps/js/src/esm/cldrDash.mjs

@@ -334,6 +452,7 @@ function convertData(json) {
 *               containing notifications for a single path (old format)
 */
 function updatePath(dashData, json) {
+  this.updatingPath = true;


Bug here: "this" = cldrDash NOT dashData

It should be dashData.updatingPath, though it still might not work right; still work in progress

FIxing that bug still doesn't make update work right; still working on it

Note: the "this" bug was fixed in #3777

btangmu self-assigned this May 24, 2024

btangmu added 3 commits May 28, 2024 12:47

btangmu requested review from srl295 and macchiati May 28, 2024 19:41

srl295 approved these changes May 28, 2024

View reviewed changes

macchiati merged commit 8f99ed9 into unicode-org:main May 29, 2024
11 checks passed

btangmu commented May 30, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CLDR-17658 Improve Dashboard performance and UI; work in progress #3752

CLDR-17658 Improve Dashboard performance and UI; work in progress #3752

btangmu commented May 24, 2024 •

edited

Loading

macchiati commented May 26, 2024

btangmu commented May 28, 2024

srl295 May 28, 2024

btangmu May 30, 2024

srl295 May 28, 2024

srl295 May 28, 2024

srl295 May 28, 2024

srl295 May 28, 2024

srl295 May 28, 2024

macchiati commented May 29, 2024

btangmu May 30, 2024

btangmu May 30, 2024 •

edited

Loading

btangmu May 30, 2024

btangmu Jun 5, 2024

		// Use the FIRST item in the array as the representative,
		// with its comment indicating the size of the array

CLDR-17658 Improve Dashboard performance and UI; work in progress #3752

CLDR-17658 Improve Dashboard performance and UI; work in progress #3752

Conversation

btangmu commented May 24, 2024 • edited Loading

macchiati commented May 26, 2024

btangmu commented May 28, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

macchiati commented May 29, 2024

Choose a reason for hiding this comment

btangmu May 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

btangmu commented May 24, 2024 •

edited

Loading

btangmu May 30, 2024 •

edited

Loading