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

Added Graphs to Statistics tab #1011

Closed
wants to merge 2 commits into from
Closed

Conversation

ghmarsh
Copy link

@ghmarsh ghmarsh commented Mar 29, 2022

Hi, I saw the post about trying to add graphs to the statistics tab in Issue #136 and thought I would try and add these graphs. I have added two graphs to the statistics page for current balance and monthly spending in the past year. These graphs were created using the Chart.js library.

For the "Balance" bar graph, it shows all the users current balance and the bar color will be red if they have a negative balance and green if they have a positive balance. Here are some pictures that show the graph with different inputs.

4813

4816

4817

For the "Expenses Per Month in The Past Year" graph, I made a line graph showing the expenses for the past 12 months. If there are less than 12 months of data available, it will just show what is available. Here are some pictures showing the graph on different inputs. The first graph shows the case when there are more than 12 months of data and the second graph shows when there are only 10 months of data.

4812

4815

In addition, I have run the tests given to make sure that these edits have not caused any further errors.

4818

This contribution was part of a school project so I might not have a ton of time to make edits in the future but feel free to reach out if you want to continue to try and implement this.

@almet
Copy link
Member

almet commented Mar 29, 2022

Hi, and thanks for doing this, it's appreciated :-) As you stated you won't be available to make edits, we will probably take it over from now on.

Maintainers, I believe we don't really need all the graphs here. I see little value in the balance bar graphs, so I think we could get rid of them, but the evolution of the monthly expenses makes more sense.

About the technical stack, this is relying on external dependencies that it vendorize. I'm not sure that's the best way to do this, but I'm not sure how to do it though. Do you have any ideas on this?

month[i] = 'Nov';
}
else if(month[i] == 12){
month[i] = 'Dec';
Copy link
Member

Choose a reason for hiding this comment

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

This means that it isn't translatable. Any idea on how we could achieve this?

}
});

const chart2 = document.getElementById('monthlySpending').getContext('2d');
Copy link
Member

Choose a reason for hiding this comment

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

we could rename this monthlyExpensesGraph

@@ -33,6 +33,12 @@ <h2>{{ _("Expenses by Month") }}</h2>
{% endfor %}
</tbody>
</table>
<h2>Graphs</h2>
Copy link
Member

Choose a reason for hiding this comment

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

We should make this translatable.

@@ -33,6 +33,12 @@ <h2>{{ _("Expenses by Month") }}</h2>
{% endfor %}
</tbody>
</table>
<h2>Graphs</h2>
<canvas id="balance" height="400"></canvas>
<canvas id="monthlySpending" height="400"></canvas>
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure about the fixed width: what happens when we view this from a mobile for instance?

let names = [];
let balance = [];
let colors = [];
for (let i = 0; i < members_stats.length; i++) {
Copy link
Member

Choose a reason for hiding this comment

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

We could probably use a for…in statement instead.

@Glandos
Copy link
Member

Glandos commented Apr 7, 2022

@almet commented on 29 mars 2022, 23:21 UTC+2:

Maintainers, I believe we don't really need all the graphs here. I see little value in the balance bar graphs, so I think we could get rid of them, but the evolution of the monthly expenses makes more sense.

I'm thinking the same thing.

About the technical stack, this is relying on external dependencies that it vendorize. I'm not sure that's the best way to do this, but I'm not sure how to do it though. Do you have any ideas on this?

I guess that we can't avoid that. But it will be time to add some code to load required JS only in some pages.

@Glandos
Copy link
Member

Glandos commented Apr 7, 2022

I even discover that there is pygal that can generate graphs on the server. There is even a Flask integration.
I think it won't be as flexible as a JS solution, but it is an alternative.

@almet
Copy link
Member

almet commented Dec 10, 2022

I'm closing this because we don't want to add and maintain some more JS on the client-side. Rather than using this approach, we would prefer using libraries like pygal as @Glandos mentioned. Thanks for your proposal though, see you around :-)

@almet almet closed this Dec 10, 2022
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.

3 participants