Skip to content
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

Pretranslation monitoring: Insights page #2932

Merged

Conversation

mathjazz
Copy link
Collaborator

@mathjazz mathjazz commented Aug 15, 2023

Part 3/3 of #2923.

This PR adds the third and final part of Pretranslation Monitoring, namely the new "Insights" page.

The PR is based on #2929, the first commit specific to this PR is 86283b6, which (together with e48f0d0) is just a preparation for the actual changes introduced in f4f5355.

It is deployed to stage:
https://mozilla-pontoon-staging.herokuapp.com/insights/

TODO:

@mathjazz mathjazz force-pushed the 2923-pretranslation-monitoring-insights-page branch from 02aab39 to f4f5355 Compare August 15, 2023 16:32
@mathjazz
Copy link
Collaborator Author

Investigating the funkiness of the sl (and All) charts. Seems like an issue with the X-axis.

@mathjazz mathjazz requested a review from eemeli August 15, 2023 17:58
@mathjazz
Copy link
Collaborator Author

Investigating the funkiness of the sl (and All) charts. Seems like an issue with the X-axis.

Switched from a built-in time-based scale on an X-axis to a default one and that solved the problem.

That charts no longer looks like this:

Screenshot 2023-08-15 at 21 44 13

🤯

Also, we should upgrade the chart library. Will file a bug for that.

@flodolo
Copy link
Collaborator

flodolo commented Aug 16, 2023

No familiar with the chart library, but would it be possible to have a "select/deselect all" in the legend (or have the "double click = disable all but this" behavior)?

@mathjazz
Copy link
Collaborator Author

Yeah, took me about a second of using the chart to had the same wish. :)

We can render the legend manually in a custom div, which allows for such customization. It will also allow us to align both charts horizontally and enable to scale better when more locales / projects are added. I'll give it a shot.

@mathjazz
Copy link
Collaborator Author

We can render the legend manually in a custom div, which allows for such customization. It will also allow us to align both charts horizontally and enable to scale better when more locales / projects are added. I'll give it a shot.

Done. Cmd + Click or Alt + Click shows only the clicked item. Let me know how it feels.

@flodolo
Copy link
Collaborator

flodolo commented Aug 16, 2023

Done. Cmd + Click or Alt + Click shows only the clicked item. Let me know how it feels.

Works great. Doesn't feel like it needs a "select all", since you can always reload the page to go back.

Copy link
Member

@eemeli eemeli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly minor JS issues, see inline. Otherwise fine.

pontoon/insights/static/js/insights_charts.js Outdated Show resolved Hide resolved
pontoon/insights/static/js/insights_charts.js Outdated Show resolved Hide resolved
pontoon/insights/static/js/insights_tab.js Outdated Show resolved Hide resolved
pontoon/insights/static/js/insights_tab.js Outdated Show resolved Hide resolved
pontoon/insights/static/js/insights_tab.js Outdated Show resolved Hide resolved
pontoon/insights/static/js/insights_tab.js Outdated Show resolved Hide resolved
pontoon/insights/static/js/insights_tab.js Show resolved Hide resolved
@mathjazz
Copy link
Collaborator Author

Fixed percentages in db32055.

Honestly, I'm not sure I like this approach better, because the values are actual percentages.

@mathjazz mathjazz requested a review from eemeli August 17, 2023 21:01
@eemeli
Copy link
Member

eemeli commented Aug 17, 2023

Would recommend that you spin off the "Upgrade Chart.js" todo item into a separately tracked issue, so it's not blocking this PR.

@mathjazz mathjazz force-pushed the 2923-pretranslation-monitoring-insights-page branch from 59890a2 to ae6623d Compare August 18, 2023 13:13
@mathjazz mathjazz requested a review from eemeli August 18, 2023 13:58
@mathjazz mathjazz merged commit d1714dd into mozilla:master Aug 18, 2023
6 checks passed
@mathjazz mathjazz deleted the 2923-pretranslation-monitoring-insights-page branch August 18, 2023 14:16
@mathjazz mathjazz linked an issue Aug 18, 2023 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement pretranslation monitoring
3 participants