fix(react/pagination): render page number twice when pagesUnknown
#12302
Conversation
According to carbon-design-system#12252, when pagesUnknown is , pagination render page number as same as selector which does not make sense to render this information twice. So, I change page number (pageText) to page label in front of the selector which makes more sense and make pagination looks cleaner. BREAKING CHANGE: typeof pageText change from function to string.
|
DCO Assistant Lite bot All contributors have signed the DCO. |
61130061
changed the title
pagesUnknown #12252pagesUnknown
✅ Deploy Preview for carbon-components-react ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site settings. |
|
I have read the DCO document and I hereby sign the DCO. |
✅ Deploy Preview for carbon-elements ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
|
@61130061 We'll need to keep this a function so that teams can pass down a function for translation keys. I think this can be fixed just by passing down .#{$prefix}--pagination__right .#{$prefix}--pagination__text:empty {
margin: 0;
} |
There was a problem hiding this comment.
One small suggested change, but overall this looks good to me. Thanks for contributing this fix!
@61130061 There are conflicts that need resolved before this can be merged
Co-authored-by: Taylor Jones <[email protected]>
…m/61130061/carbon into fix/12252-pagination-pagesUnknown
|
@tay1orjones Thank you for your approval and suggestion! I commit another |
|
@61130061 There's still some merge conflicts here |
…x/12252-pagination-pagesUnknown
|
I just fixed the merge conflicts. @tw15egan the merge was a bit hairy, but I think I got it all. Could you review again once the deploy preview is updated? |
| @@ -0,0 +1,421 @@ | |||
| /** | |||
There was a problem hiding this comment.
I believe this file was removed in #12115
packages/react/src/components/Pagination/next/__tests__/Pagination-test.js
Outdated
Show resolved
Hide resolved
|
@tw15egan Thanks for the catch - I just pushed a commit to remove those files |
Closes #12252
According to #12252, when pagesUnknown is
true, pagination render page number as same as selector which does not make sense to render this information twice as shown in figure before. So, I change page number (pageText) to page label in front of the selector which makes more sense and make pagination looks cleaner. Also, changepageTextprop fromfunctiontostringso user can change the page label.before:
after:
Changelog
New
Changed
pageTextdefault change topagefunctiontostringtruechange to expect justpagesince switch from page number to page label as shown in figuresselectortopagination__rightRemoved
Testing / Reviewing