From 957984d9e91f5ca7577efadef6e754bb23ac7d65 Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Thu, 16 May 2019 16:09:58 -0400 Subject: [PATCH 1/2] convert to PF Pagination component --- .../PaginatedDataList.test.jsx | 33 +- __tests__/components/Pagination.test.jsx | 349 ------ .../NotificationListItem.test.jsx.snap | 85 -- .../OrganizationNotifications.test.jsx.snap | 1042 +++++++++++------ src/components/AddRole/SelectResourceStep.jsx | 9 - src/components/Lookup/Lookup.jsx | 9 - .../PaginatedDataList/PaginatedDataList.jsx | 102 +- src/components/Pagination/Pagination.jsx | 275 +---- src/components/Pagination/styles.scss | 88 -- src/index.jsx | 1 - 10 files changed, 758 insertions(+), 1235 deletions(-) delete mode 100644 __tests__/components/Pagination.test.jsx delete mode 100644 src/components/Pagination/styles.scss diff --git a/__tests__/components/PaginatedDataList/PaginatedDataList.test.jsx b/__tests__/components/PaginatedDataList/PaginatedDataList.test.jsx index 5efd111cb3..f19380c229 100644 --- a/__tests__/components/PaginatedDataList/PaginatedDataList.test.jsx +++ b/__tests__/components/PaginatedDataList/PaginatedDataList.test.jsx @@ -89,10 +89,35 @@ describe('', () => { ); const pagination = wrapper.find('Pagination'); - pagination.prop('onSetPage')(2, 5); - expect(history.location.search).toEqual('?item.page=2&item.page_size=5'); + pagination.prop('onSetPage')(null, 2); + expect(history.location.search).toEqual('?item.page=2'); wrapper.update(); - pagination.prop('onSetPage')(1, 25); - expect(history.location.search).toEqual('?item.page=1&item.page_size=25'); + pagination.prop('onSetPage')(null, 1); + expect(history.location.search).toEqual('?item.page=1'); + }); + + test('should navigate to page when Pagination calls onPerPageSelect prop', () => { + const history = createMemoryHistory({ + initialEntries: ['/organizations/1/teams'], + }); + const wrapper = mountWithContexts( + , { context: { router: { history } } } + ); + + const pagination = wrapper.find('Pagination'); + pagination.prop('onPerPageSelect')(null, 5); + expect(history.location.search).toEqual('?item.page_size=5'); + wrapper.update(); + pagination.prop('onPerPageSelect')(null, 25); + expect(history.location.search).toEqual('?item.page_size=25'); }); }); diff --git a/__tests__/components/Pagination.test.jsx b/__tests__/components/Pagination.test.jsx deleted file mode 100644 index 4de96f9631..0000000000 --- a/__tests__/components/Pagination.test.jsx +++ /dev/null @@ -1,349 +0,0 @@ -import React from 'react'; -import { mount } from 'enzyme'; -import { I18nProvider } from '@lingui/react'; -import Pagination from '../../src/components/Pagination'; - -describe('', () => { - let pagination; - - afterEach(() => { - if (pagination) { - pagination.unmount(); - pagination = null; - } - }); - - test('it triggers the expected callbacks on next and last', () => { - const next = 'button[aria-label="Next"]'; - const last = 'button[aria-label="Last"]'; - - const onSetPage = jest.fn(); - - pagination = mount( - - - - ); - - pagination.find(next).simulate('click'); - - expect(onSetPage).toHaveBeenCalledTimes(1); - expect(onSetPage).toBeCalledWith(2, 5); - - pagination.find(last).simulate('click'); - - expect(onSetPage).toHaveBeenCalledTimes(2); - expect(onSetPage).toBeCalledWith(5, 5); - }); - - test('it triggers the expected callback on previous and first', () => { - const previous = 'button[aria-label="Previous"]'; - const first = 'button[aria-label="First"]'; - - const onSetPage = jest.fn(); - - pagination = mount( - - - - ); - - pagination.find(previous).simulate('click'); - - expect(onSetPage).toHaveBeenCalledTimes(1); - expect(onSetPage).toBeCalledWith(4, 5); - - pagination.find(first).simulate('click'); - - expect(onSetPage).toHaveBeenCalledTimes(2); - expect(onSetPage).toBeCalledWith(1, 5); - }); - - test('previous button does not work on page 1', () => { - const previous = 'button[aria-label="First"]'; - const onSetPage = jest.fn(); - - pagination = mount( - - - - ); - pagination.find(previous).simulate('click'); - expect(onSetPage).toHaveBeenCalledTimes(0); - }); - - test('changing pageSize works', () => { - const pageSizeDropdownToggleSelector = 'DropdownToggle DropdownToggle[className="togglePageSize"]'; - const pageSizeDropdownItemsSelector = 'DropdownItem button'; - const onSetPage = jest.fn(); - - pagination = mount( - - - - ); - const pageSizeDropdownToggle = pagination.find(pageSizeDropdownToggleSelector); - expect(pageSizeDropdownToggle.length).toBe(1); - pageSizeDropdownToggle.at(0).simulate('click'); - - const pageSizeDropdownItems = pagination.find(pageSizeDropdownItemsSelector); - expect(pageSizeDropdownItems.length).toBe(3); - pageSizeDropdownItems.at(1).simulate('click'); - expect(onSetPage).toBeCalledWith(1, 25); - }); - - test('itemCount displays correctly', () => { - const onSetPage = jest.fn(); - - pagination = mount( - - - - ); - let itemCount = pagination.find('.awx-pagination__item-count'); - expect(itemCount.text()).toEqual('Items 1 – 5 of 7'); - - pagination = mount( - - - - ); - itemCount = pagination.find('.awx-pagination__item-count'); - expect(itemCount.text()).toEqual('Items 6 – 7 of 7'); - }); - - test('itemCount matching pageSize displays correctly', () => { - const onSetPage = jest.fn(); - - pagination = mount( - - - - ); - const itemCount = pagination.find('.awx-pagination__item-count'); - expect(itemCount.text()).toEqual('Items 1 – 5 of 5'); - }); - - test('itemCount less than pageSize displays correctly', () => { - const onSetPage = jest.fn(); - - pagination = mount( - - - - ); - const itemCount = pagination.find('.awx-pagination__item-count'); - expect(itemCount.text()).toEqual('Items 1 – 3 of 3'); - }); - - test('submitting a new page by typing in input works', () => { - const textInputSelector = '.awx-pagination__page-input.pf-c-form-control'; - const submitFormSelector = '.awx-pagination__page-input-form'; - const onSetPage = jest.fn(); - - pagination = mount( - - - - ); - const paginationElem = pagination.find('Pagination'); - expect(paginationElem.state().value).toBe(1); - const textInput = pagination.find(textInputSelector); - expect(textInput.length).toBe(1); - expect(textInput.at(0).prop('value')).toBe(1); - const input = pagination.find('TextInput'); - expect(input.prop('value')).toBe(1); - input.prop('onChange')(2); - pagination.update(); - const submitForm = pagination.find(submitFormSelector); - expect(submitForm.length).toBe(1); - submitForm.simulate('submit'); - expect(pagination.find('TextInput').prop('value')).toBe(2); - // assert onPageChange was called - expect(paginationElem.state().value).toBe(2); - }); - - test('submitting a new page by typing in input does not work when invalid', () => { - const textInputSelector = '.awx-pagination__page-input.pf-c-form-control'; - const submitFormSelector = '.awx-pagination__page-input-form'; - const onSetPage = jest.fn(); - - pagination = mount( - - - - ); - const paginationElem = pagination.find('Pagination'); - expect(paginationElem.state().value).toBe(1); - const textInput = pagination.find(textInputSelector); - expect(textInput.length).toBe(1); - expect(textInput.at(0).prop('value')).toBe(1); - const input = pagination.find('TextInput'); - expect(input.prop('value')).toBe(1); - input.prop('onChange')('!invalid'); - pagination.update(); - const submitForm = pagination.find(submitFormSelector); - expect(submitForm.length).toBe(1); - submitForm.simulate('submit'); - expect(pagination.find('TextInput').prop('value')).toBe(1); - // assert onPageChange was not called - expect(paginationElem.state().value).toBe(1); - }); - - test('text input page change is not displayed when only 1 page', () => { - const onSetPage = jest.fn(); - const pageNumber = 'input[aria-label="Page Number"]'; - pagination = mount( - - - - ); - let pageInput = pagination.find(pageNumber); - expect(pageInput.length).toBe(0); - - pagination = mount( - - - - ); - pageInput = pagination.find(pageNumber); - expect(pageInput.length).toBe(1); - }); - - test('make sure componentDidUpdate calls onPageChange', () => { - const onSetPage = jest.fn(); - - pagination = mount( - - - - ); - const paginationElem = pagination.find('Pagination'); - expect(paginationElem.state().value).toBe(1); - pagination.setProps({ - children: ( - - ) - }); - paginationElem.update(); - expect(paginationElem.state().value).toBe(2); - }); - - test('when showPageSizeOptions is passed as false there should not be a dropdown in the DOM', () => { - const pageSizeDropdownSelector = '.awx-pagination__page-size-selection'; - const onSetPage = jest.fn(); - - pagination = mount( - - - - ); - const pageSizeDropdown = pagination.find(pageSizeDropdownSelector); - expect(pageSizeDropdown.length).toBe(0); - }); -}); diff --git a/__tests__/components/__snapshots__/NotificationListItem.test.jsx.snap b/__tests__/components/__snapshots__/NotificationListItem.test.jsx.snap index 5be978fcf8..8f17dfe0c1 100644 --- a/__tests__/components/__snapshots__/NotificationListItem.test.jsx.snap +++ b/__tests__/components/__snapshots__/NotificationListItem.test.jsx.snap @@ -448,91 +448,6 @@ exports[` initially renders succe - - - - - - - diff --git a/__tests__/pages/Organizations/screens/Organization/__snapshots__/OrganizationNotifications.test.jsx.snap b/__tests__/pages/Organizations/screens/Organization/__snapshots__/OrganizationNotifications.test.jsx.snap index 093b1885d8..b0ca8513a9 100644 --- a/__tests__/pages/Organizations/screens/Organization/__snapshots__/OrganizationNotifications.test.jsx.snap +++ b/__tests__/pages/Organizations/screens/Organization/__snapshots__/OrganizationNotifications.test.jsx.snap @@ -217,7 +217,6 @@ exports[` initially renders succesfully 1`] = ` } } onSelectAll={null} - paginationStyling={null} qsConfig={ Object { "defaultParams": Object { @@ -281,6 +280,7 @@ exports[` initially renders succesfully 1`] = ` ] } isAllSelected={false} + noLeftMargin={false} onSearch={[Function]} onSelectAll={null} onSort={[Function]} @@ -2075,91 +2075,6 @@ exports[` initially renders succesfully 1`] = ` - - - - - - - @@ -2635,91 +2550,6 @@ exports[` initially renders succesfully 1`] = ` - - - - - - - @@ -2729,213 +2559,525 @@ exports[` initially renders succesfully 1`] = ` - - -
-
- Items Per Page - - 50 - , - - 25 - , - - 10 - , - ] - } - isOpen={false} - isPlain={false} - onSelect={[Function]} - onToggle={[Function]} - position="left" - toggle={ - - 5 - - } +
-
- - -
- } - > - - -
+ + , + + 10 + + per page + + , + + 20 + + per page + + , + + 50 + + per page + + , + ] + } + isOpen={false} + isPlain={true} + onSelect={[Function]} + position="left" + toggle={ + } + > +
+ +
+ + + 1 + - + 2 + + of + + 2 + + + items + + +
+
+ } + toggleTemplate={[Function]} + widgetId="pagination-options-menu" + > +
+ + + + 1 + - + 2 + + of + + 2 + + + items + + + +
+ +
+ +
+ + + + - - - + + + - + diff --git a/src/components/AddRole/SelectResourceStep.jsx b/src/components/AddRole/SelectResourceStep.jsx index 01e10a2394..4f2668edce 100644 --- a/src/components/AddRole/SelectResourceStep.jsx +++ b/src/components/AddRole/SelectResourceStep.jsx @@ -8,14 +8,6 @@ import CheckboxListItem from '../ListItem'; import SelectedList from '../SelectedList'; import { getQSConfig, parseNamespacedQueryString } from '../../util/qs'; -const paginationStyling = { - paddingLeft: '0', - justifyContent: 'flex-end', - borderRight: '1px solid #ebebeb', - borderBottom: '1px solid #ebebeb', - borderTop: '0' -}; - class SelectResourceStep extends React.Component { constructor (props) { super(props); @@ -124,7 +116,6 @@ class SelectResourceStep extends React.Component { )} alignToolbarLeft showPageSizeOptions={false} - paginationStyling={paginationStyling} /> )} diff --git a/src/components/Lookup/Lookup.jsx b/src/components/Lookup/Lookup.jsx index 4b147e552b..64f59367a0 100644 --- a/src/components/Lookup/Lookup.jsx +++ b/src/components/Lookup/Lookup.jsx @@ -18,14 +18,6 @@ import CheckboxListItem from '../ListItem'; import SelectedList from '../SelectedList'; import { getQSConfig, parseNamespacedQueryString } from '../../util/qs'; -const paginationStyling = { - paddingLeft: '0', - justifyContent: 'flex-end', - borderRight: '1px solid #ebebeb', - borderBottom: '1px solid #ebebeb', - borderTop: '0' -}; - class Lookup extends React.Component { constructor (props) { super(props); @@ -186,7 +178,6 @@ class Lookup extends React.Component { )} alignToolbarLeft showPageSizeOptions={false} - paginationStyling={paginationStyling} /> {lookupSelectedItems.length > 0 && ( {error && ( - {error && ( - -
{error.message}
- {error.response && ( -
{error.response.data.detail}
- )} - // TODO: replace with proper error handling - )} - {items.length === 0 ? ( - - - - {i18n._(t`No ${ucFirst(itemNamePlural || pluralize(itemName))} Found`)} - - - {i18n._(t`Please add ${getArticle(itemName)} ${itemName} to populate this list`)} - - - ) : ( - - { }} - onSort={this.handleSort} - showSelectAll={showSelectAll} - isAllSelected={isAllSelected} - onSelectAll={onSelectAll} - additionalControls={additionalControls} - noLeftMargin={alignToolbarLeft} - /> - - {items.map(item => (renderItem ? renderItem(item) : ( - - - - - - - {item.name} - - - - - ]} - /> - - - )))} - - - +
{error.message}
+ {error.response && ( +
{error.response.data.detail}
)} // TODO: replace with proper error handling )} @@ -211,6 +147,7 @@ class PaginatedDataList extends React.Component { isAllSelected={isAllSelected} onSelectAll={onSelectAll} additionalControls={additionalControls} + noLeftMargin={alignToolbarLeft} /> {items.map(item => (renderItem ? renderItem(item) : ( @@ -239,11 +176,18 @@ class PaginatedDataList extends React.Component { )))} )} @@ -276,7 +220,6 @@ PaginatedDataList.propTypes = { onSelectAll: PropTypes.func, alignToolbarLeft: PropTypes.bool, showPageSizeOptions: PropTypes.bool, - paginationStyling: PropTypes.shape(), }; PaginatedDataList.defaultProps = { @@ -290,7 +233,6 @@ PaginatedDataList.defaultProps = { onSelectAll: null, alignToolbarLeft: false, showPageSizeOptions: true, - paginationStyling: null, }; export { PaginatedDataList as _PaginatedDataList }; diff --git a/src/components/Pagination/Pagination.jsx b/src/components/Pagination/Pagination.jsx index 9058648177..fb481e6b65 100644 --- a/src/components/Pagination/Pagination.jsx +++ b/src/components/Pagination/Pagination.jsx @@ -1,245 +1,36 @@ -import React, { Component } from 'react'; -import PropTypes from 'prop-types'; -import { withI18n } from '@lingui/react'; +import React from 'react'; +import styled, { css } from 'styled-components'; +import { Pagination as PFPagination } from '@patternfly/react-core'; +import { I18n } from '@lingui/react'; import { t } from '@lingui/macro'; -import { - Button, - Dropdown, - DropdownDirection, - DropdownItem, - DropdownToggle, - TextInput -} from '@patternfly/react-core'; -class Pagination extends Component { - constructor (props) { - super(props); +const AWXPagination = styled(PFPagination)` + ${props => (props.perPageOptions && !props.perPageOptions.length && css` + .pf-c-options-menu__toggle-button { + display: none; + } + `)} +`; - const { page } = props; - this.state = { value: page, isOpen: false }; - - this.onPageChange = this.onPageChange.bind(this); - this.onSubmit = this.onSubmit.bind(this); - this.onFirst = this.onFirst.bind(this); - this.onPrevious = this.onPrevious.bind(this); - this.onNext = this.onNext.bind(this); - this.onLast = this.onLast.bind(this); - this.onTogglePageSize = this.onTogglePageSize.bind(this); - this.onSelectPageSize = this.onSelectPageSize.bind(this); - } - - componentDidUpdate (prevProps) { - const { page } = this.props; - - if (prevProps.page !== page) { - this.onPageChange(page); - } - } - - onPageChange (value) { - this.setState({ value }); - } - - onSubmit (event) { - const { onSetPage, page, pageCount, page_size } = this.props; - const { value } = this.state; - - event.preventDefault(); - - // eslint-disable-next-line no-bitwise - const isPositiveInteger = value >>> 0 === parseFloat(value) && parseInt(value, 10) > 0; - const isValid = isPositiveInteger && parseInt(value, 10) <= pageCount; - - if (isValid) { - onSetPage(value, page_size); - } else { - this.setState({ value: page }); - } - } - - onFirst () { - const { onSetPage, page_size } = this.props; - - onSetPage(1, page_size); - } - - onPrevious () { - const { onSetPage, page, page_size } = this.props; - const previousPage = page - 1; - - onSetPage(previousPage, page_size); - } - - onNext () { - const { onSetPage, page, page_size } = this.props; - const nextPage = page + 1; - - onSetPage(nextPage, page_size); - } - - onLast () { - const { onSetPage, pageCount, page_size } = this.props; - - onSetPage(pageCount, page_size); - } - - onTogglePageSize (isOpen) { - this.setState({ isOpen }); - } - - onSelectPageSize ({ target }) { - const { onSetPage } = this.props; - const page = 1; - const page_size = parseInt(target.innerText || target.textContent, 10); - - this.setState({ isOpen: false }); - - onSetPage(page, page_size); - } - - render () { - const { up } = DropdownDirection; - const { - count, - page, - pageCount, - page_size, - pageSizeOptions, - showPageSizeOptions, - style, - i18n - } = this.props; - const { value, isOpen } = this.state; - let opts = []; - if (pageSizeOptions) { - opts = pageSizeOptions.slice().reverse().filter(o => o !== page_size); - } - const isOnFirst = page === 1; - const isOnLast = page === pageCount; - - let itemCount; - if (!isOnLast || count === page_size) { - itemCount = page_size; - } else { - itemCount = count % page_size; - } - const itemMin = ((page - 1) * page_size) + 1; - const itemMax = itemMin + itemCount - 1; - - const dropdownItems = opts.map(option => ( - - {option} - - )); - - return ( -
- {showPageSizeOptions && ( -
- {i18n._(t`Items Per Page`)} - - {page_size} - - )} - dropdownItems={dropdownItems} - /> -
- )} -
-
- {i18n._(t`Items ${itemMin} – ${itemMax} of ${count}`)} -
- {pageCount !== 1 && ( -
-
- - -
-
- {i18n._(t`Page `)} - - {i18n._(t` of ${pageCount}`)} - -
- - -
-
- )} -
-
- ); - } -} - -Pagination.propTypes = { - count: PropTypes.number, - onSetPage: PropTypes.func.isRequired, - page: PropTypes.number.isRequired, - pageCount: PropTypes.number, - pageSizeOptions: PropTypes.arrayOf(PropTypes.number), - page_size: PropTypes.number.isRequired, - showPageSizeOptions: PropTypes.bool -}; - -Pagination.defaultProps = { - count: null, - pageCount: null, - pageSizeOptions: [5, 10, 25, 50], - showPageSizeOptions: true -}; - -export default withI18n()(Pagination); +export default (props) => ( + + {({ i18n }) => ( + + )} + +); diff --git a/src/components/Pagination/styles.scss b/src/components/Pagination/styles.scss deleted file mode 100644 index e238b25266..0000000000 --- a/src/components/Pagination/styles.scss +++ /dev/null @@ -1,88 +0,0 @@ -.awx-pagination { - --awx-pagination--BackgroundColor: var(--pf-global--BackgroundColor--light-100); - --awx-pagination--BorderColor: #dbdbdb; - --awx-pagination--disabled-BackgroundColor: #f2f2f2; - --awx-pagination--disabled-Color: #C2C2CA; - - border-top: 1px solid var(--awx-pagination--BorderColor); - border-bottom: 1px solid var(--awx-pagination--BorderColor); - background-color: var(--awx-pagination--BackgroundColor); - height: 55px; - display: flex; - align-items: center; - justify-content: space-between; - padding: 0 20px; - font-size: 14px; - font-weight: bold; - - --pf-global--target-size--MinHeight: 30px; - --pf-global--target-size--MinWidth: 30px; - --pf-global--FontSize--md: 14px; - - .awx-pagination__page-size-selection .pf-c-dropdown__toggle { - font-weight: bold; - margin-left: 10px; - } - - .awx-pagination__counts { - display: flex; - align-items: center; - margin-right: -20px; - } - - .awx-pagination__item-count { - margin-right: 20px; - } - - .awx-pagination__page-count { - margin-left: -10px; - display: flex; - align-items: center; - } - - .awx-pagination__page-input-form { - display: flex; - align-items: center; - margin: 0 10px; - white-space: nowrap; - } - - .awx-pagination__page-input { - width: 35px; - margin: 0 10px; - font-weight: bold; - } - - .awx-pagination__page-button { - width: 55px; - height: 55px; - } - - .pf-c-input-group .awx-pagination__page-button, - .pf-c-input-group .awx-pagination__page-button:after { - border: 0; - border-radius: 0; - } - - .pf-c-input-group .pf-c-button { - border-left: 1px solid var(--awx-pagination--BorderColor); - } - - .pf-c-input-group { - height: 56px; - } - - .pf-c-input-group.pf-m-previous { - border-right: 1px solid var(--awx-pagination--BorderColor); - } - - .pf-c-input-group .pf-c-button.pf-m-disabled { - border: 1px solid var(--awx-pagination--BorderColor); - background-color: var(--awx-pagination--disabled-BackgroundColor); - color: var(--awx-pagination--disabled-Color); - } - - .pf-c-input-group.pf-m-previous .pf-c-button.pf-m-disabled { - border-right: 0; - } -} diff --git a/src/index.jsx b/src/index.jsx index 4b961c6405..bf063d9b22 100644 --- a/src/index.jsx +++ b/src/index.jsx @@ -12,7 +12,6 @@ import { t } from '@lingui/macro'; import '@patternfly/react-core/dist/styles/base.css'; import './app.scss'; -import './components/Pagination/styles.scss'; import './components/SelectedList/styles.scss'; import './components/AddRole/styles.scss'; From 7ff7517bdf25f5ea5e6ec33d262d0c74da0a4fb0 Mon Sep 17 00:00:00 2001 From: Keith Grant Date: Fri, 17 May 2019 07:47:17 -0700 Subject: [PATCH 2/2] change pagination drop direction to up --- .../OrganizationNotifications.test.jsx.snap | 12 +++++++----- src/components/Pagination/Pagination.jsx | 3 ++- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/__tests__/pages/Organizations/screens/Organization/__snapshots__/OrganizationNotifications.test.jsx.snap b/__tests__/pages/Organizations/screens/Organization/__snapshots__/OrganizationNotifications.test.jsx.snap index b0ca8513a9..912c791a0a 100644 --- a/__tests__/pages/Organizations/screens/Organization/__snapshots__/OrganizationNotifications.test.jsx.snap +++ b/__tests__/pages/Organizations/screens/Organization/__snapshots__/OrganizationNotifications.test.jsx.snap @@ -2592,6 +2592,7 @@ exports[` initially renders succesfully 1`] = ` withHash={true} > initially renders succesfully 1`] = ` variant="bottom" > initially renders succesfully 1`] = ` > initially renders succesfully 1`] = ` className="pf-c-pagination pf-m-footer Pagination__AWXPagination-c2dclb-0 dXBcqV" > initially renders succesfully 1`] = ` initially renders succesfully 1`] = ` } >
initially renders succesfully 1`] = ` optionsToggle="Select" parentRef={
( currPage: i18n._(t`Current page`), paginationTitle: i18n._(t`Pagination`) }} + dropDirection={DropdownDirection.up} {...props} /> )}