-
Notifications
You must be signed in to change notification settings - Fork 2
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
Import PaginationNavigation
from client as Pagination
#1817
Changes from all commits
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,148 @@ | ||
import classnames from 'classnames'; | ||
import type { JSX } from 'preact'; | ||
|
||
import type { PresentationalProps } from '../../types'; | ||
import { pageNumberOptions } from '../../util/pagination'; | ||
import { ArrowLeftIcon, ArrowRightIcon } from '../icons'; | ||
import Button from '../input/Button'; | ||
import type { ButtonProps } from '../input/Button'; | ||
|
||
type NavigationButtonProps = PresentationalProps & | ||
ButtonProps & | ||
Omit<JSX.HTMLAttributes<HTMLButtonElement>, 'icon' | 'size'>; | ||
|
||
function NavigationButton({ ...buttonProps }: NavigationButtonProps) { | ||
return ( | ||
<Button | ||
classes={classnames( | ||
'px-3.5 py-2.5 gap-x-1', | ||
'font-semibold rounded', | ||
// These colors are the same as the "dark" variant of IconButton | ||
'text-grey-7 bg-grey-2 enabled:hover:text-grey-9 enabled:hover:bg-grey-3', | ||
'disabled:text-grey-5 aria-pressed:bg-grey-3 aria-expanded:bg-grey-3', | ||
)} | ||
{...buttonProps} | ||
size="custom" | ||
variant="custom" | ||
/> | ||
); | ||
} | ||
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. There are some visual issues here: the Prev/Next buttons have a different height than the others, and un-pressed buttons have a grey background that matches the background color of the Notebook, instead of no background. I will address this as part of a design update. |
||
|
||
export type PaginationProps = { | ||
/** 1-indexed page number of currently-visible page of results */ | ||
currentPage: number; | ||
|
||
/** | ||
* Callback invoked when the user clicks a navigation button to change the | ||
* current page. | ||
*/ | ||
onChangePage: (page: number) => void; | ||
|
||
/** The total number of available pages. */ | ||
totalPages: number; | ||
}; | ||
|
||
/** | ||
* Render controls for navigating between pages in a paginated list of items. | ||
* | ||
* Buttons corresponding to nearby pages are shown on wider screens; for narrow | ||
* screens only Prev and Next buttons are shown. | ||
*/ | ||
export default function Pagination({ | ||
currentPage, | ||
onChangePage, | ||
totalPages, | ||
}: PaginationProps) { | ||
// Pages are 1-indexed | ||
const hasNextPage = currentPage < totalPages; | ||
const hasPreviousPage = currentPage > 1; | ||
const pageNumbers = pageNumberOptions(currentPage, totalPages); | ||
|
||
/** | ||
* @param {number} pageNumber | ||
* @param {HTMLElement} element | ||
*/ | ||
const changePageTo = (pageNumber: number, element: HTMLElement) => { | ||
onChangePage(pageNumber); | ||
// Because changing pagination page doesn't reload the page (as it would | ||
// in a "traditional" HTML context), the clicked-upon navigation button | ||
// will awkwardly retain focus unless it is actively removed. | ||
// TODO: Evaluate this for a11y issues | ||
element.blur(); | ||
}; | ||
|
||
return ( | ||
<div | ||
className="flex items-center text-md" | ||
data-testid="pagination-navigation" | ||
> | ||
<div className="w-28 h-10"> | ||
{hasPreviousPage && ( | ||
<NavigationButton | ||
title="Go to previous page" | ||
onClick={e => | ||
changePageTo(currentPage - 1, e.target as HTMLElement) | ||
} | ||
> | ||
<ArrowLeftIcon /> | ||
prev | ||
</NavigationButton> | ||
)} | ||
</div> | ||
<ul | ||
className={classnames( | ||
// Where there's enough horizontal space, | ||
// lay out page navigation buttons horizontally between prev/next: | ||
// | prevPage | numberedPages | nextPage | ||
// | ||
// e.g. | ||
// | [<- prev] | [2] ... [5] [6] [7] ... [10] | [next ->] | | ||
// | ||
// These page buttons are hidden on narrow screens | ||
'hidden', | ||
// For slightly wider screens, they are shown in a horizontal row | ||
'md:flex md:items-center md:justify-center md:gap-x-2', | ||
// when visible, this element should stretch to fill available space | ||
'md:grow', | ||
)} | ||
> | ||
{pageNumbers.map((page, idx) => ( | ||
<li key={idx}> | ||
{page === null ? ( | ||
<div data-testid="pagination-gap">...</div> | ||
) : ( | ||
<NavigationButton | ||
key={`page-${idx}`} | ||
title={`Go to page ${page}`} | ||
pressed={page === currentPage} | ||
onClick={e => changePageTo(page, e.target as HTMLElement)} | ||
> | ||
{page.toString()} | ||
</NavigationButton> | ||
)} | ||
</li> | ||
))} | ||
</ul> | ||
<div | ||
className={classnames( | ||
'w-28 h-10 flex justify-end', | ||
// When page buttons are not shown, this element should grow to fill | ||
// available space. But when page buttons are shown, it should not. | ||
'grow md:grow-0', | ||
)} | ||
> | ||
{hasNextPage && ( | ||
<NavigationButton | ||
title="Go to next page" | ||
onClick={e => | ||
changePageTo(currentPage + 1, e.target as HTMLElement) | ||
} | ||
> | ||
next | ||
<ArrowRightIcon /> | ||
</NavigationButton> | ||
)} | ||
</div> | ||
</div> | ||
); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,13 @@ | ||
export { default as Link } from './Link'; | ||
export { default as LinkButton } from './LinkButton'; | ||
export { default as Pagination } from './Pagination'; | ||
export { default as PointerButton } from './PointerButton'; | ||
export { default as Tab } from './Tab'; | ||
export { default as TabList } from './TabList'; | ||
|
||
export type { LinkProps } from './Link'; | ||
export type { LinkButtonProps } from './LinkButton'; | ||
export type { PaginationProps } from './Pagination'; | ||
export type { PointerButtonProps } from './PointerButton'; | ||
export type { TabProps } from './Tab'; | ||
export type { TabListProps } from './TabList'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,132 @@ | ||
import { checkAccessibility, mount } from '@hypothesis/frontend-testing'; | ||
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. This is unchanged from |
||
|
||
import Pagination, { $imports } from '../Pagination'; | ||
|
||
describe('Pagination', () => { | ||
let fakeOnChangePage; | ||
let fakePageNumberOptions; | ||
|
||
const findButton = (wrapper, title) => | ||
wrapper.find('button').filterWhere(n => n.props().title === title); | ||
|
||
const createComponent = (props = {}) => { | ||
return mount( | ||
<Pagination | ||
currentPage={1} | ||
onChangePage={fakeOnChangePage} | ||
totalPages={10} | ||
{...props} | ||
/>, | ||
); | ||
}; | ||
|
||
beforeEach(() => { | ||
fakeOnChangePage = sinon.stub(); | ||
fakePageNumberOptions = sinon.stub().returns([1, 2, 3, 4, null, 10]); | ||
|
||
$imports.$mock({ | ||
'../../util/pagination': { pageNumberOptions: fakePageNumberOptions }, | ||
}); | ||
}); | ||
|
||
afterEach(() => { | ||
$imports.$restore(); | ||
}); | ||
|
||
describe('prev button', () => { | ||
it('should render a prev button when there are previous pages to show', () => { | ||
const wrapper = createComponent({ currentPage: 2 }); | ||
const button = findButton(wrapper, 'Go to previous page'); | ||
assert.isTrue(button.exists()); | ||
}); | ||
|
||
it('should not render a prev button if there are no previous pages to show', () => { | ||
const wrapper = createComponent({ currentPage: 1 }); | ||
const button = findButton(wrapper, 'Go to previous page'); | ||
assert.isFalse(button.exists()); | ||
}); | ||
|
||
it('should invoke the onChangePage callback when clicked', () => { | ||
const wrapper = createComponent({ currentPage: 2 }); | ||
const button = findButton(wrapper, 'Go to previous page'); | ||
button.simulate('click'); | ||
assert.calledWith(fakeOnChangePage, 1); | ||
}); | ||
|
||
it('should remove focus from button after clicked', () => { | ||
const wrapper = createComponent({ currentPage: 2 }); | ||
const button = findButton(wrapper, 'Go to previous page'); | ||
const buttonEl = button.getDOMNode(); | ||
const blurSpy = sinon.spy(buttonEl, 'blur'); | ||
|
||
button.simulate('click'); | ||
|
||
assert.equal(blurSpy.callCount, 1); | ||
}); | ||
}); | ||
|
||
describe('next button', () => { | ||
it('should render a next button when there are further pages to show', () => { | ||
const wrapper = createComponent({ currentPage: 1 }); | ||
const button = findButton(wrapper, 'Go to next page'); | ||
assert.isTrue(button.exists()); | ||
}); | ||
|
||
it('should not render a next button if there are no further pages to show', () => { | ||
const wrapper = createComponent({ currentPage: 10 }); | ||
const button = findButton(wrapper, 'Go to next page'); | ||
assert.isFalse(button.exists()); | ||
}); | ||
|
||
it('should invoke the `onChangePage` callback when clicked', () => { | ||
const wrapper = createComponent({ currentPage: 1 }); | ||
const button = findButton(wrapper, 'Go to next page'); | ||
button.simulate('click'); | ||
assert.calledWith(fakeOnChangePage, 2); | ||
}); | ||
|
||
it('should remove focus from button after clicked', () => { | ||
const wrapper = createComponent({ currentPage: 1 }); | ||
const button = findButton(wrapper, 'Go to next page'); | ||
const buttonEl = button.getDOMNode(); | ||
const blurSpy = sinon.spy(buttonEl, 'blur'); | ||
|
||
button.simulate('click'); | ||
|
||
assert.equal(blurSpy.callCount, 1); | ||
}); | ||
}); | ||
|
||
describe('page number buttons', () => { | ||
it('should render buttons for each page number available', () => { | ||
fakePageNumberOptions.returns([1, 2, 3, 4, null, 10]); | ||
const wrapper = createComponent(); | ||
|
||
[1, 2, 3, 4, 10].forEach(pageNumber => { | ||
const button = findButton(wrapper, `Go to page ${pageNumber}`); | ||
assert.isTrue(button.exists()); | ||
}); | ||
|
||
// There is one "gap": | ||
assert.equal(wrapper.find('[data-testid="pagination-gap"]').length, 1); | ||
}); | ||
|
||
it('should invoke the onChangePage callback when page number button clicked', () => { | ||
fakePageNumberOptions.returns([1, 2, 3, 4, null, 10]); | ||
const wrapper = createComponent(); | ||
|
||
[1, 2, 3, 4, 10].forEach(pageNumber => { | ||
const button = findButton(wrapper, `Go to page ${pageNumber}`); | ||
button.simulate('click'); | ||
assert.calledWith(fakeOnChangePage, pageNumber); | ||
}); | ||
}); | ||
}); | ||
|
||
it( | ||
'should pass a11y checks', | ||
checkAccessibility({ | ||
content: () => createComponent({ currentPage: 2 }), | ||
}), | ||
); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,71 @@ | ||
import { useState } from 'preact/hooks'; | ||
|
||
import { Pagination } from '../../../../'; | ||
import Library from '../../Library'; | ||
|
||
export default function PaginationPage() { | ||
const [currentPage, setCurrentPage] = useState(1); | ||
|
||
return ( | ||
<Library.Page | ||
title="Pagination" | ||
intro={ | ||
<p> | ||
<code>Pagination</code> is a component that allows navigating between | ||
pages in a paginated set of items. | ||
</p> | ||
} | ||
> | ||
<Library.Section> | ||
<Library.Pattern> | ||
<Library.Usage symbolName="Pagination" /> | ||
<Library.Example> | ||
<Library.Demo title="Basic usage" withSource> | ||
<Pagination | ||
currentPage={currentPage} | ||
totalPages={10} | ||
onChangePage={page => setCurrentPage(page)} | ||
/> | ||
</Library.Demo> | ||
</Library.Example> | ||
</Library.Pattern> | ||
|
||
<Library.Pattern title="Component API"> | ||
<Library.Example title="currentPage"> | ||
<Library.Info> | ||
<Library.InfoItem label="description"> | ||
The 1-based number of the currently visible page. | ||
</Library.InfoItem> | ||
<Library.InfoItem label="type"> | ||
<code>number</code> | ||
</Library.InfoItem> | ||
</Library.Info> | ||
</Library.Example> | ||
|
||
<Library.Example title="onChangePage"> | ||
<Library.Info> | ||
<Library.InfoItem label="description"> | ||
Callback invoked with the new page number when the user clicks a | ||
navigation button. | ||
</Library.InfoItem> | ||
<Library.InfoItem label="type"> | ||
<code>(newPage: number) {'=>'} void</code> | ||
</Library.InfoItem> | ||
</Library.Info> | ||
</Library.Example> | ||
|
||
<Library.Example title="totalPages"> | ||
<Library.Info> | ||
<Library.InfoItem label="description"> | ||
The total number of pages available. | ||
</Library.InfoItem> | ||
<Library.InfoItem label="type"> | ||
<code>number</code> | ||
</Library.InfoItem> | ||
</Library.Info> | ||
</Library.Example> | ||
</Library.Pattern> | ||
</Library.Section> | ||
</Library.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.
This is unchanged from
PaginationNavigation
in the client aside from the rename and a few minor JSDoc additions.