adxmantium commented 4 years ago

Describe your PR

In #245 I fixed some pagination hover/focus styling issues, but there was some apparent tabbing issues (as noted by reviewers 🙏 ). Basically, it takes two tab presses in order to get to the next/prev pagination link - it should only take one, which is what this PR addresses.

As @magnificode mentions, this is caused by the Button component (<button>) being wrapped in an Link component (<a>) and both of these elements are naturally tabbable elements, so the fix was to remove one of them - or better yet - combine them! 😮 Was able to maintain the Button styles, and turn it into a clickable link by setting it as={Link} and removing the <Link> component. I made this change for the pagination links and the pagination arrows as well.

While I was in here, I fixed up two minor things as well related to pagination:

In the dev tools console, there was an error regarding isDisabled is a required prop in PaginationArrow, but it didn't look like that prop was being used at all, so I removed it from propTypes so that it didn't continue to yell about it being required.
The getPageLink func was returning a url with a ? even if there were no params (like when page=1) bc getUpdatedSearchParams returns an empty string when page=1. So I just added a condition to only return the url with ? if it has param.

Pages/Interfaces that will change

Affects tabbing on Pagination links Before: it took two tab presses to nav to the next pagination link After: it will take one tab press to nav to next pagination link

Screenshots / video of changes

pic of what screen readers pic up (via firefox)	vid of tabbing once to nav from link to link
https://share.getcloudapp.com/Wnubb5Kj	https://share.getcloudapp.com/E0uzzNKX

pagination link for page=1 (before)	pagination link for page=1 (after)
https://share.getcloudapp.com/rRulldbJ	https://share.getcloudapp.com/2Nu55QlZ

Steps to test

go to /businesses
click on a pagination page link
scroll back down to pagination component and start tabbing - should only only take one tab press now to move to the next link

Additional Notes

I used this a11y style guide as a guide.
So TIL that firefox actually defers tab controls to the operating system and on MacOS it's not enabled by default, so there are two solutions to enabling tabbing to links in Firefox as outlined in this stackoverflow thread.
Some additional info from MDN on tabfocus