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(pagination-nav): add data-page attributes #6437

Merged
merged 2 commits into from
Jul 10, 2020

Conversation

janhassel
Copy link
Member

Adds data-page attribute to the page buttons and select options in the PaginationNav component. This is to follow the vanilla implementation (https://the-carbon-components.netlify.app/?nav=pagination-nav) and to give developers better access for advanced behaviour that relies on handling the DOM node (such as adding a custom css rule or other data-* attributes.

I'm so sorry for the extra request, @joshblack @aledavila! Our intended implementation was counting on data-page and yet somehow it slipped through my first PR.

Changelog

New

  • Added data-page="<page-number>" to button.bx--pagination-nav__page and select.bx--pagination-nav__page option

Testing / Reviewing

  • Automated tests should make sure no behaviour is changed. Verify data-page attribute holds the actual page number, not the index.

@janhassel janhassel requested a review from a team as a code owner July 10, 2020 12:31
@ghost ghost requested review from dakahn and tw15egan July 10, 2020 12:31
@netlify
Copy link

netlify bot commented Jul 10, 2020

Deploy preview for carbon-elements ready!

Built with commit a545f36

https://deploy-preview-6437--carbon-elements.netlify.app

@netlify
Copy link

netlify bot commented Jul 10, 2020

Deploy preview for carbon-components-react ready!

Built without sensitive environment variables with commit a545f36

https://deploy-preview-6437--carbon-components-react.netlify.app

@kodiakhq kodiakhq bot merged commit 8c84f34 into carbon-design-system:master Jul 10, 2020
@janhassel janhassel deleted the pagination-nav-fix branch July 21, 2020 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants