-
Notifications
You must be signed in to change notification settings - Fork 385
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CLDR-15649 Dashboard using filters #3640
Conversation
-Add checkbox next to each notification category in Dashboard header -The category is hidden if the checkbox is unchecked -Error and Missing notifications still cannot be hidden with pre-existing right-side path-specific checkboxes, but they can be hidden categorically using the new checkboxes -Show each path with notifications only once in Dashboard -If a path has more than one notification, combine them on one line -Perform data conversion on the front end; http response still has multiple notifications per path -Refactor to avoid http code in Vue; call cldrAjax.mjs from cldrDash.mjs not from DashboardWidget.vue -New classes DashData and DashEntry in cldrDash.mjs -Use new method dashData.addEntriesFromJson for both whole-page json and single-path json -Make hover titles clearer for circled abbreviations like EC for English Changed -As before, always hide Abstained notifications if user is TC, but implement on the back end instead of the front end (simpler and more efficient) -Turn off debug-logging in cldrTable (CLDR_TABLE_DEBUG) and cldrVote (CLDR_VOTE_DEBUG)
-Add checkbox next to each notification category in Dashboard header -The category is hidden if the checkbox is unchecked -Error and Missing notifications still cannot be hidden with pre-existing right-side path-specific checkboxes, but they can be hidden categorically using the new checkboxes -Show each path with notifications only once in Dashboard -If a path has more than one notification, combine them on one line -Perform data conversion on the front end; http response still has multiple notifications per path -Refactor to avoid http code in Vue; call cldrAjax.mjs from cldrDash.mjs not from DashboardWidget.vue -New classes DashData and DashEntry in cldrDash.mjs -Use new method dashData.addEntriesFromJson for both whole-page json and single-path json -Make hover titles clearer for circled abbreviations like EC for English Changed -As before, always hide Abstained notifications if user is TC, but implement on the back end instead of the front end (simpler and more efficient) -Update and extend the unit test TestCldrDash.mjs, since DashData is now different format from json -Turn off debug-logging in cldrTable (CLDR_TABLE_DEBUG) and cldrVote (CLDR_VOTE_DEBUG)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Just tried this out on Smoke.
https://cldr-smoke.unicode.org/cldr-apps/v#/ab//
Two things I ran into:
1. *I turned off several items, but then couldn't turn them back on. *The
problem is that the check mark doesn't change to ON until the dashboard has
refreshed. So there is a long wait, then it flips. In the meantime, I would
click again, not realizing it was "in process". That would start the ball
rolling again, and so on. I see two possibilities.
1. Flip the check mark to an image indicating 'in process', then to
check (or empty) once completed.
2. Gray out the items in the dashboard until the refresh is done.
2. *The "buttons" are really labels now, and should work like labels and
look like labels. *That is, the check should go on the right side (not
left), and clicking on the label should be equivalent to clicking on the
check box. I see no need for us to have the box around the label — if
anything, because it is not normal iconography, it misleads people.
…On Mon, Apr 29, 2024 at 10:47 AM Tom Bishop ***@***.***> wrote:
Merged #3640 <#3640> into main.
—
Reply to this email directly, view it on GitHub
<#3640 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACJLEMHW35TCO2LWV2S5GI3Y72BUPAVCNFSM6AAAAABGLVZM2CVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJSGY2DMOBRGA3DMOI>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
@macchiati "Two things" -- 1. The response to clicking a checkbox takes about a second when I test it, and you're right it ought to update immediately at least to show the current checkbox state even if updating all the rows might take a second. I'll investigate that. |
@macchiati Here's a PR to fix the first thing, checkbox update speed: #3662 The checkbox now updates as soon as clicked. It may still take a second for the rows to refresh. There could be additional visual feedback to show that's in progress, however it might be more distracting than helpful... |
- It would be better to have some indication that it is in progress,
since in the cases I saw it was not just a second, but several seconds
(long enough to make me think it wasn't working). One option is to have the
same visual as when the dashboard is first opened; a spinning image,
where once the dashboard is populated it disappears. I'm guessing since you
already have that it would be straightforward.
- Checkbox on the left is clearly the default for HTML that people are
used to, eg
https://www.w3schools.com/tags/tryit.asp?filename=tryhtml5_input_type_checkbox
- The button functionality is very confusing. Since we have filters now
I don't think it is necessary. If we really wanted that, we would have an
additional icon after the label, something like (fast forward) rotated to
point down.
So something like (ignore the placement/style)
✅ Missing (374) [image: Screenshot 2024-04-30 at 18.24.18.png]
…On Tue, Apr 30, 2024 at 10:25 AM Tom Bishop ***@***.***> wrote:
@macchiati <https://github.com/macchiati> Here's a PR to fix the first
thing, checkbox update speed: #3662
<#3662>
The checkbox now updates as soon as clicked. It may still take a second
for the rows to refresh. There could be additional visual feedback to show
that's in progress, however it might be more distracting than helpful...
—
Reply to this email directly, view it on GitHub
<#3640 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACJLEMC37YRQPTOSBS7TMKDY77HY3AVCNFSM6AAAAABGLVZM2CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOBWGEYTQMBYHE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
- fix parameters to sheet construction - refactor XLS sheet and file calculation - unicode-org#3640 restructured some of the front-end data
- fix parameters to sheet construction - refactor XLSX sheet and file calculation - unicode-org#3640 restructured some of the front-end data
-Add checkbox next to each notification category in Dashboard header
-The category is hidden if the checkbox is unchecked
-Error and Missing notifications still cannot be hidden with pre-existing right-side path-specific checkboxes, but they can be hidden categorically using the new checkboxes
-Show each path with notifications only once in Dashboard
-If a path has more than one notification, combine them on one line
-Perform data conversion on the front end; http response still has multiple notifications per path
-Refactor to avoid http code in Vue; call cldrAjax.mjs from cldrDash.mjs not from DashboardWidget.vue
-New classes DashData and DashEntry in cldrDash.mjs
-Use new method dashData.addEntriesFromJson for both whole-page json and single-path json
-Make hover titles clearer for circled abbreviations like EC for English Changed
-As before, always hide Abstained notifications if user is TC, but implement on the back end instead of the front end (simpler and more efficient)
-Update and extend the unit test TestCldrDash.mjs, since DashData is now different format from json
-Turn off debug-logging in cldrTable (CLDR_TABLE_DEBUG) and cldrVote (CLDR_VOTE_DEBUG)
CLDR-15649
ALLOW_MANY_COMMITS=true