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

LABX-379 Fix time calculation #171

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

alanhanaev
Copy link
Contributor

No description provided.

@alanhanaev alanhanaev changed the title WIP LABX-379 LABX-379 Fix time calculation Aug 6, 2018
@coveralls
Copy link

Coverage Status

Coverage remained the same at 10.92% when pulling 4beedd3 on alanhanaev:feature/LABX-379/to-do-fix_worked_time_calculation into 04ea3a7 on ChronoBank:develop.

this.workedTimeRender = this.workedTimeRender.bind(this)
this.progressIcon = this.progressIcon.bind(this)
this.handleComplete = this.handleComplete.bind(this)
this.handlePausePlayClick = this.handlePausePlayClick.bind(this)
Copy link
Collaborator

Choose a reason for hiding this comment

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

please remove all bind(this)
replace it with arrow function method

componentDidMount () {
const { job } = this.props
if (job.state === JOB_STATE_STARTED) {
this.intervalRef = setInterval(this.oneSecondInterv.bind(this), 1000)
Copy link
Collaborator

Choose a reason for hiding this comment

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

instead of bind(this) use arrow function method

/>
)
}

oneSecondInterv () {
const { job } = this.props
const pausedFor = get(job, "pausedFor") * 1000
Copy link
Collaborator

Choose a reason for hiding this comment

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

this usage of get function is redundunt
just use plain dot notation job.property


if (job.paused && job.state === JOB_STATE_STARTED) {
const now = Date.now()
const pausedAt = get(job, "pausedAt").getTime()
Copy link
Collaborator

Choose a reason for hiding this comment

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

this usage of get function is redundunt
just use plain dot notation job.property

return (
<p>
<span className={css.medium}>
{`${this.getDurationString(this.state.pausedTime)} total pause time`}
Copy link
Collaborator

Choose a reason for hiding this comment

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

http://test.laborx.io/worker-to-do.html
no 'total pause time' in mock up

@alanhanaev
Copy link
Contributor Author

There are problems with the travis, it loads the wrong version for verification

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