-
Notifications
You must be signed in to change notification settings - Fork 528
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
Add button to show older activity in activity log #3255
Conversation
@flodolo Could you please test this feature on stage to verify it indeed fixes the bug? Note that behavior on GitHub is different - rather than expanding the observed period by 1 month on each click, the button on GitHub appends 1-month period to the timeline. |
I think it's a bit confusing. https://mozilla-pontoon-staging.herokuapp.com/contributors/flod/
When I click the button, I would expect more activity to be loaded if available (e.g. load X more events, not expand the time filter). It's even more confusing when I select a day with activity, e.g. in February, and then click the button. |
@harmitgoswami Can we change the logic to load the data from the beginning of the preceding month? So if the currently displayed data ends in June 10, we load data from May 1 to June 10 and append it. We'd also need to add month indicators to the timeline. |
That makes sense to me. So then do we still want the button to add another month to the timeline per click, or like Flod suggested, retrieve latest X events? |
What flod suggests would be even better - so you don't click in vain. |
Thanks for the update! When I test the patch locally, my profile page says "Contribution activity since May 2024: ". It's not clear why since May and if that's since the beginning or since the end of May. My original suggestion was just for the data that we load additionally with the button. Sorry for not making that clearer. Ideally, I'd refactor |
@flodolo It turns out it'll be much easier for us to change the functionality to load events for the current month initially, and then append events for each preceding month after clicking the Show more activity button. That's also the behavior on the GH profile page. For less active users, that could mean no new events will be loaded after clicking the button, which we could address subsequently by automatically extending the period on the backend if no events are returned. Thoughts? |
By default, we're showing the graph for the last 12 months, the activity below should match that interval of time.
|
@flodolo How about the button then just loads all the remaining events rendered on the graph, i.e. for the last 365 days? We can also group events by month in the timeline. |
This works for me. And grouping would help visually. |
Not sure what the best way is to display this information... any thoughts? |
Can you expand on "this information"? |
I think we need @mathjazz here, to avoid going too far in the wrong direction, but this looks like too much data. My expectation is that we keep the current structure, but group it by month (i.e. take the current look of activities, repeat it for each month if there is data). |
Agreed with Flod. And we should follow the GitHub profile page UI - add "Month YEAR" (e.g. May 2024) to the vertical timeline. |
@harmitgoswami This is currently marked as a draft PR; could you clarify what sort of review you'd like of it, and/or what's still intended to be done to it? |
Hi Eemeli, I’m mainly looking for code style review for this PR. I’ve implemented the design and functionality that @mathjazz had requested, but am looking for code readability/performance feedback before I mark this PR as Open once again. |
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.
A bunch of small stuff, but the big issue is the inefficiency in how the data is queried from the DB.
pontoon/contributors/templates/contributors/includes/timeline.html
Outdated
Show resolved
Hide resolved
pontoon/contributors/templates/contributors/includes/timeline.html
Outdated
Show resolved
Hide resolved
c98598c
to
48a2360
Compare
</ul> | ||
|
||
{% endfor %} | ||
{% endif %} |
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.
Nit: mind the blank line.
pontoon/contributors/templates/contributors/includes/timeline.html
Outdated
Show resolved
Hide resolved
pontoon/contributors/templates/contributors/includes/timeline.html
Outdated
Show resolved
Hide resolved
pontoon/contributors/templates/contributors/includes/timeline.html
Outdated
Show resolved
Hide resolved
pontoon/insights/utils.py
Outdated
@@ -460,6 +460,6 @@ def get_global_pretranslation_quality(category, id): | |||
] | |||
|
|||
return { | |||
"dates": list({convert_to_unix_time(x["month"]) for x in actions}), | |||
"dates": sorted(list({convert_to_unix_time(x["month"]) for x in actions})), |
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.
This is already merged, so it shouldn't appear in the diff:
136e382
pontoon/contributors/templates/contributors/includes/timeline.html
Outdated
Show resolved
Hide resolved
pontoon/contributors/templates/contributors/includes/timeline.html
Outdated
Show resolved
Hide resolved
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.
I tried removing myself as a reviewer, but that didn't seem to work. I'm fine with this if Matjaž is, but please explicitly re-request a review from me if one is desired.
Massive refactor of all prior work on this ticket. Given the new QuerySet fields (thanks Matjaz), the entire information flow has been dramatically reduced in complexity. Some points of weakness are that I still need to manually restructure and reorder the object before passing to the template
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.
|
||
{% for title, values in contribution_timeline.contributions.items() %} | ||
{% for month, data in contribution_timeline.contributions.items() %} | ||
{{month}} |
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.
Let's wrap month into a HTML element for easier styling. Please also move it within the div.contribution-group
element where it semantically belongs.
pontoon/contributors/templates/contributors/includes/timeline.html
Outdated
Show resolved
Hide resolved
pontoon/contributors/templates/contributors/includes/timeline.html
Outdated
Show resolved
Hide resolved
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.
Great work, Harmit!
As mentioned, I'll push minor CSS changes directly and ask you for a review. Then we can land this. Well done!
@harmitgoswami I've updated the CSS slightly as we agreed. Let me know if changes look good to you. @flodolo Could you please test this a little on stage? |
Seems to work correctly as far as I can tell (but stage doesn't have recent activity, so the list is always empty until I click the button). |
LGTM! |
Fix #2622
Implemented a button to show more (i.e. older) activity in the user profile activity log. This feature was achieved by modifying the existing
get_contribution_timeline_data
function to retrieve data from the past N months, where N is a counter that the user can increase by one via clicking on the "Show more activity" button. Once the number of months shown on the activity log predates the user's account creation date, the "Show more activity" button is hidden, similar to how GitHub implements this feature.To reproduce results: After starting the server, navigate to any user's profile
(localhost:8000/contributors/[username])
. Once there, click the show more activity button to display an additional month's worth of data. When choosing to view a specific contribution type, the counter resets to 0, and the "Show more activity" button is displayed again (if it was hidden before).