carbon-design-system / carbon

A design system built by IBM
https://www.carbondesignsystem.com
Apache License 2.0
7.73k stars 1.79k forks source link

[Bug]: Adjacent ellipses in PaginationNav #15830

Open cmarcink opened 6 months ago

cmarcink commented 6 months ago

Package

@carbon/react

Browser

Chrome, Firefox

Package version

v1.50.0

React version

v17.0.2

Description

Should not have adjacent ellipses in pagination. image

Reproduction/example

https://react.carbondesignsystem.com/?path=/story/components-paginationnav--playground&args=itemsShown:4;totalItems:10

Steps to reproduce

  1. Render with props itemsShown = 4, totalItems = something >= 10
  2. Select a page from the ellipses
  3. Select the first page
  4. Notice the adjacent ellipses

image

Suggested Severity

None

Application/PAL

Cloud Pak for AIOps

Code of Conduct

tay1orjones commented 6 months ago

Thanks for reporting, can confirm this looks off. I think something's wrong within this logic here: https://github.com/carbon-design-system/carbon/blob/754ff7965e3693bd445e8a4d46b4b31bc7e91417/packages/react/src/components/PaginationNav/PaginationNav.tsx#L454-L466

tay1orjones commented 6 months ago

It's clear we don't want the overflows next to one another, but this brings up an interesting question for me: what do we want to happen? Should overflows be limited in certain cases beyond what we already have? What should we be aiming for when we fix this?

@carbon-design-system/design do you have any thoughts?

hollyos commented 5 months ago

@tay1orjones It looks like this might be a bug with how the PaginationNav page selectors are working. The double ellipsis only shows when you navigate to page 1 from a page in the middle of the range (only when there are 2 ellipsis around the number). If you use the arrows to go up and down, the UI updates correctly to have 1 and 2 showing when 1 is selected.

Screenshot 2024-04-01 at 11 04 39

That said, I noticed that the UI should be more uniform in terms of which pages are visible for each active page, regardless of selection method. For example, when clicking on page 10 while page 1 is active, the page changes to 10, but 1 & 2 remain visible. Meanwhile, 8, 9, and 10 are visible with the arrows.

Page 1 Active

Screenshot 2024-04-01 at 10 43 00 Screenshot 2024-04-01 at 10 43 08

Page 10 Active

Screenshot 2024-04-01 at 10 42 52 Screenshot 2024-04-01 at 10 43 15

Seems the solution here would be to get the page list visually consistent for visible pages regardless of the selection method for each active page.


Outside of that fix, I have a couple of suggestions that could be implemented in a few steps.

  1. Change itemsShown to only target the number nav items. Then, update the minimum allowed value to 3.

    This is a potentially visually breaking change for some users' UIs. It would add 2 new items to current UIs in some cases.

    3

    active page example
    1 1 2 ... 10
    2 1 2 ... 10
    3 1 ... 3 ... 10
    9 1 ... 9 10
    10 1 ... 9 10

    4

    active page example
    1 1 2 3 ... 10
    2 1 2 3 ... 10
    3 1 ... 3 4 ... 10
    7 1 ... 7 8 ... 10
    8 1 ... 8 9 10
    10 1 ... 8 9 10

    * Examples here have been updated to incorporate the below changes. The boundary numbers in the examples above do not represent the current implementation.

  2. Add 2 new props; boundaryCount & siblingCount

    prop description
    boundaryCount Number of pages visible at the beginning and end.
    siblingCount Number of pages visible before and after the current page.

    * These should be implemented in separate PRs.

    These would provide more customization in the presentation of the pagination, allowing for control over which pages get displayed before, inside, and after the ellipsis.

janhassel commented 5 months ago

When my design team was originally working on this we prototyped a few approaches, including the one outlined by @hollyos.

What we noticed during internal user testing was that it is extremely distracting / confusing to users when the width of the component changes while flipping through the pages. It also defeated the reason why we needed such a compact component since if there is enough space to show another item, why not show it initially.

I'm referring to these examples:

◂ 1 2 … 9 ▸
◂ 1 … 3 … 9 ▸
◂ 1 … 8 9 ▸
◂ 1 2 3 … 9 ▸
◂ 1 … 3 4 … 9 ▸
◂ 1 … 7 8 9 ▸

That's why we opted to include the overflows in the number of items shown when contributing the implementation.

laurenmrice commented 3 months ago

I feel like Holly and Jan's approach for this makes sense, to try to keep the page list visually consistent for visible pages regardless of the selection method for each active page, and this solution would resolve the problem of having the double ellipsis appearing next to each other in the pagination.

Adding this as props is also a good idea, so people can use this if they need more control for their pagination needs.