Refactor(Pagination): convert Pagination instances to PF4

[jenny-s51]

What's included

...
Closes #128

Example

Before:

After:

Updates issue/story

DISCOVERY-154

[codecov-commenter]

Codecov Report

❗ No coverage uploaded for pull request base (dev@ceca4e3). Click here to learn what that means.
The diff coverage is n/a.

@@          Coverage Diff           @@
##             dev     #129   +/-   ##
======================================
  Coverage       ?   74.74%           
======================================
  Files          ?       98           
  Lines          ?     3093           
  Branches       ?      852           
======================================
  Hits           ?     2312           
  Misses         ?      691           
  Partials       ?       90           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ceca4e3...4bd65d9. Read the comment docs.

[cdcabrera]

@jenny-s51
This looks like another Redux state piece we can simplify. I gave you enough to get started, there are a few pieces missing... like passing the values back to state, updating the constants file associated with viewPaginationConstants.js and the unit test viewOptionsReducer.test.js

We'll see how it goes, then move to the next set of tweaks

[jenny-s51]

@cdcabrera Thank you for all your awesome and helpful suggestions on this!! I think I've addressed your comments and just wanted to confirm... is this what you had in mind for the bottom pagination?

I had to make a couple more updates which included making the perPage menu open upwards, and removing the padding-bottom: 0 !important;
from the css class since visually we'd need to keep that bottom padding there! WDYT?

[cdcabrera]

@jenny-s51 heh, close, but not quite... paging needed to stay where it was to account for the infinite page scroll. We're just using the styling from bottom paging, not the location... the top styling unfortunately removes some of the features on smaller screens, limiting accessibility and general use, blame PF for that one.

Once you move the paging back to the top, with the bottom styling, while doing the required updates, I'll review

[jenny-s51]

@cdcabrera found a bug while making the most recent updates -- I tested out a whole bunch of z-index changes on our css (and did a whole bunch of experimenting in the PF docs), but can't seem to get that options menu out in front of the list content on smaller viewports!

This behavior currently exists in the PF docs too, so I opened an issue: patternfly/patternfly-react#7786

Definitely a weird one! Let me know what you think is the best course of action here. A potential workaround might be to have both top and bottom pagination for now?

[cdcabrera]

@jenny-s51 I can not recreate the z-index issue that you're seeing, even after

• multiple reinstalls,
• a clean install of just your repo and branch
  -making sure the latest dev branch changes were there
• making sure the NPM packages were updated
• checking other browsers
• and generally making sure my tests are coming back clean
• time has now been burned...

I recommend you

• run through a normal clean install
• investigate your tests
• double check your commit rebase
• pause the bug you opened on PF and acknowledge that you still need to investigate the issue Bug - Pagination - dropdown obstructed on dropDirection="down" in smaller viewports patternfly/patternfly-react#7786

Your unit snapshots are showing you exactly the z-index issue, HTML sequencing... there is a high probability this is an issue with your PR, not Patternfly.

If you continue to have issues, we can walk through what each of is seeing as a 1:1 after you've gone through the recommendations.

You've got more investigation to do

@jenny-s51 I was finally able to recreate this issue by doing another clean install, and dropping down to below 800pxish screen-width. That being said, this is a non-issue for this repository, specifically, because we're planning on updating the inventory list it is important to follow process.

We're still recommending you

• make sure your tests are updated, snapshots and all
• update your branch and commits
• and still acknowledge that this may be a non-issue for this repository on the PF issue
• Continue with doing the updates as is. Once that inventory update goes into place refactors will be happening based on UX-designs recommendations.

In the future, to avoid confusion and wasting time, it is recommended your branch and ALL unit tests be updated before calling out issues. It makes reviewing more difficult when things are partially updated.

[jenny-s51]

Thank you for your feedback and suggestions @cdcabrera - moving forward I will make sure to update tests/snapshots before raising an issue.

I've proceeded with the updates as-is, but this PR will include a buggy instance of the pagination dropdown on viewports < 800px, since we're using the bottom styling at the top. Based on your comments, it seems like that's fine for the time being?

We are going against design guidelines by implementing pagination this way, so I can close the PF issue, since this one specific use case probably doesn't need to be supported! As you said, it sounds like we will be doing some more refactoring to follow UX guidelines down the road!

this is a non-issue for this repository, specifically, because we're planning on updating the inventory list

Would you mind clarifying what you mean by this? Are you referring to #106 or something else?

[cdcabrera]

@jenny-s51 We are headed towards release points, 3 of them to be exact, 1 we've already completed.

This is going to sound harsh, but as a general developer you do not need the full picture, that is the role.

[cdcabrera]

Minor update

After that and the unit tests are updated, if any, and short of any other issues that pop up, we'll be good

[bclarhk]

z-index issue to be addressed with table update.
Everything else is groovy