FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix(PF4: Pagination): When 0 or negative number is passed to Pagintion show 0 pages by karelhala · Pull Request #2558 · patternfly/patternfly-react · GitHub

/ patternfly-react Public

fix(PF4: Pagination): When 0 or negative number is passed to Pagintion show 0 pages - #2558

Merged
redallen merged 3 commits into
patternfly:masterfrom
karelhala:fix-zero-items-pagination
Jul 26, 2019
ConversationCommits (3)ChecksFiles changed
Merged

fix(PF4: Pagination): When 0 or negative number is passed to Pagintion show 0 pages#2558
redallen merged 3 commits into
patternfly:masterfrom
karelhala:fix-zero-items-pagination

Conversation

karelhala commented Jul 22, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor

What:

When there is 0 items or negative number for pagination show 0th page out of 0 and disable paginating.

fix #2557

karelhala force-pushed the fix-zero-items-pagination branch from d24e798 to 9325a0d Compare July 22, 2019 12:08

patternfly-build commented Jul 22, 2019

Copy link
Copy Markdown
Collaborator

PatternFly-React preview: https://patternfly-react-pr-2558.surge.sh

redallen suggested changes Jul 22, 2019
View reviewed changes
Comment thread packages/patternfly-4/react-core/src/components/Pagination/Navigation.tsx Outdated
static defaultProps = {
className: '',
lastPage: 0,
firstIndex: 0,

redallen Jul 22, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Don't think we need this here. Did you mean firstPage?

karelhala Jul 22, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Yup, at first I passed firstIndex and renamed it to firstPage good catch!

Comment thread packages/patternfly-4/react-core/src/components/Pagination/Pagination.md Outdated
return (
<Pagination
itemCount={523}
itemCount={0}

redallen Jul 22, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Perhaps create another example?

karelhala force-pushed the fix-zero-items-pagination branch from 55c2a02 to f640325 Compare July 23, 2019 07:32
karelhala requested a review from redallen July 23, 2019 07:33
kmcfaul previously approved these changes Jul 24, 2019
View reviewed changes
tlabaj requested a review from mcarrano July 25, 2019 14:59
tlabaj added the PF4 label Jul 25, 2019
mcarrano reviewed Jul 25, 2019
View reviewed changes

mcarrano left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@karelhala I am seeing some unexpected behavior. When I initially open the component on Firefox I am seeing a red error border around the text field.

But if I refresh the page, this goes away. Then on either Firefox or Chrome I am able to place focus in the field and type. But no matter what I type the value is set to 1. Can the field just show 0 and be disabled?

karelhala force-pushed the fix-zero-items-pagination branch from f640325 to 406bb64 Compare July 26, 2019 08:09

karelhala commented Jul 26, 2019

Copy link
Copy Markdown
Contributor Author

@mcarrano good point! I disabled all navigation elements including per page (it doesn't make any sense to change per page if server returns 0 items) and fixed that firefox issue by adding correct min value to input.

mcarrano approved these changes Jul 26, 2019
View reviewed changes

mcarrano left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Looks great @karelhala !

tlabaj approved these changes Jul 26, 2019
View reviewed changes

tlabaj left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

redallen merged commit 7dee544 into patternfly:master Jul 26, 2019
tlabaj mentioned this pull request Oct 1, 2019
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

kmcfaul kmcfaul left review comments
jschuler Awaiting requested review from jschuler
dlabaj Awaiting requested review from dlabaj
nicolethoen Awaiting requested review from nicolethoen
+3 more reviewers
mcarrano mcarrano approved these changes
tlabaj tlabaj approved these changes
redallen redallen approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

0 or negative number of items for pagination

6 participants

Back | FazBrowse Home | New Git URL