Skip to content

Conversation

@Naghal
Copy link

@Naghal Naghal commented Jun 14, 2017

Added pagination system that allows the user to enter a number of row to display on the table. If nothing is specified, the default rows per page is 10.

Anthony Duhaime added 3 commits June 14, 2017 14:18
… first object if the dataset is not an array. Refactored some methods and variable's names
if (page >= 0 && page < this.props.pageCount) {
this.props.changeHandler(page);
}
this.props.currentPage === 0 ? this.setState({ previousDisabled: true }) : this.setState({ previousDisabled: false });
Copy link
Owner

Choose a reason for hiding this comment

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

This whole line could be replace with :

this.setState({ previousDisabled: !!this.props.currentPage });

in fact, the "previousDisabled" property should be replaced by and accessor such as :

isPreviousDisabled() {
    return !!this.props.currentPage;
}

Copy link
Owner

Choose a reason for hiding this comment

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

Same as nextDisabled

this.props.currentPage == this.props.pageCount - 2 ? this.setState({ nextDisabled: true }) : this.setState({ nextDisabled: false });
}

renderPagination() {
Copy link
Owner

Choose a reason for hiding this comment

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

This method should be divided into smaller ones

@@ -0,0 +1,20 @@
import React from 'react';

class TablePaginationPage extends React.Component {
Copy link
Owner

Choose a reason for hiding this comment

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

This could be a stateless component

export default class Tabledata extends Component {
constructor(props) {
super(props);
this._rowsToDisplay = this.props.rowsToDisplay ? this.props.rowsToDisplay : DEFAULT_ROWS_PER_PAGE;
Copy link
Owner

Choose a reason for hiding this comment

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

Could be an accessor instead

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.

2 participants