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

billed time in project progress view #4236

Merged
merged 9 commits into from
Aug 26, 2019

Conversation

youri-k
Copy link
Contributor

@youri-k youri-k commented Aug 15, 2019

URL of deployed dev instance (used for testing):

Steps to test:

  • abc

Issues:


@youri-k
Copy link
Contributor Author

youri-k commented Aug 15, 2019

@MichaelBuessemeyer The backend now reports billedMilliseconds. If this is too precise, the backend can calculate a better value.

@youri-k
Copy link
Contributor Author

youri-k commented Aug 15, 2019

@fm3 the backend should be ready to review 🙂

Copy link
Member

@fm3 fm3 left a comment

Choose a reason for hiding this comment

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

backend LGTM

@MichaelBuessemeyer
Copy link
Contributor

@daniel-wer Could you please review the frontend part?

Copy link
Member

@daniel-wer daniel-wer left a comment

Choose a reason for hiding this comment

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

Very nice, LGTM, apart from a small change 👍

title="Time [h]"
dataIndex="billedMilliseconds"
sorter={Utils.compareBy(typeHint, project => project.billedMilliseconds)}
render={number => Utils.millisecondsToHours(number).toLocaleString()}
Copy link
Member

Choose a reason for hiding this comment

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

toLocaleString seems to show up to three decimal places by default. Although the original request was to floor this number, I see a benefit in seeing a decimal place. Three may be a bit too much, but you can supply some options to toLocaleString, I'd go with 1 or 2: .toLocaleString(undefined, { maximumFractionDigits: 1}) :)

@bulldozer-boy bulldozer-boy bot merged commit e42abda into master Aug 26, 2019
@bulldozer-boy bulldozer-boy bot deleted the billed-hours-project-progress-view branch August 26, 2019 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show billed hours in project progress view
4 participants