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

Omit chart if all values are zero #54

Merged
merged 1 commit into from
Sep 6, 2024
Merged

Conversation

senier
Copy link
Contributor

@senier senier commented Aug 31, 2024

Do not show a chart if all values of the data series(es) are zero. Optionally, print a text in place of the chart.

This makes the UI clearer if certain features are not being used (like RPE).

Don't hesitate to request (or make) changes if you think this should be solved differently!

Copy link
Owner

@treiher treiher left a comment

Choose a reason for hiding this comment

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

At first I was a bit worried that omitting diagrams might be confusing in some cases, but I agree that the user interface is clearer when unused diagrams are omitted. I think this is a good improvement.

frontend/src/page/body_weight.rs Outdated Show resolved Hide resolved
frontend/src/page/menstrual_cycle.rs Outdated Show resolved Hide resolved
frontend/src/common.rs Outdated Show resolved Hide resolved
@senier senier force-pushed the hide_empty_charts branch 2 times, most recently from fed192b to 4e6c1ac Compare September 3, 2024 21:55
@senier
Copy link
Contributor Author

senier commented Sep 3, 2024

@treiher I applied the suggested changes and also fixed some complaints that clippy had.

Do you know what the issue with the frontend checks is? This happens to me on main, too, and doesn't seem to be related to my changes.

@senier senier requested a review from treiher September 3, 2024 22:01
@treiher
Copy link
Owner

treiher commented Sep 4, 2024

The failing end-to-end tests were caused by an update of Chromium. Unfortunately, I couldn't find a solution for these issues, so I had to disable the affected test parts (see #56 and #57). All checks on main are successful now.

frontend/src/common.rs Outdated Show resolved Hide resolved
frontend/src/common.rs Outdated Show resolved Hide resolved
@senier
Copy link
Contributor Author

senier commented Sep 5, 2024

The failing end-to-end tests were caused by an update of Chromium. Unfortunately, I couldn't find a solution for these issues, so I had to disable the affected test parts (see #56 and #57). All checks on main are successful now.

That's annoying! Thanks for looking into that issue. The tests succeed for me now, too.

@senier senier requested a review from treiher September 5, 2024 20:33
@treiher treiher merged commit 597d7bd into treiher:main Sep 6, 2024
26 checks passed
@senier senier deleted the hide_empty_charts branch September 6, 2024 21:10
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.

2 participants