Skip to content

fix(ui-pagination): make Pagination wrap on smaller screen sizes and prevent scrollbars#1912

Merged
ToMESSKa merged 1 commit intomasterfrom
INSTUI-4461_pagination_shows_scrollbars
Apr 2, 2025
Merged

fix(ui-pagination): make Pagination wrap on smaller screen sizes and prevent scrollbars#1912
ToMESSKa merged 1 commit intomasterfrom
INSTUI-4461_pagination_shows_scrollbars

Conversation

@ToMESSKa
Copy link
Contributor

@ToMESSKa ToMESSKa commented Mar 20, 2025

INSTUI-4461

ISSUE:

  • on smaller screen sizes the Pagination has a horizontal scrollbar
  • in some cases it has a vertical scrollbar and page buttons and their focus ring aren't fully visible when the parent has no padding

TEST PLAN:

test no horizontal scrollbars:

  • open the examples in Pagination
  • click Inspect then switch to mobile device view
  • when exceeding the width of the container, the pagination numbers should wrap instead of a horizontal scrollbars appearing, and the new lines should be centered (similarly to the the legacy examples)

test no vertical scrollbars:

  • go to the first example, click Inspect and on the Styles tab remove the preview parent element's (class="css-kinttp-preview") padding
  • while the parent's padding removed, try when no margin prop is provided
  • while the parent's padding removed when no margin prop is provided, the component should have NO vertical scrollbars (make sure you hover over the component and try scrolling with the mouse too)
  • while the parent's padding removed and when no margin prop is provided, the click on the page buttons and make sure the buttons including their focus rings should remain fully visible (e.g. their top or bottom edges should not covered by the parent checkerboard container)

@ToMESSKa ToMESSKa self-assigned this Mar 20, 2025
@ToMESSKa ToMESSKa changed the title fix(ui-pagination): make Pagination wrap on smaller screen sizes fix(ui-pagination): make Pagination wrap on smaller screen sizes and prevent horizontal scrollbars Mar 20, 2025
@ToMESSKa ToMESSKa changed the title fix(ui-pagination): make Pagination wrap on smaller screen sizes and prevent horizontal scrollbars fix(ui-pagination): make Pagination wrap on smaller screen sizes and prevent scrollbars Mar 20, 2025
@github-actions
Copy link

github-actions bot commented Mar 20, 2025

PR Preview Action v1.6.0
Preview removed because the pull request was closed.
2025-04-02 07:12 UTC

@ToMESSKa ToMESSKa marked this pull request as draft March 20, 2025 10:15
@ToMESSKa ToMESSKa force-pushed the INSTUI-4461_pagination_shows_scrollbars branch 2 times, most recently from 5266f4f to abaed58 Compare March 21, 2025 15:44
@ToMESSKa ToMESSKa requested review from balzss and matyasf March 21, 2025 15:53
@ToMESSKa ToMESSKa marked this pull request as ready for review March 21, 2025 15:53
Copy link
Collaborator

@matyasf matyasf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice work!

display: 'inline-flex',
alignItems: 'center'
alignItems: 'center',
margin: '0.3rem'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this needed? and why this value?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of adding a margin here (so focusring isn't cut of), you could set the default margin to something like space8 (just make sure to use something from the new set of spacing tokens and not something like xSmall). this makes sure that when devs are implementing a design they don't have to deal with random margin around the component

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@balzss removed the 0.3rem margin and set a default 'space8' margin

@ToMESSKa ToMESSKa force-pushed the INSTUI-4461_pagination_shows_scrollbars branch from abaed58 to 161d765 Compare March 28, 2025 15:38
@ToMESSKa ToMESSKa force-pushed the INSTUI-4461_pagination_shows_scrollbars branch from 161d765 to cfd8c40 Compare March 28, 2025 15:45
@ToMESSKa ToMESSKa requested a review from balzss March 28, 2025 15:55
@ToMESSKa ToMESSKa merged commit 75e1540 into master Apr 2, 2025
10 of 11 checks passed
@ToMESSKa ToMESSKa deleted the INSTUI-4461_pagination_shows_scrollbars branch April 2, 2025 07:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants