Implement contribution graph #2615
Conversation
Fix tooltip plurals Modernize date display Replace formatString with Template literals Simplify input data conversion into an object Various simplifications of the contributor graph lib More simplification Simplify date handling Less configuration Remove header Move chart code to profile.js No need for data reformatting Remove underscores
|
Deployed to stage: |
| <div class="menu" style="display: none;"> | ||
| <ul> | ||
| <li class="clearfix"> | ||
| <span data-type="user_translations" title="Translations submitted by the user">User translations</span> |
There was a problem hiding this comment.
There are not the categories in the specs? I believe those are clearer.
There was a problem hiding this comment.
I changed them, because the ones in the spec weren't clear to me. :-)
E.g. We don't use the term "Submissions", "performed" and "received" sound unusual when used together with Review and "Submissions and reviews" isn't clear about excluding reviews of user's translations.
I tried to make thing clearer in the tooltip, but I understand that's not very visible. Maybe we should show the tooltip text underneath the chart for the selected option?
There was a problem hiding this comment.
- The concept of "peer" doesn't exist anywhere. Funny enough, it's almost never a peer who reviews your work.
- "User review" doesn't say much: are those done by the user, or received by the user.
Submissions was used to make it clear that's something entered by the user, and translation can't be used.
Since this went through review internally and externally, I'd stick to the specs. If those turn out to be unclear in everyday use, changing them should be very quick.
There was a problem hiding this comment.
Since this went through review internally and externally, I'd stick to the specs. If those turn out to be unclear in everyday use, changing them should be very quick.
Fair point. Updated the wording.
| const graph = $('#contribution-graph'); | ||
| const contributions = graph.data('contributions'); | ||
|
|
||
| // Set start date to 365 days before now | ||
| var startDate = new Date(); |
There was a problem hiding this comment.
Mixing const and var like this is weird. Just make them all const and let, as appropriate.
| gx = week * step; | ||
| } | ||
|
|
||
| startDate.setUTCDate(startDate.getUTCDate() + 1); |
There was a problem hiding this comment.
This should not be called startDate if/as it's being used as an iterator.
| var count = $(e.target).attr('data-count'); | ||
| var date = shortDateFormat.format($(e.target).attr('data-date')); | ||
| var action = count == 1 ? 'contribution' : 'contributions'; | ||
| var text = `${count} ${action} on ${date}`; |
There was a problem hiding this comment.
Alas, if only we had a localization system that could handle this sort of stuff. 😇
There was a problem hiding this comment.
:-)
We don't use it outside the translate app, yet.
pontoon/contributors/utils.py
Outdated
| total = sum(contributions.values()) | ||
|
|
||
| return { | ||
| "contributions": json.dumps(contributions), |
There was a problem hiding this comment.
Why does this need to get double-encoded like this? If there is a need for this, some code comment here explaining the situation would be a really good idea.
Co-authored-by: Eemeli Aro <eemeli@gmail.com>
Co-authored-by: Eemeli Aro <eemeli@gmail.com>
There was a problem hiding this comment.
I've added a commit with a proposed change that would allow contributor data to not get double-encoded when passed through the GET /update-contribution-graph REST endpoint.
@mathjazz I hope that's ok with you? This seemed like the easiest way for me to explain what I think should be done here.
Ohhhh, now I see what you meant with double-encoding. Thanks for the fix! I'm refactoring the outputs of the utils functions in the 2nd half of #2487, but I'll take that commit and update there. |
That's consistent with the rest of the app. There are a few other things to improve in Pontoon dropdowns (e.g. some lack and arrow and hence don't look like dropdowns, some don't work well with long texts, etc.), so let's address this afterwards. |
This patch partially fixes #2487. It implements the contribution graph, with the ability to display different contribution types (translations, reviews, both, peer reviews, etc.),
I still need to add a unit test for the utils function and optimize the request to render all contributions.The missing piece is the timeline that renders under the graph on page load and updates after selecting a day on the graph.