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

Issue with Tummy time graph bar #556

Closed
Anoerak opened this issue Oct 29, 2022 · 4 comments
Closed

Issue with Tummy time graph bar #556

Anoerak opened this issue Oct 29, 2022 · 4 comments
Labels
enhancement Feature requests or improvements to existing functionality good first issue Good candidates for simple work for first time contributors

Comments

@Anoerak
Copy link
Contributor

Anoerak commented Oct 29, 2022

There is an issue on the Tummy time graph bar report where session above an hour are shown for only the minutes past the hours (1h10min becomes 10min) in the bar legend.
e.g. screenshots

image
image

@Anoerak
Copy link
Contributor Author

Anoerak commented Oct 30, 2022

Hi,
Issue's coming from file 'tummytime_duration.py', line 73:
return "{}m{}s".format(m, s)
Missing hours here ao should be:
return "{}h{}m{}s".format(h, m, s)

I'm not familiar with open source code and team work (newbie in development, actually following a course on JS & React aftter graduating in web development last year).
So if I can help in a more efficient way, I'm all ears :).

@cdubz
Copy link
Member

cdubz commented Oct 30, 2022

Nice catch. Here is where you found the issue:

def _duration_string_ms(duration):
"""
Format a "short" duration string with only minutes and seconds. This is
intended to fit better in smaller spaces on a graph.
:returns: a string of the form Xm.
"""
h, m, s = duration_parts(duration)
return "{}m{}s".format(m, s)

It would seem this was intentional to save space and because tummy time generally is not expected to be that long. I think an ideal solution here would be to add support for hours but only display it if necessary. E.g., we don't want 0h10m30s for a 10 minute, 30 seconds entry.

I'm not familiar with open source code and team work (newbie in development, actually following a course on JS & React aftter graduating in web development last year).
So if I can help in a more efficient way, I'm all ears :).

Well reporting an issue is an important first step 😄 The next thing we'd want here is pull request to fix the issue. The basic steps to do so would be:

  • Fork the repository.
  • Make the necessary changes in your fork.
  • Open a PR against master with your fork.

Let me know if you want to give this a try. Even with limited Python experience this one should be pretty straight forward.

@cdubz cdubz added bug Reports of unexpected problems or errors good first issue Good candidates for simple work for first time contributors hacktoberfest enhancement Feature requests or improvements to existing functionality and removed bug Reports of unexpected problems or errors labels Oct 30, 2022
@Anoerak
Copy link
Contributor Author

Anoerak commented Oct 31, 2022

OK, thanks.
I wrote a condition to display hour when needed and created a PR.
Let me know if I did right or wrong :)

edit: the Dokku test is failing and have no idea why.

@cdubz
Copy link
Member

cdubz commented Oct 31, 2022

@Anoerak haha yeah sorry about the Dokku test -- don't worry about it. Just something I am trying to get working but haven't yet 😄

#561 resolves this and is merged. Thank you!!

@cdubz cdubz closed this as completed Oct 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature requests or improvements to existing functionality good first issue Good candidates for simple work for first time contributors
Projects
None yet
Development

No branches or pull requests

2 participants