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

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

Merged
merged 3 commits into from Jul 26, 2019

Conversation

@karelhala
Copy link
Contributor

karelhala commented Jul 22, 2019

What:

When there is 0 items or negative number for pagination show 0th page out of 0 and disable paginating.
Screenshot from 2019-07-26 10-09-40

fix #2557

@karelhala karelhala requested review from jschuler, dlabaj, nicolethoen and tlabaj Jul 22, 2019
@karelhala karelhala force-pushed the karelhala:fix-zero-items-pagination branch from d24e798 to 9325a0d Jul 22, 2019
@patternfly-build

This comment has been minimized.

Copy link
Contributor

patternfly-build commented Jul 22, 2019

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

@@ -54,6 +56,7 @@ export class Navigation extends React.Component<NavigationProps, NavigationState
static defaultProps = {
className: '',
lastPage: 0,
firstIndex: 0,

This comment has been minimized.

Copy link
@redallen

redallen Jul 22, 2019

Contributor

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

This comment has been minimized.

Copy link
@karelhala

karelhala Jul 22, 2019

Author Contributor

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

@@ -36,7 +36,7 @@ class PaginationTop extends React.Component {
render() {
return (
<Pagination
itemCount={523}
itemCount={0}

This comment has been minimized.

Copy link
@redallen

redallen Jul 22, 2019

Contributor

Perhaps create another example?

@tlabaj tlabaj added the ux review label Jul 22, 2019
@karelhala karelhala force-pushed the karelhala:fix-zero-items-pagination branch from 55c2a02 to f640325 Jul 23, 2019
@karelhala karelhala requested a review from redallen Jul 23, 2019
@tlabaj tlabaj requested a review from mcarrano Jul 25, 2019
@tlabaj tlabaj added the PF4 label Jul 25, 2019
Copy link
Member

mcarrano left a comment

@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.

Screen Shot 2019-07-25 at 11 19 20 AM

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?

Screen Shot 2019-07-25 at 11 19 43 AM

@karelhala karelhala force-pushed the karelhala:fix-zero-items-pagination branch from f640325 to 406bb64 Jul 26, 2019
@karelhala

This comment has been minimized.

Copy link
Contributor Author

karelhala commented Jul 26, 2019

@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.

Copy link
Member

mcarrano left a comment

Looks great @karelhala !

@mcarrano mcarrano added ux approved and removed ux review labels Jul 26, 2019
@tlabaj
tlabaj approved these changes Jul 26, 2019
Copy link
Contributor

tlabaj left a comment

LGTM

@redallen redallen merged commit 7dee544 into patternfly:master Jul 26, 2019
8 checks passed
8 checks passed
ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: build_integration Your tests passed on CircleCI!
Details
ci/circleci: build_pf3_docs Your tests passed on CircleCI!
Details
ci/circleci: build_pf4_docs Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: test_jest_other Your tests passed on CircleCI!
Details
ci/circleci: test_jest_pf4 Your tests passed on CircleCI!
Details
ci/circleci: upload_docs Your tests passed on CircleCI!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.