Upgraded Chart.js to version 4.4.2 #3233

harmitgoswami · 2024-05-17T13:48:46Z

Upgraded Chart.js dependency to latest version (4.4.2). Previously, Chart.js was included via a Chart.bundle.js static file. Support for this method has ended, so the dependency was included via a CDN. Changed all instances of deprecated functions to the new equivalents according to the migration guide.

mathjazz · 2024-05-17T20:02:16Z

pontoon/base/templates/base.html

+    <!-- Add Chart.js and Moment CDN link -->
+    <script src="https://cdn.jsdelivr.net/npm/[email protected]/dist/chart.umd.min.js"></script>
+    <script src="https://cdn.jsdelivr.net/npm/moment@^2.30.1"></script>
+    <script src="https://cdn.jsdelivr.net/npm/chartjs-adapter-moment@^1.0.1"></script>


We try to avoid loading static files from external sources. Please instead include the files in the repo.

…ment.js

mathjazz · 2024-05-20T13:06:34Z

pontoon/base/templates/base.html

+    <!-- Import Chart.js and Moment.js from static files -->
+    <script language="javascript" type="text/javascript" src="{{ static('js/lib/chart.umd.min.js') }}"></script>
+    <script language="javascript" type="text/javascript" src="{{ static('js/lib/moment.min.js') }}"></script>
+    <script language="javascript" type="text/javascript" src="{{ static('js/lib/chartjs-adapter-moment.min.js') }}"></script>


This will load all these libs on most of the views, even when they aren't needed.

We should load them, like other static files, through Django pipeline in settings, and only in the views where Chart.js was previously used.

Also, seems like pontoon/base/static/js/lib/chart.umd.js.map is not used?

You're right, I was getting a console error when I didn't include this file through base.html, but now that I've shifted over to local imports it seems this file isn't necessary.

mathjazz

The libraries are now loaded as expected. Nice work!

Please see my comment inline for the Profile chart.

I've noticed that the charts on http://localhost:8000/insights/ don't load.

Also, the Age of unreviewed suggestions and the Time to review pretranslations charts on http://localhost:8000/sl/insights/ shouldn't have legends.

mathjazz · 2024-05-21T19:15:21Z

pontoon/base/templates/base.html

@@ -28,7 +28,7 @@
      {% block extend_css %}
      {% endblock %}
    {% endblock %}
-
+    


Please restore this change.

mathjazz · 2024-05-21T19:38:37Z

pontoon/contributors/static/js/profile.js

+                  beginAtZero: true,
+                  maxTicksLimit: 3,
+                  precision: 2,
+                  callback: (value) => nf.format(value),


Why the change in the precision and the callback settings?

There are a few things that don't work as expected in the new Profile chart:

The curve doesn't have a background.

The Y scale data is off.

The tooltip: items overlap, the data format is off and there is no border.

The chart is cut-off at the top.

Can we keep the line curved?

mathjazz

I haven't looked at the code yet, but the changes look good. Nice work!

I noticed a few minor things, which should still be changed:

On the X scales, there should be no day number (e.g. "Jun" instead of "Jun 1").
We don't use a thousands separator on the Y scale ATM. I think it's a good idea to use it, but it should be , rather than . (same as in the tooltip).
Borders in bullet points in the tooltips don't look the same as in production - they have a 3Dish look and the first one is bigger than the rest.

harmitgoswami · 2024-05-23T19:27:21Z

I haven't looked at the code yet, but the changes look good. Nice work!

I noticed a few minor things, which should still be changed:

* On the X scales, there should be no day number (e.g. "Jun" instead of "Jun 1").

* We don't use a thousands separator on the Y scale ATM. I think it's a good idea to use it, but it should be `,` rather than `.` (same as in the tooltip).

* Borders in bullet points in the tooltips don't look the same as in production - they have a 3Dish look and the first one is bigger than the rest.

Thank you! It seems that both the day number and thousands separator styling issues are dependent on browser/location configuration (i.e. the thousands separator is indeed , instead of . and day numbers already appear in production for Firefox v126.0 en-US).

mathjazz · 2024-05-27T12:29:45Z

Thank you! It seems that both the day number and thousands separator styling issues are dependent on browser/location configuration (i.e. the thousands separator is indeed , instead of . and day numbers already appear in production for Firefox v126.0 en-US).

True, but I believe the situation is worse than on prod ATM, where e.g. for me all charts show months as "May" rather than "May 1", which is the case in some charts now. E.g. the chart on the /profile page shows months rightfully as "May", whereas the Time to review suggestions chart on /sl/insights shows months as "May 1".

Also, it seems like charts on the /insights page are broken again.

mathjazz · 2024-05-28T12:06:14Z

pontoon/insights/static/js/insights_tab.js

                  },
+                  tooltipFormat: 'MMM YYYY',


This (and other instances of the same value) should remain MMMM YYYY, which will show the full month name in the tooltip, where we have enough space.

harmitgoswami · 2024-05-28T20:02:22Z

The decision to switch from Moment.js over to Date-fns.js was mainly in hopes that the date display bug for the x-axis on charts will be resolved. The documentation for Moment.js states that no further features will be added, so it's possible that the visual consistency between that and Chart.js got lost at some point.

…perator for all locales

mathjazz

Very well done!

All charts work as expected.

Left some comments inline, mostly just to improve the code maintainability.

pontoon/contributors/static/js/profile.js

pontoon/insights/static/js/insights_charts.js

pontoon/insights/static/js/insights.js

pontoon/insights/static/js/insights_tab.js

pontoon/contributors/static/js/profile.js

mathjazz

Excellent work!

Just that minor Arrow function trick and we're good!

mathjazz · 2024-05-30T19:44:51Z

pontoon/contributors/static/js/profile.js

+                  labelColor: (context) => {
+                    return Pontoon.insights.setLabelColor(context);
+                  },


You can drop the body braces and the word "return":

Suggested change

labelColor: (context) => {

return Pontoon.insights.setLabelColor(context);

},

labelColor: (context) =>

Pontoon.insights.setLabelColor(context),

},

The same applies for all other calls of this function.

mathjazz

\o/

Very well done!

Harmit Goswami added 4 commits May 15, 2024 13:38

Upgraded Chart.js to v4.4.2

ffeef9f

Got chart legends to appear again, with incorrect CSS

35a348c

Fixed major chart legend styling issues

e940f2d

Added cubic interpolation and gradients back

5309640

harmitgoswami requested a review from mathjazz May 17, 2024 13:48

Removed chart variable assignments that caused linting errors

e90421c

mathjazz reviewed May 17, 2024

View reviewed changes

Changed to local import for Chart.js, Moment.js, and Chart-adapter-Mo…

f4eeb50

…ment.js

harmitgoswami requested a review from mathjazz May 17, 2024 21:20

mathjazz reviewed May 20, 2024

View reviewed changes

Loaded Chart.js through Django pipeline instead of base.html

5a56023

harmitgoswami requested a review from mathjazz May 21, 2024 16:06

mathjazz reviewed May 21, 2024

View reviewed changes

Harmit Goswami added 7 commits May 21, 2024 21:28

Fixed major styling bugs for locale/insights charts

0932ef6

Fixed linting issue in insights_tab

ceda07e

Fixed major styling issues for insights charts

5bdec21

Fixed linting errors

11c7506

Fixed profile chart styling, clipping issues for all graphs

3eda4ce

Fixed formatting issue in Profile.js

3b599c4

Fixed styling issues for tooltips on all charts

291e68f

harmitgoswami requested a review from mathjazz May 22, 2024 20:55

mathjazz reviewed May 23, 2024

View reviewed changes

Fixed tooltip label styling, disabled graphs bug

e30ae90

harmitgoswami requested a review from mathjazz May 23, 2024 19:27

Harmit Goswami added 2 commits May 27, 2024 15:44

fixed rander issue for insights and date formatting

5c62ae6

linting fixes

60b514a

mathjazz reviewed May 28, 2024

View reviewed changes

Refactored HTML Legend logic

5aa5780

Harmit Goswami added 4 commits May 28, 2024 11:41

linting fixes

43d8f17

fixed display bug for insights charts

ba5081a

Changed moment.js to date-fns.js

483d471

Fixed tooltip import for date-fns and alt-click legned bug

ab0f3f9

harmitgoswami requested a review from mathjazz May 28, 2024 19:57

Improved line visibility on insights chart, standardized thousands se…

4df1662

…perator for all locales

mathjazz reviewed May 29, 2024

View reviewed changes

Implemented review comments, refactored labelColor for tooltips

811a555

mathjazz reviewed May 30, 2024

View reviewed changes

pontoon/contributors/static/js/profile.js Outdated Show resolved Hide resolved

Harmit Goswami added 2 commits May 30, 2024 13:41

Refactored logic for changing tooltip label colors

3ee556b

Refactored htmlLegendPlugin logic for better code readability

ddbf226

harmitgoswami requested a review from mathjazz May 30, 2024 18:39

mathjazz reviewed May 30, 2024

View reviewed changes

Simplified anonymous function calls for setLabelColor

3729a90

harmitgoswami requested a review from mathjazz May 30, 2024 19:55

mathjazz approved these changes May 30, 2024

View reviewed changes

mathjazz merged commit d9679bb into mozilla:main May 30, 2024
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgraded Chart.js to version 4.4.2 #3233

Upgraded Chart.js to version 4.4.2 #3233

harmitgoswami commented May 17, 2024

mathjazz May 17, 2024

mathjazz May 20, 2024 •

edited

Loading

harmitgoswami May 21, 2024

mathjazz left a comment

mathjazz May 21, 2024

mathjazz May 21, 2024

mathjazz left a comment

harmitgoswami commented May 23, 2024

mathjazz commented May 27, 2024

mathjazz May 28, 2024

harmitgoswami commented May 28, 2024

mathjazz left a comment •

edited

Loading

mathjazz left a comment

mathjazz May 30, 2024

mathjazz left a comment

Upgraded Chart.js to version 4.4.2 #3233

Upgraded Chart.js to version 4.4.2 #3233

Conversation

harmitgoswami commented May 17, 2024

mathjazz May 17, 2024

Choose a reason for hiding this comment

mathjazz May 20, 2024 • edited Loading

Choose a reason for hiding this comment

harmitgoswami May 21, 2024

Choose a reason for hiding this comment

mathjazz left a comment

Choose a reason for hiding this comment

mathjazz May 21, 2024

Choose a reason for hiding this comment

mathjazz May 21, 2024

Choose a reason for hiding this comment

mathjazz left a comment

Choose a reason for hiding this comment

harmitgoswami commented May 23, 2024

mathjazz commented May 27, 2024

mathjazz May 28, 2024

Choose a reason for hiding this comment

harmitgoswami commented May 28, 2024

mathjazz left a comment • edited Loading

Choose a reason for hiding this comment

mathjazz left a comment

Choose a reason for hiding this comment

mathjazz May 30, 2024

Choose a reason for hiding this comment

mathjazz left a comment

Choose a reason for hiding this comment

mathjazz May 20, 2024 •

edited

Loading

mathjazz left a comment •

edited

Loading