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

Visual feedback on number of hours logged #473

Merged
6 commits merged into from
May 20, 2022
Merged

Visual feedback on number of hours logged #473

6 commits merged into from
May 20, 2022

Conversation

HenrikeW
Copy link
Contributor

@HenrikeW HenrikeW commented May 20, 2022

Related issue(s) and PR(s)

This PR closes #156

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Other

List of changes made

  • the sum cell for each day will have a check icon if 8 hours are logged on the day
  • it will have a warning icon if less or more hours are logged
  • the weekly sum cell will have the check icon if 40 hours were logged
  • it will have a warning icon if less or more than 40 hours were logged
  • the icons are only displayed for weeks in the past. (For the current week, it needs to be at least Saturday for the icons to be displayed).

Screenshot of the fix

Testing

  • Go to a future week: you shouldn't see any icons
  • Go to the current week: if it's before Saturday you shouldn't see any icons
  • Go to a past week: you should see the icons as described above.
  • Make a change in the hours of a time entry to meet the cases described above: As soon as you type, the icon should update together with the sum. (You don't need to save)

Further comments

Definition of Done checklist

  • I have made an effort making the commit history understandable
  • I have performed a self-review of my own code and commented any hard-to-understand areas
  • Tests and lint/format validations are passing
  • My changes generate no new warnings

@HenrikeW HenrikeW requested review from jonandernovella, KattisLej, a user and kusalananda May 20, 2022 09:29
Copy link
Contributor

@jonandernovella jonandernovella left a comment

Choose a reason for hiding this comment

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

Looks good! i left a few comments

@KattisLej
Copy link
Contributor

Starting review

Copy link
Contributor

@KattisLej KattisLej left a comment

Choose a reason for hiding this comment

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

In redmine you can log time on one day, more than 8 hours, but here the sum can be 40 but still showing warnings on the fields of the days. Will urdr not have this possibility?

@HenrikeW
Copy link
Contributor Author

In redmine you can log time on one day, more than 8 hours, but here the sum can be 40 but still showing warnings on the fields of the days. Will urdr not have this possibility?

Good point. Will ask Johan about that.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Works ok, but do we know that all Urdr users really are expected to report 8 x 5 = 40 hours every week?

@kusalananda
Copy link
Member

Works ok, but do we know that all Urdr users are expected to report 8 x 5 = 40 hours every week?

Users are expected to log precisely 40 hours every week, but there is no expectation that these should be distributed evenly over all days. Some users may prefer to log all their time for the week on Fridays.

Having said that, having the indicators on every day would not stop anyone from logging time only on Fridays.

@ghost ghost merged commit b313e68 into develop May 20, 2022
@ghost ghost deleted the dev/hour-feedback branch May 20, 2022 13:32
@jonandernovella jonandernovella mentioned this pull request May 20, 2022
This pull request was closed.
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.

Visual indication for number of hours logged
4 participants