Skip to content
This repository has been archived by the owner on May 24, 2024. It is now read-only.

[terra-paginator] Add support for unknown total page count in the progressive paginator #3190

Merged
merged 17 commits into from
Nov 18, 2020
+554 −172
Merged

[terra-paginator] Add support for unknown total page count in the progressive paginator #3190

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
6daedf1
Changed Progressive Paginattor to Support unknown page count data
supreethmr Oct 6, 2020
9328638
fixed unknown item count issue
supreethmr Oct 13, 2020
05d3e83
fixing lint errors
supreethmr Oct 13, 2020
1d02fc6
Merge branch 'main' into paginator_wo_totalcount
supreethmr Oct 14, 2020
9bcdd58
Merge branch 'main' into paginator_wo_totalcount
supreethmr Oct 19, 2020
53e3382
Merge branch 'main' into paginator_wo_totalcount
supreethmr Oct 21, 2020
877dbe5
Merge branch 'main' into paginator_wo_totalcount
supreethmr Oct 27, 2020
1440dc0
Removed Terra-form-input
supreethmr Oct 29, 2020
e85b208
Merge branch 'main' into paginator_wo_totalcount
supreethmr Oct 29, 2020
38eacfa
Merge branch 'main' into paginator_wo_totalcount
supreethmr Oct 30, 2020
d0e5ef4
addressed code review comments
supreethmr Nov 5, 2020
b7c3d93
Apply suggestions from code review
mjhenkes Nov 9, 2020
d069686
Merge branch 'main' into paginator_wo_totalcount
supreethmr Nov 11, 2020
22baadd
made controlled paginator example
supreethmr Nov 16, 2020
09971f7
Merge branch 'main' into paginator_wo_totalcount
supreethmr Nov 16, 2020
9bd7ee2
Merge branch 'main' into paginator_wo_totalcount
supreethmr Nov 18, 2020
cc5f214
fixed linter
supreethmr Nov 18, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions packages/terra-paginator/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
# Changelog

## Unreleased
* Changed
* Changed progressive paginator to support unknown total page count.
* Replaced `Terra.it()` with `Terra.validates.element()`.

* Fixed
* Fixed paginator not considering `totalCount` when `itemCountPerPage` was not provided.

## 2.64.0 - (October 27, 2020)

Expand Down
1 change: 0 additions & 1 deletion packages/terra-paginator/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
"prop-types": "^15.5.8",
"terra-button": "^3.48.0",
"terra-dialog": "^2.51.0",
"terra-form-input": "^4.6.0",
"terra-icon": "^3.37.0",
"terra-mixins": "^1.38.0",
"terra-responsive-element": "^5.24.0",
Expand Down
39 changes: 22 additions & 17 deletions packages/terra-paginator/src/ControlledPaginator.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ class Paginator extends React.Component {
super(props);

this.handlePageChange = this.handlePageChange.bind(this);
this.hasNavContext = this.hasNavContext.bind(this);
this.buildPageButtons = this.buildPageButtons.bind(this);
this.reducedPaginator = this.reducedPaginator.bind(this);
this.setPaginator = this.setPaginator.bind(this);
Expand Down Expand Up @@ -114,21 +113,22 @@ class Paginator extends React.Component {
return pageButtons;
}

hasNavContext() {
return this.props.totalCount && this.props.itemCountPerPage;
}
Copy link
Contributor

@StephenEsser StephenEsser Nov 2, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

This seems like it played an important role in the logic statements below, can you explain why the check is not necessary? Is this no longer necessary to account for an unknown total count? totalCount and itemCountPerPage are marked as required props in the declaration above. Is that still correct for the controlled paginator?

Copy link
Contributor Author

@supreethmr supreethmr Nov 3, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

This check was removed to ensure paginator to consider totalCount when itemCountPerPage was not provided. I missed to notice that itemCountPerPage was required prop in controlled-paginator.

This is bit confusing so with existing design only paginator supports unknown Page count and controlled-paginator doesn’t support unknown Page count...?

If this is expected behavior then I need to add this check back OR if controlled-paginator should also updated to support unknown page counts then we need to change props totalCount and itemCountPerPage to optional similar to progressive-paginator.

@neilpfeiffer Should controlled-paginator also be updated to support unknown page-counts...?? with Current design totalCount is required prop in controlled-paginator.

Copy link
Contributor

Choose a reason for hiding this comment

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

Spoke with @StephenEsser, @neilpfeiffer, and others and we should support unknown page counts and we should go ahead and make totalCount and itemCountPerPage optional.


defaultPaginator() {
const theme = this.context;
const totalPages = calculatePages(this.props.totalCount, this.props.itemCountPerPage);
const { selectedPage, intl } = this.props;
const {
selectedPage,
intl,
totalCount,
itemCountPerPage,
} = this.props;
const totalPages = calculatePages(totalCount, itemCountPerPage);
const previousPageIndex = selectedPage === 1 ? 1 : selectedPage - 1;
const nextPageIndex = selectedPage === totalPages ? totalPages : selectedPage + 1;

const fullView = (
<div className={cx('paginator', !this.hasNavContext() && 'pageless', theme.className)}>
<div className={cx('paginator', !totalCount && 'pageless', theme.className)}>
{
this.hasNavContext() && (
totalCount && (
<PaginatorButton
ariaDisabled={selectedPage === 1}
ariaLabel={intl.formatMessage({ id: 'Terra.paginator.first' })}
Expand All @@ -152,7 +152,7 @@ class Paginator extends React.Component {
<span className={cx('icon')} />
{intl.formatMessage({ id: 'Terra.paginator.previous' })}
</PaginatorButton>
{this.hasNavContext() && this.buildPageButtons(totalPages, this.handlePageChange)}
{totalCount && this.buildPageButtons(totalPages, this.handlePageChange)}
<PaginatorButton
ariaDisabled={selectedPage === totalPages}
ariaLabel={intl.formatMessage({ id: 'Terra.paginator.next' })}
Expand All @@ -165,7 +165,7 @@ class Paginator extends React.Component {
<span className={cx('icon')} />
</PaginatorButton>
{
this.hasNavContext() && (
totalCount && (
<PaginatorButton
ariaDisabled={selectedPage === totalPages}
ariaLabel={intl.formatMessage({ id: 'Terra.paginator.last' })}
Expand All @@ -186,15 +186,20 @@ class Paginator extends React.Component {

reducedPaginator() {
const theme = this.context;
const totalPages = calculatePages(this.props.totalCount, this.props.itemCountPerPage);
const { selectedPage, intl } = this.props;
const {
selectedPage,
intl,
totalCount,
itemCountPerPage,
} = this.props;
const totalPages = calculatePages(totalCount, itemCountPerPage);
const previousPageIndex = selectedPage === 1 ? 1 : selectedPage - 1;
const nextPageIndex = selectedPage === totalPages ? totalPages : selectedPage + 1;

const reducedView = (
<div className={cx('paginator', !this.hasNavContext() && 'pageless', theme.className)} role="navigation" aria-label="pagination">
<div className={cx('paginator', !totalCount && 'pageless', theme.className)} role="navigation" aria-label="pagination">
{
this.hasNavContext() && (
totalCount && (
<PaginatorButton
ariaDisabled={selectedPage === 1}
ariaLabel={intl.formatMessage({ id: 'Terra.paginator.first' })}
Expand All @@ -218,7 +223,7 @@ class Paginator extends React.Component {
<span className={cx('icon')} />
{intl.formatMessage({ id: 'Terra.paginator.previous' })}
</PaginatorButton>
{this.hasNavContext() && intl.formatMessage({ id: 'Terra.paginator.pageIndex' }, { pageNumber: selectedPage })}
{totalCount && intl.formatMessage({ id: 'Terra.paginator.pageIndex' }, { pageNumber: selectedPage })}
<PaginatorButton
ariaDisabled={selectedPage === totalPages}
ariaLabel={intl.formatMessage({ id: 'Terra.paginator.next' })}
Expand All @@ -231,7 +236,7 @@ class Paginator extends React.Component {
<span className={cx('icon')} />
</PaginatorButton>
{
this.hasNavContext() && (
totalCount && (
<PaginatorButton
ariaDisabled={selectedPage === totalPages}
ariaLabel={intl.formatMessage({ id: 'Terra.paginator.last' })}
Expand Down
132 changes: 80 additions & 52 deletions packages/terra-paginator/src/ControlledProgressivePaginator.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,19 +24,19 @@ const propTypes = {
/**
* Total number of all items being paginated.
*/
totalCount: PropTypes.number.isRequired,
totalCount: PropTypes.number,
/**
* Total number of items per page.
*/
itemCountPerPage: PropTypes.number.isRequired,
itemCountPerPage: PropTypes.number,
/**
* @private
* The intl object to be injected for translations.
*/
intl: intlShape.isRequired,
};

class ProgressivePaginator extends React.Component {
class ControlledProgressivePaginator extends React.Component {
constructor(props) {
super(props);

Expand Down Expand Up @@ -68,27 +68,37 @@ class ProgressivePaginator extends React.Component {

defaultProgressivePaginator() {
const theme = this.context;
const totalPages = calculatePages(this.props.totalCount, this.props.itemCountPerPage);
const { selectedPage, intl } = this.props;
const {
selectedPage,
intl,
totalCount,
itemCountPerPage,
} = this.props;
const totalPages = (totalCount) ? calculatePages(totalCount, itemCountPerPage) : 0;
const previousPageIndex = selectedPage === 1 ? 1 : selectedPage - 1;
const nextPageIndex = selectedPage === totalPages ? totalPages : selectedPage + 1;

return (
<div className={cx('paginator', 'progressive', theme.className)} role="navigation" aria-label="pagination">
<div>
{intl.formatMessage({ id: 'Terra.paginator.pageCount' }, { pageNumber: selectedPage, pageNumberTotal: totalPages })}
{(totalCount) ? intl.formatMessage({ id: 'Terra.paginator.pageCount' }, { pageNumber: selectedPage, pageNumberTotal: totalPages })
: intl.formatMessage({ id: 'Terra.paginator.pageIndex' }, { pageNumber: selectedPage })}
</div>
<div>
<PaginatorButton
ariaDisabled={selectedPage === 1}
ariaLabel={intl.formatMessage({ id: 'Terra.paginator.first' })}
className={cx(['nav-link', selectedPage === 1 ? 'is-disabled' : null])}
disabled={selectedPage === 1}
onClick={this.handlePageChange(1)}
onKeyDown={this.handlePageChange(1)}
>
{intl.formatMessage({ id: 'Terra.paginator.first' })}
</PaginatorButton>
{
totalCount && (
<PaginatorButton
ariaDisabled={selectedPage === 1}
ariaLabel={intl.formatMessage({ id: 'Terra.paginator.first' })}
className={cx(['nav-link', selectedPage === 1 ? 'is-disabled' : null])}
disabled={selectedPage === 1}
onClick={this.handlePageChange(1)}
onKeyDown={this.handlePageChange(1)}
>
{intl.formatMessage({ id: 'Terra.paginator.first' })}
</PaginatorButton>
)
}
<PaginatorButton
ariaDisabled={selectedPage === 1}
ariaLabel={intl.formatMessage({ id: 'Terra.paginator.previous' })}
Expand All @@ -111,41 +121,54 @@ class ProgressivePaginator extends React.Component {
{intl.formatMessage({ id: 'Terra.paginator.next' })}
<span className={cx('icon')} />
</PaginatorButton>
<PaginatorButton
ariaDisabled={selectedPage === totalPages}
ariaLabel={intl.formatMessage({ id: 'Terra.paginator.last' })}
className={cx(['nav-link', selectedPage === totalPages ? 'is-disabled' : null])}
disabled={selectedPage === totalPages}
onClick={this.handlePageChange(totalPages)}
onKeyDown={this.handlePageChange(totalPages)}
>
{intl.formatMessage({ id: 'Terra.paginator.last' })}
</PaginatorButton>
{
(totalCount) && (
<PaginatorButton
ariaDisabled={selectedPage === totalPages}
ariaLabel={intl.formatMessage({ id: 'Terra.paginator.last' })}
className={cx(['nav-link', selectedPage === totalPages ? 'is-disabled' : null])}
disabled={selectedPage === totalPages}
onClick={this.handlePageChange(totalPages)}
onKeyDown={this.handlePageChange(totalPages)}
>
{intl.formatMessage({ id: 'Terra.paginator.last' })}
</PaginatorButton>
)
}
</div>
</div>
);
}

reducedProgressivePaginator() {
const theme = this.context;
const totalPages = calculatePages(this.props.totalCount, this.props.itemCountPerPage);
const { selectedPage, intl } = this.props;
const {
selectedPage,
intl,
totalCount,
itemCountPerPage,
} = this.props;
const totalPages = (totalCount) ? calculatePages(totalCount, itemCountPerPage) : 0;
const previousPageIndex = selectedPage === 1 ? 1 : selectedPage - 1;
const nextPageIndex = selectedPage === totalPages ? totalPages : selectedPage + 1;

return (
<div className={cx('paginator', theme.className)} role="navigation" aria-label="pagination">
<div>
<PaginatorButton
ariaDisabled={selectedPage === 1}
ariaLabel={intl.formatMessage({ id: 'Terra.paginator.first' })}
className={cx(['nav-link', selectedPage === 1 ? 'is-disabled' : null])}
disabled={selectedPage === 1}
onClick={this.handlePageChange(1)}
onKeyDown={this.handlePageChange(1)}
>
{intl.formatMessage({ id: 'Terra.paginator.first' })}
</PaginatorButton>
{
(totalCount) && (
<PaginatorButton
ariaDisabled={selectedPage === 1}
ariaLabel={intl.formatMessage({ id: 'Terra.paginator.first' })}
className={cx(['nav-link', selectedPage === 1 ? 'is-disabled' : null])}
disabled={selectedPage === 1}
onClick={this.handlePageChange(1)}
onKeyDown={this.handlePageChange(1)}
>
{intl.formatMessage({ id: 'Terra.paginator.first' })}
</PaginatorButton>
)
}
<PaginatorButton
ariaDisabled={selectedPage === 1}
ariaLabel={intl.formatMessage({ id: 'Terra.paginator.previous' })}
Expand All @@ -159,7 +182,8 @@ class ProgressivePaginator extends React.Component {
</PaginatorButton>
</div>
<div>
{intl.formatMessage({ id: 'Terra.paginator.pageCount' }, { pageNumber: selectedPage, pageNumberTotal: totalPages })}
{(totalCount) ? intl.formatMessage({ id: 'Terra.paginator.pageCount' }, { pageNumber: selectedPage, pageNumberTotal: totalPages })
: intl.formatMessage({ id: 'Terra.paginator.pageIndex' }, { pageNumber: selectedPage })}
</div>
<div>
<PaginatorButton
Expand All @@ -173,16 +197,20 @@ class ProgressivePaginator extends React.Component {
<VisuallyHiddenText text={intl.formatMessage({ id: 'Terra.paginator.next' })} />
<span className={cx('icon')} />
</PaginatorButton>
<PaginatorButton
ariaDisabled={selectedPage === totalPages}
ariaLabel={intl.formatMessage({ id: 'Terra.paginator.last' })}
className={cx(['nav-link', selectedPage === totalPages ? 'is-disabled' : null])}
disabled={selectedPage === totalPages}
onClick={this.handlePageChange(totalPages)}
onKeyDown={this.handlePageChange(totalPages)}
>
{intl.formatMessage({ id: 'Terra.paginator.last' })}
</PaginatorButton>
{
(totalCount) && (
<PaginatorButton
ariaDisabled={selectedPage === totalPages}
ariaLabel={intl.formatMessage({ id: 'Terra.paginator.last' })}
className={cx(['nav-link', selectedPage === totalPages ? 'is-disabled' : null])}
disabled={selectedPage === totalPages}
onClick={this.handlePageChange(totalPages)}
onKeyDown={this.handlePageChange(totalPages)}
>
{intl.formatMessage({ id: 'Terra.paginator.last' })}
</PaginatorButton>
)
}
</div>
</div>
);
Expand All @@ -199,7 +227,7 @@ class ProgressivePaginator extends React.Component {
}
}

ProgressivePaginator.propTypes = propTypes;
ProgressivePaginator.contextType = ThemeContext;
ControlledProgressivePaginator.propTypes = propTypes;
ControlledProgressivePaginator.contextType = ThemeContext;

export default injectIntl(ProgressivePaginator);
export default injectIntl(ControlledProgressivePaginator);
Loading