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

Add timeout timer to close error message after five seconds. Resolves #51 #69

Merged
merged 1 commit into from Jan 18, 2017

Conversation

jason00111
Copy link
Collaborator

No description provided.

window.setTimeout(() => {
this.setState({error: null})
}, 5000)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

this is a good start. But we want to make sure we dont remove the error sooner than 5 seconds. Imagine the scenario:
0. user doest something that causes an error
0. we show the error and schedule the error removal for 5 seconds from now
0. 3 seconds go by
0. user doest something that causes another error
0. the error will be removed too quickly.

To solve this lets make sure that when an error occurs we use clearTimeout to disable any previously scheduled error removal timeoutes.

Copy link
Contributor

Choose a reason for hiding this comment

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

rename startErrorTimeout toscheduleErrorRemoval

@jaredatron jaredatron changed the title Add timeout timer to close error message after five seconds. Resolves #51 WIP: Add timeout timer to close error message after five seconds. Resolves #51 Jan 17, 2017
@jason00111 jason00111 changed the title WIP: Add timeout timer to close error message after five seconds. Resolves #51 Add timeout timer to close error message after five seconds. Resolves #51 Jan 18, 2017
})
}

scheduleErrorRemoval() {
if (this.state.errorTimeoutId) {
window.clearTimeout(this.state.errorTimeoutId)
Copy link
Contributor

Choose a reason for hiding this comment

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

you do not need the window. part. setTimeout and clearTimeout are global variables.

@jaredatron
Copy link
Contributor

Looks good. For what it's worth I could have written it like this.

import React, { Component, PropTypes } from 'react'
import moment from 'moment'
import Link from '../../atoms/Link'
import Button from '../../atoms/Button'
import PrrrsTable from '../PrrrsTable'
import ErrorMessage from '../../atoms/ErrorMessage'
import claimPrrr from '../../../actions/claimPrrr'

const ERROR_DISPLAY_DURATION = 5000

export default class PendingPrrrs extends Component {
  static propTypes = {
    currentUser: PropTypes.object.isRequired,
    prrrs: PropTypes.array.isRequired,
  }

  constructor(props){
    super(props)
    this.state = {
      error: null,
      errorTimeoutId: null
    }
    this.clearError = this.clearError.bind(this)
  }

  claimPrrr(prrr){
    claimPrrr(prrr.id)
      .then(_ => {
        const url = `https://github.com/${prrr.owner}/${prrr.repo}/pull/${prrr.number}`
        const popup = window.open(url, '_blank')
        if (popup) popup.focus()
      })
      .catch(error => {
        this.reportError(error)
      })
  }

  reportError(error) {
    if (this.state.errorTimeoutId) {
      clearTimeout(this.state.errorTimeoutId)
    }
    const errorTimeoutId = setTimeout(this.clearError, ERROR_DISPLAY_DURATION)
    this.setState({error, errorTimeoutId})
  }

  clearError(){
    this.setState({error: null})
  }

  renderAdditionalHeaders(){
    return [
      <th key="actions">Actions</th>,
    ]
  }

  renderAdditionalCells = (prrr) => {
    const { currentUser } = this.props
    const disabled = prrr.requested_by === currentUser.github_username
    return [
      <td key="actions">
        <Button onClick={_ => this.claimPrrr(prrr)} disabled={disabled}>
          Claim
        </Button>
      </td>,
    ]
  }

  render(){
    const prrrs = this.props.prrrs
      .filter(prrr => !prrr.claimed_by)
      .sort((a, b) =>
        moment(a.created_at).valueOf() -
        moment(b.created_at).valueOf()
      )

    return <div>
      {this.state.error ? <ErrorMessage error={this.state.error} /> : null}
      <PrrrsTable
        className="PendingPrrrs"
        currentUser={this.props.currentUser}
        prrrs={prrrs}
        renderAdditionalHeaders={this.renderAdditionalHeaders}
        renderAdditionalCells={this.renderAdditionalCells}
      />
    </div>
  }
}

@jaredatron jaredatron merged commit 3f387be into LearnersGuild:master Jan 18, 2017
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.

None yet

2 participants