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

[ER-45674] Allow not providing totalCount for compact Pagination #4673

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 27 additions & 7 deletions packages/base/Pagination/src/Pagination/Pagination.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,39 @@ import { Typography } from '@toptal/picasso-typography'
import { getRange, ELLIPSIS } from './utils'
import { PaginationButton } from '../PaginationButton'

export interface Props extends BaseProps, HTMLAttributes<HTMLDivElement> {
interface CommonProps extends BaseProps, HTMLAttributes<HTMLDivElement> {
/** Value of the current highlighted page */
activePage: number
/** Shows `Pagination` in disabled state when pages are not changeable */
disabled?: boolean
/** Callback invoked when any page number is clicked */
onPageChange: (page: number) => void
/** Value of total pages of the data set used for calculation of page buttons */
totalPages: number
/** Number of the active page siblings */
siblingCount?: number
/** Shows the next button as disabled. */
nextDisabled?: boolean
}

interface DefaultVariantProps {
/** Variant of the pagination representation */
variant?: 'default'
/** Value of total pages of the data set used for calculation of page buttons. */
totalPages: number
}

interface CompactVariantProps {
/** Variant of the pagination representation */
variant?: 'default' | 'compact'
variant: 'compact'
/** Value of total pages of the data set used for calculation of page buttons.
* Only optional for the `compact` variant.
* When not provided the last page can't be detected, so next button will always be enabled.
* Use `nextDisabled=true` to disable it when rendering the last page.
* */
totalPages?: number
}

export type Props = CommonProps & (DefaultVariantProps | CompactVariantProps)

export const Pagination = forwardRef<HTMLDivElement, Props>(function Pagination(
props,
ref
Expand All @@ -35,15 +53,17 @@ export const Pagination = forwardRef<HTMLDivElement, Props>(function Pagination(
onPageChange,
siblingCount = 2,
variant,
nextDisabled,
...rest
} = props

const pages = useMemo(
() => getRange({ activePage, totalPages, siblingCount }),
() =>
totalPages ? getRange({ activePage, totalPages, siblingCount }) : [],
[activePage, totalPages, siblingCount]
)

if (pages.length <= 1) {
if (totalPages !== undefined && totalPages <= 1) {
return null
}

Expand Down Expand Up @@ -92,7 +112,7 @@ export const Pagination = forwardRef<HTMLDivElement, Props>(function Pagination(

<Button
className='[&+&]:!ml-2'
disabled={isLastActive || disabled}
disabled={isLastActive || disabled || nextDisabled}
onClick={handleNextClick}
variant='secondary'
size='small'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,47 @@ exports[`Pagination renders 1`] = `
</div>
`;

exports[`Pagination renders compact without totalPages 1`] = `
<div>
<div
class="Picasso-root"
>
<div
class="items-center inline-flex"
>
<button
aria-disabled="false"
class="base-Button text-lg inline-flex items-center justify-center select-none appearance-none m-0 relative normal-case align-middle transition-colors duration-350 ease-out shrink-0 outline-none [[data-component-type="button"]+&]:ml cursor-pointer no-underline hover:no-underline rounded-sm shadow-none focus-visible:shadow-[0_0_0_3px_rgba(32,78,207,0.48)] focus-within:shadow-[0_0_0_3px_rgba(32,78,207,0.48)] border border-solid text-black hover:border-black visited:text-black active:bg-gray active:border-black bg-white border-gray min-w h-6 py-0 px-3 [&+&]:!ml-2"
data-component-type="button"
role="button"
tabindex="0"
type="button"
>
<span
class="items-center inline-flex font-semibold whitespace-nowrap text-button"
>
Prev
</span>
</button>
<button
aria-disabled="false"
class="base-Button text-lg inline-flex items-center justify-center select-none appearance-none m-0 relative normal-case align-middle transition-colors duration-350 ease-out shrink-0 outline-none [[data-component-type="button"]+&]:ml cursor-pointer no-underline hover:no-underline rounded-sm shadow-none focus-visible:shadow-[0_0_0_3px_rgba(32,78,207,0.48)] focus-within:shadow-[0_0_0_3px_rgba(32,78,207,0.48)] border border-solid text-black hover:border-black visited:text-black active:bg-gray active:border-black bg-white border-gray min-w h-6 py-0 px-3 [&+&]:!ml-2"
data-component-type="button"
role="button"
tabindex="0"
type="button"
>
<span
class="items-center inline-flex font-semibold whitespace-nowrap text-button"
>
Next
</span>
</button>
</div>
</div>
</div>
`;

exports[`Pagination renders disabled 1`] = `
<div>
<div
Expand Down Expand Up @@ -352,3 +393,45 @@ exports[`Pagination renders nothing for 1 page 1`] = `
/>
</div>
`;

exports[`Pagination renders with next disabled 1`] = `
<div>
<div
class="Picasso-root"
>
<div
class="items-center inline-flex"
>
<button
aria-disabled="false"
class="base-Button text-lg inline-flex items-center justify-center select-none appearance-none m-0 relative normal-case align-middle transition-colors duration-350 ease-out shrink-0 outline-none [[data-component-type="button"]+&]:ml cursor-pointer no-underline hover:no-underline rounded-sm shadow-none focus-visible:shadow-[0_0_0_3px_rgba(32,78,207,0.48)] focus-within:shadow-[0_0_0_3px_rgba(32,78,207,0.48)] border border-solid text-black hover:border-black visited:text-black active:bg-gray active:border-black bg-white border-gray min-w h-6 py-0 px-3 [&+&]:!ml-2"
data-component-type="button"
role="button"
tabindex="0"
type="button"
>
<span
class="items-center inline-flex font-semibold whitespace-nowrap text-button"
>
Prev
</span>
</button>
<button
aria-disabled="true"
class="base-Button base- text-lg inline-flex items-center justify-center select-none appearance-none m-0 relative normal-case align-middle transition-colors duration-350 ease-out shrink-0 outline-none [[data-component-type="button"]+&]:ml cursor-default pointer-events no-underline hover:no-underline rounded-sm shadow-none border border-solid text-gray visited:text-gray border-gray bg-white min-w h-6 py-0 px-3 [&+&]:!ml-2"
data-component-type="button"
disabled=""
role="button"
tabindex="-1"
type="button"
>
<span
class="items-center inline-flex font-semibold whitespace-nowrap text-button"
>
Next
</span>
</button>
</div>
</div>
</div>
`;
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ const Example = () => (
<Pagination
activePage={3}
onPageChange={handlePageChange}
totalPages={5}
Copy link

Choose a reason for hiding this comment

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

Would be nice to also show a variant with nextDisabled

variant='compact'
/>
</Container>
Expand Down
32 changes: 31 additions & 1 deletion packages/base/Pagination/src/Pagination/test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,23 @@ import type { Props } from './Pagination'
import { Pagination } from './Pagination'

const renderPagination = (props: OmitInternalProps<Props>) => {
const { activePage, disabled, onPageChange, totalPages } = props
const {
activePage,
disabled,
onPageChange,
totalPages,
nextDisabled,
variant,
} = props

return render(
<Pagination
activePage={activePage}
disabled={disabled}
onPageChange={onPageChange}
totalPages={totalPages}
nextDisabled={nextDisabled}
variant={variant}
/>
)
}
Expand All @@ -29,6 +38,16 @@ describe('Pagination', () => {
expect(container).toMatchSnapshot()
})

it('renders compact without totalPages', () => {
const { container } = renderPagination({
activePage: 5,
variant: 'compact',
onPageChange: () => {},
})

expect(container).toMatchSnapshot()
})

it('renders disabled', () => {
const { container } = renderPagination({
activePage: 5,
Expand All @@ -40,6 +59,17 @@ describe('Pagination', () => {
expect(container).toMatchSnapshot()
})

it('renders with next disabled', () => {
const { container } = renderPagination({
activePage: 5,
variant: 'compact',
nextDisabled: true,
onPageChange: () => {},
})

expect(container).toMatchSnapshot()
})

it('renders nothing for 1 page', () => {
const { container } = renderPagination({
activePage: 1,
Expand Down