A multi-years graph with week granularity doesn't show the year

plausible / analytics

Simple, open source, lightweight (< 1 KB) and privacy-friendly web analytics alternative to Google Analytics.

https://plausible.io

GNU Affero General Public License v3.0

19.69k stars 1.05k forks source link

A multi-years graph with week granularity doesn't show the year #2573

Closed mquandalle closed 1 year ago

mquandalle commented 1 year ago

Past Issues Searched

[X] I have searched open and closed issues to make sure that the bug has not yet been reported

Issue is a Bug Report

[X] This is a bug report and not a feature request, nor asking for self-hosted support

Using official Plausible Cloud hosting or self-hosting?

Plausible Cloud from plausible.io

Describe the bug

If we display a multi-years graph while keeping the default granularity of a month, the corresponding years are displayed in the legend:

But there are not if we display the graph by "weeks" or "days":

Expected behavior

Display the year

Screenshots

No response

Environment

- OS:
- Browser:
- Browser Version:

metmarkosaric commented 1 year ago

thanks for reporting this @mquandalle, appreciate it!

idfunctor commented 1 year ago

If this issue is up for grabs, I'll be happy to take it up. I'd like to involve myself in small bugfixes like this so I can use my React expertise to slowly acquaint myself with an Elixir codebase, and in the future hopefully even be able to contribute meaningfully to non-UI issues.

metmarkosaric commented 1 year ago

Hi @idfunctor! This should be fixed for sure. We haven't started any work on it as all the devs are busy with other tasks so if you're up for it, feel free to go for it. Thanks very much for your contribution in helping us making Plausible better!

idfunctor commented 1 year ago

@metmarkosaric Hey! Alright, thanks, I'll begin tomorrow or Sunday. Given it's my first PR on the project, it will take me some time to find the right file, how to reproduce the issue etc. But I'll comment here for help if I am stuck with reproduction steps or finding the code. Once I find the related code it should be a quick fix.

Vigasaurus commented 1 year ago

@idfunctor Here's just a quick set of associated lines, if you'd like/need: https://github.com/plausible/analytics/blob/master/assets/js/dashboard/stats/graph/visitor-graph.js#L76-L100

idfunctor commented 1 year ago

@Vigasaurus Thanks for pointing me to the exact lines! Would have been much more time consuming for me without it. I am not sure how to proceed with of the review, but I just made a PR.

vinibrsl commented 1 year ago

Fixed by #2607 (thank you @idfunctor!). We'll include it in the next release of Plausible Cloud.

idfunctor commented 1 year ago

@vinibrsl Thanks! Please feel free to delegate to me any frontend issues or really basic backend issues (I'm an elixir noob but frontend expert) that need any work. I could take up react-router upgrade (refer https://github.com/plausible/analytics/pull/2671) but only if I'm sure it's an effort that needs time right now and would actually impact the project.