Feat/react-pagination-component #1501
Conversation
terrance456
commented
Jul 20, 2024
terrance456
commented
🦋 Changeset statusNone or empty changeset found. This PR will not result any new releases. If you believe this is incorrect, please add a changeset by running You can also explicitly add an empty changeset by running: |
1 similar comment
🦋 Changeset statusNone or empty changeset found. This PR will not result any new releases. If you believe this is incorrect, please add a changeset by running You can also explicitly add an empty changeset by running: |
|
👋 Thanks for creating a pull request! 🚀 Checkout the storybook we've created for it: |
…seb-oss/green into feat/react-pagination-component
terrance456
requested review from
splashdust,
mario-subo,
ethan-qsh and
wallybeetles
| <li> | ||
| { | ||
| // eslint-disable-next-line jsx-a11y/anchor-is-valid | ||
| <a |
There was a problem hiding this comment.
Using an anchor tag w/o href causes problems with focus. The browser skips them all even if they role="button".
Is there a reason for using anchors tags?
Suggestion: Could these be an option to render button or a tags in this component? Using button will update the current page dynamically and using a would take you to a new page, user would also need to provide hrefs for each anchor tag.
There was a problem hiding this comment.
Totally agree in using button but the chlorophyll styling was added only for anchor specific and angular component is also using anchor tags. So wasn't sure if we should change it. Maybe we can add in tabIndex=0 to solve the issue here
There was a problem hiding this comment.
Yea, looks like this is how it was done in the angular component.
Ok for now! (until replaced with a core web component)
…seb-oss/green into feat/react-pagination-component
|
Some minor chlorophyll changes. This pull request should not be effected. |
