Skip to content
This repository was archived by the owner on Mar 16, 2022. It is now read-only.

Conversation

eburgos
Copy link
Contributor

@eburgos eburgos commented Jan 24, 2018

Refers to #62

Disputes grid with pagination. I hope it helps.

Regards,

# Conflicts:
#	src/Components/DisputesTable/GridContent/index.js
@coveralls
Copy link

coveralls commented Jan 24, 2018

Coverage Status

Coverage increased (+3.4%) to 5.619% when pulling 6669632 on eburgos:disputes-paginated into 72c32b6 on kleros:develop.

Changed code to meet ESLint issues. Added a test to not
decrease overall coverage.
currentPage: 1
}
this.paginateHandler = this.paginateHandler.bind(this)
this.paginateInputHandler = this.paginateInputHandler.bind(this)
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Inside those method I'm settings this.state. If I don't bind then I won't have this available (would be undefined). Please let me know if you consider a workaround.

componentWillMount () {
this.props.getDataDisputes()
}

paginateHandler (event) {
Copy link
Contributor

Choose a reason for hiding this comment

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

could use the functional syntax for your own function like paginateHandler = e => (this.setState({ currentPage: Number(e.target.dataset.pageNumber) }))

this.setState({
currentPage: Number(event.target.value)
})
}
Copy link
Contributor

Choose a reason for hiding this comment

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

idems

})
const currentRows = list.slice(indexOfFirstRow, indexOfLastRow)

const totalPages = Math.ceil(list.length / rowsPerPage) || 1
Copy link
Contributor

Choose a reason for hiding this comment

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

can you create a function like const currentRows = (list, indexOfLastRow, indexOfFirstRow) in the utils folder

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I couldn't find a utils folder. Perhaps you meant "helpers" ?

@n1c01a5
Copy link
Contributor

n1c01a5 commented Jan 25, 2018

and can you hide the "paginator" if there are not enough disputes

Pagination now shows only if there is more than 1 page.
Added story to storybook showing dispute table with no pagination.
Styled the paginator a bit to make it look more like the theme.
Added test on the grid not showing pagination.
@eburgos
Copy link
Contributor Author

eburgos commented Jan 26, 2018

@n1c01a5 Fixed most of the stuff you requested.

  1. I couldn't find that "utils" folder, perhaps you meant the "helpers" one
  2. Regarding the .bind(this) I need to use "this" inside the function so I can't skip that bind. Unless there is another way of achieving this that I'm not aware.

@n1c01a5
Copy link
Contributor

n1c01a5 commented Jan 29, 2018

@eburgos yes it's the helpers folder and yes I think we can skip it but it's about the refactoring. We can see that later. Tnx for your contribution.

@n1c01a5 n1c01a5 merged commit edb8cc9 into kleros:develop Jan 29, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants