-
Notifications
You must be signed in to change notification settings - Fork 21
feat(Pagination): add Pagination component aligned with Cyber skin #1555
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
base: master
Are you sure you want to change the base?
Changes from 20 commits
a3127a8
6d5331f
08e0990
aa4aada
d30e104
a9064f8
ad0df1c
c5c4061
8e9de99
f68dd1f
588bf02
ba5d146
7a4f114
857005b
b932a73
c4026f3
cedf287
fbb08ae
b3f213c
337bb8b
7d7f4de
604be41
cbc31aa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| import {openStoryPage, screen} from '../test-utils'; | ||
|
|
||
| import type {Device, StoryArgs} from '../test-utils'; | ||
|
|
||
| const STORY_ID = 'components-pagination--default'; | ||
|
|
||
| const CASES: ReadonlyArray<{name: string; args: StoryArgs}> = [ | ||
| {name: 'Default', args: {totalPages: 9, currentPage: 3}}, | ||
| {name: 'FirstPage', args: {totalPages: 10, currentPage: 1}}, | ||
| {name: 'LastPage', args: {totalPages: 10, currentPage: 10}}, | ||
| {name: 'WithEllipsis', args: {totalPages: 20, currentPage: 10}}, | ||
| { | ||
| name: 'NavOnlyResponsive', | ||
| args: {totalPages: 10, currentPage: 5, hidePageList: true}, | ||
| }, | ||
| { | ||
| name: 'PagesOnly', | ||
| args: {totalPages: 5, currentPage: 3, hideNavigationControls: true}, | ||
| }, | ||
| { | ||
| name: 'IconOnlyControls', | ||
| args: {totalPages: 10, currentPage: 5, mode: 'iconOnly', hidePageList: true}, | ||
| }, | ||
| { | ||
| name: 'NextChapterLink', | ||
| args: { | ||
| totalPages: 10, | ||
| currentPage: 1, | ||
| hidePageList: true, | ||
| navLeftLabel: 'Anterior capítulo', | ||
| navRightLabel: 'Siguiente capítulo', | ||
| }, | ||
| }, | ||
| ]; | ||
|
|
||
| const DEVICES: ReadonlyArray<Device> = ['DESKTOP', 'MOBILE_IOS']; | ||
|
|
||
| const TABLE = CASES.flatMap((c) => DEVICES.map((device) => ({...c, device}))); | ||
|
|
||
| test.each(TABLE)('Pagination $name - $device', async ({args, device}) => { | ||
| await openStoryPage({id: STORY_ID, device, args}); | ||
|
|
||
| const pagination = await screen.findByTestId('Pagination'); | ||
| const image = await pagination.screenshot(); | ||
| expect(image).toMatchImageSnapshot(); | ||
| }); | ||
|
|
||
| test('Pagination CompactView - MOBILE_IOS_SMALL', async () => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. are we sure we want this? by fiddling with the component, I see that even smaller viewports could take advantage of a single line, maybe. should we talk to design maybe?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @aweell @AnaMontes11 what do you think? |
||
| await openStoryPage({ | ||
| id: STORY_ID, | ||
| device: 'MOBILE_IOS_SMALL', | ||
| args: {totalPages: 50, currentPage: 24}, | ||
| }); | ||
|
|
||
| const pagination = await screen.findByTestId('Pagination'); | ||
| const image = await pagination.screenshot(); | ||
| expect(image).toMatchImageSnapshot(); | ||
| }); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should have added controls instead of creating a lot of examples like the rest of the stories. Perhaps this is something we should add to AGENTS.md so it is the default behaviour
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 100% agreed. we can improve this by exploiting controls. that way it will be improving the DX! |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| import * as React from 'react'; | ||
| import {Pagination} from '..'; | ||
|
|
||
| export default { | ||
| title: 'Components/Pagination', | ||
| argTypes: { | ||
| mode: { | ||
| options: ['default', 'iconOnly'], | ||
| control: {type: 'select'}, | ||
| }, | ||
| totalPages: {control: {type: 'number'}}, | ||
| currentPage: {control: {type: 'number'}}, | ||
| dynamicCount: {control: {type: 'number'}}, | ||
| showEllipsis: {control: {type: 'boolean'}}, | ||
| hideNavigationControls: {control: {type: 'boolean'}}, | ||
| hidePageList: {control: {type: 'boolean'}}, | ||
| disabled: {control: {type: 'boolean'}}, | ||
| navLeftLabel: {control: {type: 'text'}}, | ||
| navRightLabel: {control: {type: 'text'}}, | ||
| }, | ||
| }; | ||
|
|
||
| type Args = { | ||
| totalPages: number; | ||
| currentPage: number; | ||
| dynamicCount: number; | ||
| showEllipsis: boolean; | ||
| hideNavigationControls: boolean; | ||
| hidePageList: boolean; | ||
| disabled: boolean; | ||
| mode: 'default' | 'iconOnly'; | ||
| navLeftLabel: string; | ||
| navRightLabel: string; | ||
| }; | ||
|
|
||
| export const Default: StoryComponent<Args> = ({ | ||
| totalPages, | ||
| currentPage, | ||
| dynamicCount, | ||
| showEllipsis, | ||
| hideNavigationControls, | ||
| hidePageList, | ||
| disabled, | ||
| mode, | ||
| navLeftLabel, | ||
| navRightLabel, | ||
| }) => ( | ||
| <Pagination | ||
| totalPages={totalPages} | ||
| currentPage={currentPage} | ||
| dynamicCount={dynamicCount} | ||
| showEllipsis={showEllipsis} | ||
| hideNavigationControls={hideNavigationControls} | ||
| hidePageList={hidePageList} | ||
| disabled={disabled} | ||
| mode={mode} | ||
| navLeftLabel={navLeftLabel || undefined} | ||
| navRightLabel={navRightLabel || undefined} | ||
| /> | ||
| ); | ||
|
|
||
| Default.storyName = 'Pagination'; | ||
| Default.args = { | ||
| totalPages: 9, | ||
| currentPage: 3, | ||
| dynamicCount: 3, | ||
| showEllipsis: true, | ||
| hideNavigationControls: false, | ||
| hidePageList: false, | ||
| disabled: false, | ||
| mode: 'default', | ||
| navLeftLabel: '', | ||
| navRightLabel: '', | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,118 @@ | ||
| import * as React from 'react'; | ||
| import {render, screen} from '@testing-library/react'; | ||
| import userEvent from '@testing-library/user-event'; | ||
| import ThemeContextProvider from '../theme-context-provider'; | ||
| import {makeTheme} from './test-utils'; | ||
| import Pagination, {getPaginationItems} from '../pagination'; | ||
|
|
||
| test('renders pagination navigation landmark', () => { | ||
| render( | ||
| <ThemeContextProvider theme={makeTheme()}> | ||
| <Pagination totalPages={5} defaultPage={1} /> | ||
| </ThemeContextProvider> | ||
| ); | ||
|
|
||
| expect(screen.getByRole('navigation', {name: 'Paginación - Página 1 de 5'})).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| test('does not render when there is a single page', () => { | ||
| render( | ||
| <ThemeContextProvider theme={makeTheme()}> | ||
| <Pagination totalPages={1} /> | ||
| </ThemeContextProvider> | ||
| ); | ||
|
|
||
| expect(screen.queryByRole('navigation')).not.toBeInTheDocument(); | ||
| }); | ||
|
|
||
| test('calls onChange when a page button is clicked (uncontrolled)', async () => { | ||
| const onChange = jest.fn(); | ||
| render( | ||
| <ThemeContextProvider theme={makeTheme()}> | ||
| <Pagination totalPages={5} defaultPage={1} onChange={onChange} /> | ||
| </ThemeContextProvider> | ||
| ); | ||
|
|
||
| await userEvent.click(screen.getByRole('button', {name: 'Ir a la página 3'})); | ||
|
|
||
| expect(onChange).toHaveBeenCalledWith(3); | ||
| }); | ||
|
|
||
| test('calls onChange when Next is clicked', async () => { | ||
| const onChange = jest.fn(); | ||
| render( | ||
| <ThemeContextProvider theme={makeTheme()}> | ||
| <Pagination totalPages={5} defaultPage={2} onChange={onChange} /> | ||
| </ThemeContextProvider> | ||
| ); | ||
|
|
||
| await userEvent.click(screen.getByRole('button', {name: 'Página siguiente'})); | ||
|
|
||
| expect(onChange).toHaveBeenCalledWith(3); | ||
| }); | ||
|
|
||
| test('does not change page when disabled', async () => { | ||
| const onChange = jest.fn(); | ||
| render( | ||
| <ThemeContextProvider theme={makeTheme()}> | ||
| <Pagination totalPages={5} defaultPage={1} onChange={onChange} disabled /> | ||
| </ThemeContextProvider> | ||
| ); | ||
|
|
||
| await userEvent.click(screen.getByRole('button', {name: 'Ir a la página 3'})); | ||
|
|
||
| expect(onChange).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| test('keeps Previous and Next visible at page boundaries (aria-disabled)', async () => { | ||
| const onChange = jest.fn(); | ||
| render( | ||
| <ThemeContextProvider theme={makeTheme()}> | ||
| <Pagination totalPages={5} defaultPage={1} onChange={onChange} /> | ||
| </ThemeContextProvider> | ||
| ); | ||
|
|
||
| // At the first page, Previous is rendered but marked aria-disabled and | ||
| // clicking it should not change pages. Next stays interactive. | ||
| const prev = screen.getByRole('button', {name: 'Página anterior'}); | ||
| const next = screen.getByRole('button', {name: 'Página siguiente'}); | ||
| expect(prev).toHaveAttribute('aria-disabled', 'true'); | ||
| expect(next).not.toHaveAttribute('aria-disabled'); | ||
|
|
||
| await userEvent.click(prev); | ||
| expect(onChange).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| test('honors controlled currentPage', () => { | ||
| render( | ||
| <ThemeContextProvider theme={makeTheme()}> | ||
| <Pagination totalPages={5} currentPage={3} /> | ||
| </ThemeContextProvider> | ||
| ); | ||
|
|
||
| // The current page is rendered as an aria-disabled button so VoiceOver can | ||
| // still land on it and announce "Página 3, página actual" — but the | ||
| // "Ir a la página 3" navigation button is absent (it's not navigable). | ||
| expect(screen.queryByRole('button', {name: 'Ir a la página 3'})).not.toBeInTheDocument(); | ||
| expect(screen.getByRole('button', {name: 'Página 3, página actual'})).toBeInTheDocument(); | ||
| expect(screen.getByRole('button', {name: 'Ir a la página 2'})).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| describe('getPaginationItems', () => { | ||
| test('returns an empty array for a single page', () => { | ||
| expect(getPaginationItems({totalPages: 1, currentPage: 1})).toEqual([]); | ||
| }); | ||
|
|
||
| test('returns all pages when total fits without ellipsis', () => { | ||
| const items = getPaginationItems({totalPages: 5, currentPage: 3}); | ||
| expect(items.filter((i) => i.type === 'ellipsis')).toHaveLength(0); | ||
| expect(items).toHaveLength(5); | ||
| }); | ||
|
|
||
| test('inserts ellipsis when middle pages are skipped', () => { | ||
| const items = getPaginationItems({totalPages: 20, currentPage: 10}); | ||
| expect(items.some((i) => i.type === 'ellipsis')).toBe(true); | ||
| expect(items[0]).toMatchObject({type: 'page', page: 1}); | ||
| expect(items[items.length - 1]).toMatchObject({type: 'page', page: 20}); | ||
| }); | ||
| }); |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks that previewTools is not working with Cyber skin, is this expected?
https://mistica-pl9771ukj-flows-projects-65bb050e.vercel.app/playroom#?code=N4Igxg9gJgpiBcIA8AFATjAbgSxgdwBUIIAbAZwD4kB6dLXQ48igHQDtUBDAc2zc4Au2CGwAE7UaP6YAMjABmAmZwBGMEgF4WIATDIDtEqZ0wAlbNwAWS1eq069BkEYEQBnEih56NwAIwADAC%2BRgC20DD2sPKcAK4kTuJiomCxaBhsAl7ckcA5AgDK7roAFNoADjx8gsJsAJKQbNkw2gA0ogDMAJQhySIAwpacbDm%2BJZU5XaIaFKJkMIXFMGUgE9VCIg0izW2iEzA97NQUIK06ljCheggA2iD9EKGhsWzYAgCeAPr972poIABdM54bBQASWMi3DoANgCAKCQA
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmmm idk it does work for me...
Grabacion.de.pantalla.2026-06-09.a.las.16.05.01.mov
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is the PreviewTools component that is broken
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Marcosld correct, we figured it out yesterday in pair programming. the skin is missing/not supported yet. I'd open a ticket to address it in a different branch. it's a quick and easy fix :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#1569