feat(Pagination): adds pager #254

AllenBW · 2018-02-26T21:04:19Z

closes #217

Given discussion in #217, gonna rescope this to just be the pager component, the rest isn't desired for implementation

https://AllenBW.github.io/patternfly-react/

☝️ 📚

coveralls · 2018-02-26T21:08:56Z

Pull Request Test Coverage Report for Build 815

3 of 11 (27.27%) changed or added relevant lines in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-0.5%) to 69.909%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/components/Pagination/Pager.js	3	11	27.27%

Totals
Change from base Build 804:	-0.5%
Covered Lines:	1087
Relevant Lines:	1400

💛 - Coveralls

coveralls · 2018-02-26T21:08:56Z

Pull Request Test Coverage Report for Build 977

11 of 11 (100.0%) changed or added relevant lines in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.2%) to 71.577%

Totals
Change from base Build 964:	0.2%
Covered Lines:	1255
Relevant Lines:	1585

💛 - Coveralls

jeff-phillips-18 · 2018-03-05T12:02:33Z

src/components/Pagination/Pager.js

+  return (
+    <ul className={classes}>
+      <li className={disablePrevious ? 'previous disabled' : 'previous'}>
+        <a


I know Patternfly uses anchors but I think we should use buttons. Using <Button bsStyle="link" > provides accessibility and putting disabled on the button uses the disabled cursor and doesn't show the clicks.

We may need to add scss/less to do this but I think it's the right thing to do. Thoughts? I think we should open an issue against Patternfly to support the use of buttons in this context.

Whether its buttons or anchors, the issue (of disabled buttons not showing appropriate cursor type) will persist so long as this thinggah lurks about https://github.com/patternfly/patternfly/blob/master/src/less/pager.less#L35

SO we can open an issue to fixup the cursor type on disabled.. buttttt doesn't necessarily require the change from anchors to buttons 🤔

Putting disabled on a button shows the disabled cursor.

Putting disabled on the anchor rather than just on the li actually disables the anchor (no click action). Adding an href would also address the accessibility. At a minimum, we should do that.

I've updated the storybook so yah can see, putting the disabled on the anchor does not remedy the situation as the aforementioned class overrides with cursor: default;

correct, no cursor change but it at least it doesn't handle the click (same should be done in pagination row for #253)

? I'm confused, it didn't handle the click before because of line 27, the presence or absence of disabled has no bearings on the triggering of an event 😕

OK, you're right. I was being mislead by my eyes and the fact that selecting the Previous button disabled knob is not disabling the previous button.

OH THE KNOB ISN'T WORKING great catch, on it.

AllenBW · 2018-03-07T15:21:17Z

Or maybe this doesn't need ux review cuz its a preexisting pattern ? 🤷‍♀️ Storybook has been restored to include pager.

priley86 · 2018-03-12T13:57:12Z

src/components/Pagination/Pager.js

+      <li className={disablePrevious ? 'previous disabled' : 'previous'}>
+        <a
+          href="#"
+          className={disablePrevious ? 'disabled' : ''}


we've been using className={classNames({'disabled': disablePrevious})} for this sort of thing

AllenBW · 2018-03-14T15:21:58Z

~~@jeff-phillips-18 @priley86 ok ok ok updated the snapshot, nowwwwwwww this is really g2g~~

priley86 · 2018-03-15T14:59:38Z

src/components/Pagination/Pager.js

+            }
+          }}
+        >
+          <span className="i fa fa-angle-left" />


oooo forgot one other thing... can we use <Icon name="angle-left"> or something like that?

otherwise i am totes mcgoates happy 👍

jeff-phillips-18 · 2018-03-15T15:07:22Z

@patternfly/patternfly-react-ux

maryclarke · 2018-03-15T15:39:04Z

Are both sets of buttons suppose to be affected by the checkboxes below for disabling? Only the mini ones seem to respond.

AllenBW · 2018-03-15T15:48:23Z

Nope only the mini ones @maryclarke

priley86 · 2018-03-19T00:45:24Z

@AllenBW to @maryclarke 's question... WDYT about making additional knobs for disabling the default buttons too (or using the same knobs)? Should be easy enough ;)

AllenBW · 2018-03-19T12:30:07Z

Oh oh apologies @maryclarke @priley86 didn't realize this was a request for change, updated storybook, both buttons now behave exactly the same 🙇‍♀️

priley86 · 2018-03-19T12:52:02Z

@AllenBW thanks!! 👍 this looks great to me...

@maryclarke anything else here?

priley86 · 2018-03-19T12:53:38Z

src/components/Pagination/Pager.js

@@ -0,0 +1,86 @@
+import React from 'react';
+import PropTypes from 'prop-types';
+import cx from 'classnames';


ugh one more thing now... we are rebasing everything to classNames soon in #274 ... do you mind updating this one last thing? I am so sorry 😆

maryclarke · 2018-03-19T12:56:16Z

Nothing else, LGTM!

AllenBW · 2018-03-19T15:25:19Z

@priley86 updated!

priley86 · 2018-03-19T19:33:26Z

awesome! you are the 💣 @AllenBW

jgiardino added the review label Feb 26, 2018

AllenBW removed the review label Feb 26, 2018

AllenBW force-pushed the #217-adds-pager-pagination branch from ac130e3 to 8ae0182 Compare February 27, 2018 14:48

AllenBW requested review from jeff-phillips-18 and priley86 March 2, 2018 20:19

AllenBW added the enhancement label Mar 2, 2018

AllenBW changed the title ~~[WIP]feat(Pagination): adds pager and simple pagination~~ feat(Pagination): adds pager and simple pagination Mar 2, 2018

AllenBW changed the title ~~feat(Pagination): adds pager and simple pagination~~ feat(Pagination): adds pager Mar 2, 2018

AllenBW added review labels Mar 2, 2018

AllenBW force-pushed the #217-adds-pager-pagination branch from 8ae0182 to 49d7fb7 Compare March 3, 2018 12:53

jeff-phillips-18 requested changes Mar 5, 2018

View reviewed changes

AllenBW force-pushed the #217-adds-pager-pagination branch 3 times, most recently from 9555bbc to 51d89a6 Compare March 5, 2018 19:56

jeff-phillips-18 previously approved these changes Mar 5, 2018

View reviewed changes

AllenBW dismissed jeff-phillips-18’s stale review via eed7986 March 6, 2018 12:54

AllenBW force-pushed the #217-adds-pager-pagination branch from 51d89a6 to eed7986 Compare March 6, 2018 12:54

AllenBW added ux review and removed css review labels Mar 6, 2018

AllenBW requested a review from sharvit March 7, 2018 14:33

priley86 previously approved these changes Mar 12, 2018

View reviewed changes

priley86 reviewed Mar 12, 2018

View reviewed changes

AllenBW dismissed priley86’s stale review via 67c3850 March 12, 2018 14:40

AllenBW force-pushed the #217-adds-pager-pagination branch from eed7986 to 67c3850 Compare March 12, 2018 14:40

jeff-phillips-18 previously approved these changes Mar 13, 2018

View reviewed changes

AllenBW dismissed jeff-phillips-18’s stale review via 21c0baa March 14, 2018 15:14

AllenBW force-pushed the #217-adds-pager-pagination branch from 67c3850 to 21c0baa Compare March 14, 2018 15:14

AllenBW force-pushed the #217-adds-pager-pagination branch 3 times, most recently from e144743 to 429e47f Compare March 15, 2018 14:42

priley86 reviewed Mar 15, 2018

View reviewed changes

AllenBW force-pushed the #217-adds-pager-pagination branch from 429e47f to 0f4a381 Compare March 15, 2018 15:32

AllenBW mentioned this pull request Mar 16, 2018

Docs(storybook): update to storybook 3.4 #269

Merged

AllenBW force-pushed the #217-adds-pager-pagination branch from 0f4a381 to 8d85cd6 Compare March 19, 2018 12:20

priley86 reviewed Mar 19, 2018

View reviewed changes

feat(Pagination): adds pager and simple pagination

ab0dd2f

AllenBW force-pushed the #217-adds-pager-pagination branch from 8d85cd6 to ab0dd2f Compare March 19, 2018 15:12

jeff-phillips-18 approved these changes Mar 19, 2018

View reviewed changes

priley86 merged commit 4fd67b4 into patternfly:master Mar 19, 2018

jgiardino removed the review label Mar 19, 2018

AllenBW deleted the #217-adds-pager-pagination branch March 19, 2018 20:22

feat(Pagination): adds pager #254

feat(Pagination): adds pager #254

Uh oh!

Conversation

AllenBW commented Feb 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Feb 26, 2018

Pull Request Test Coverage Report for Build 815

💛 - Coveralls

Uh oh!

coveralls commented Feb 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 977

💛 - Coveralls

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AllenBW commented Mar 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AllenBW commented Mar 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jeff-phillips-18 commented Mar 15, 2018

Uh oh!

maryclarke commented Mar 15, 2018

Uh oh!

AllenBW commented Mar 15, 2018

Uh oh!

priley86 commented Mar 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AllenBW commented Mar 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

priley86 commented Mar 19, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maryclarke commented Mar 19, 2018

Uh oh!

AllenBW commented Mar 19, 2018

Uh oh!

priley86 commented Mar 19, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

AllenBW commented Feb 26, 2018 •

edited

Loading

coveralls commented Feb 26, 2018 •

edited

Loading

AllenBW commented Mar 7, 2018 •

edited

Loading

AllenBW commented Mar 14, 2018 •

edited

Loading

priley86 commented Mar 19, 2018 •

edited

Loading

AllenBW commented Mar 19, 2018 •

edited

Loading