Skip to content

Commit

Permalink
prev and next button should be a button (#282)
Browse files Browse the repository at this point in the history
close #280
  • Loading branch information
afc163 authored Jun 19, 2020
1 parent 3a0ee0d commit 3094ee1
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 45 deletions.
24 changes: 13 additions & 11 deletions src/Options.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,19 @@ class Options extends React.Component {
return !goInputText || isNaN(goInputText) ? current : Number(goInputText);
}

buildOptionText = value => `${value} ${this.props.locale.items_per_page}`;
buildOptionText = (value) => `${value} ${this.props.locale.items_per_page}`;

changeSize = value => {
changeSize = (value) => {
this.props.changeSize(Number(value));
};

handleChange = e => {
handleChange = (e) => {
this.setState({
goInputText: e.target.value,
});
};

handleBlur = e => {
handleBlur = (e) => {
const { goButton, quickGo, rootPrefixCls } = this.props;
if (goButton) {
return;
Expand All @@ -44,7 +44,7 @@ class Options extends React.Component {
quickGo(this.getValidValue());
};

go = e => {
go = (e) => {
const { goInputText } = this.state;
if (goInputText === '') {
return;
Expand All @@ -58,11 +58,12 @@ class Options extends React.Component {
};

getPageSizeOptions() {
const {
pageSize,
pageSizeOptions,
} = this.props;
if (pageSizeOptions.some(option => option.toString() === pageSize.toString())) {
const { pageSize, pageSizeOptions } = this.props;
if (
pageSizeOptions.some(
(option) => option.toString() === pageSize.toString(),
)
) {
return pageSizeOptions;
}
return pageSizeOptions.concat([pageSize.toString()]).sort((a, b) => {
Expand Down Expand Up @@ -117,7 +118,7 @@ class Options extends React.Component {
dropdownMatchSelectWidth={false}
value={(pageSize || pageSizeOptions[0]).toString()}
onChange={this.changeSize}
getPopupContainer={triggerNode => triggerNode.parentNode}
getPopupContainer={(triggerNode) => triggerNode.parentNode}
>
{options}
</Select>
Expand All @@ -133,6 +134,7 @@ class Options extends React.Component {
onClick={this.go}
onKeyUp={this.go}
disabled={disabled}
className={`${prefixCls}-quick-jumper-button`}
>
{locale.jump_to_confirm}
</button>
Expand Down
23 changes: 18 additions & 5 deletions src/Pagination.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -133,10 +133,15 @@ class Pagination extends React.Component {
* @param {React.ReactNode | React.ComponentType<PaginationProps>} icon received icon.
* @returns {React.ReactNode}
*/
getItemIcon = (icon) => {
getItemIcon = (icon, label) => {
const { prefixCls } = this.props;
// eslint-disable-next-line jsx-a11y/anchor-has-content
let iconNode = icon || <a className={`${prefixCls}-item-link`} />;
let iconNode = icon || (
<button
type="button"
label={label}
className={`${prefixCls}-item-link`}
/>
);
if (typeof icon === 'function') {
iconNode = React.createElement(icon, { ...this.props });
}
Expand Down Expand Up @@ -321,7 +326,11 @@ class Pagination extends React.Component {

renderPrev(prevPage) {
const { prevIcon, itemRender } = this.props;
const prevButton = itemRender(prevPage, 'prev', this.getItemIcon(prevIcon));
const prevButton = itemRender(
prevPage,
'prev',
this.getItemIcon(prevIcon, 'prev page'),
);
const disabled = !this.hasPrev();
return isValidElement(prevButton)
? cloneElement(prevButton, { disabled })
Expand All @@ -330,7 +339,11 @@ class Pagination extends React.Component {

renderNext(nextPage) {
const { nextIcon, itemRender } = this.props;
const nextButton = itemRender(nextPage, 'next', this.getItemIcon(nextIcon));
const nextButton = itemRender(
nextPage,
'next',
this.getItemIcon(nextIcon, 'next page'),
);
const disabled = !this.hasNext();
return isValidElement(nextButton)
? cloneElement(nextButton, { disabled })
Expand Down
31 changes: 9 additions & 22 deletions tests/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,9 @@ describe('Uncontrolled Pagination', () => {
it('should quick jump to expect page', () => {
const quickJumper = wrapper.find('.rc-pagination-options-quick-jumper');
const input = quickJumper.find('input');
const goButton = quickJumper.find('button');
const goButton = quickJumper.find(
'.rc-pagination-options-quick-jumper-button',
);
input.simulate('change', { target: { value: '2' } });
goButton.simulate('click');
expect(wrapper.state().current).toBe(2);
Expand Down Expand Up @@ -213,7 +215,7 @@ describe('Other props', () => {
});

describe('hideOnSinglePage props', () => {
const itemRender = current => <a href={`#${current}`}>{current}</a>;
const itemRender = (current) => <a href={`#${current}`}>{current}</a>;

it('should hide pager if hideOnSinglePage equals true', () => {
const wrapper = mount(
Expand Down Expand Up @@ -261,15 +263,10 @@ describe('Other props', () => {
expect(wrapper.find('.rc-pagination-disabled').exists()).toBe(true);
expect(wrapper.find('input').exists()).toBe(true);
expect(wrapper.find(Select).props().disabled).toBe(true);
expect(wrapper.find('input').at(0).getDOMNode().disabled).toBe(true);
expect(
wrapper
.find('input')
.at(0)
.getDOMNode().disabled,
).toBe(true);
expect(
wrapper
.find('button')
.find('.rc-pagination-options-quick-jumper-button')
.at(0)
.getDOMNode().disabled,
).toBe(true);
Expand Down Expand Up @@ -339,17 +336,11 @@ describe('current value on onShowSizeChange when total is 0', () => {

it('size changer show logic', () => {
const wrapper1 = mount(
<Pagination
selectComponentClass={Select}
total={50}
/>,
<Pagination selectComponentClass={Select} total={50} />,
);
expect(wrapper1.exists('.rc-pagination-options-size-changer')).toBe(false);
const wrapper2 = mount(
<Pagination
selectComponentClass={Select}
total={51}
/>,
<Pagination selectComponentClass={Select} total={51} />,
);
expect(wrapper2.exists('.rc-pagination-options-size-changer')).toBe(true);
const wrapper3 = mount(
Expand All @@ -361,11 +352,7 @@ describe('current value on onShowSizeChange when total is 0', () => {
);
expect(wrapper3.exists('.rc-pagination-options-size-changer')).toBe(false);
const wrapper4 = mount(
<Pagination
selectComponentClass={Select}
showSizeChanger
total={50}
/>,
<Pagination selectComponentClass={Select} showSizeChanger total={50} />,
);
expect(wrapper4.exists('.rc-pagination-options-size-changer')).toBe(true);
});
Expand Down
32 changes: 25 additions & 7 deletions tests/jumper.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,13 @@ describe('simple quick jumper', () => {
onChange={onChange}
defaultCurrent={1}
total={25}
showQuickJumper={{ goButton: <button type="button">go</button> }}
showQuickJumper={{
goButton: (
<button type="button" className="go-button">
go
</button>
),
}}
showTotal={(total, range) =>
`${range[0]} - ${range[1]} of ${total} items`
}
Expand All @@ -67,7 +73,7 @@ describe('simple quick jumper', () => {
it('should quick jump to expect page', () => {
const quickJumper = wrapper.find('.rc-pagination-simple');
const input = quickJumper.find('input');
const goButton = quickJumper.find('button');
const goButton = quickJumper.find('.go-button');
input.simulate('change', { target: { value: '2' } });
goButton.simulate('click');
expect(wrapper.state().current).toBe(2);
Expand All @@ -81,7 +87,13 @@ describe('simple quick jumper', () => {
onChange={onChange}
defaultCurrent={1}
total={25}
showQuickJumper={{ goButton: <button type="button">go</button> }}
showQuickJumper={{
goButton: (
<button type="button" className="go-button">
go
</button>
),
}}
showTotal={(total, range) =>
`${range[0]} - ${range[1]} of ${total} items`
}
Expand All @@ -92,7 +104,7 @@ describe('simple quick jumper', () => {
it('should quick jump to expect page', () => {
const quickJumper = wrapper.find('.rc-pagination-options-quick-jumper');
const input = quickJumper.find('input');
const goButton = quickJumper.find('button');
const goButton = quickJumper.find('.go-button');
input.simulate('change', { target: { value: '2' } });
goButton.simulate('click');
expect(wrapper.state().current).toBe(2);
Expand Down Expand Up @@ -120,7 +132,9 @@ describe('simple quick jumper', () => {
showQuickJumper={{ goButton: true }}
/>,
);
expect(wrapper.find('button').exists()).toBe(true);
expect(
wrapper.find('.rc-pagination-options-quick-jumper-button').exists(),
).toBe(true);
});

it('goButton defaultly hidden', () => {
Expand All @@ -132,7 +146,9 @@ describe('simple quick jumper', () => {
showQuickJumper
/>,
);
expect(wrapper.find('button').exists()).toBe(false);
expect(
wrapper.find('.rc-pagination-options-quick-jumper-button').exists(),
).toBe(false);
});

it('goButton could be false', () => {
Expand All @@ -144,6 +160,8 @@ describe('simple quick jumper', () => {
showQuickJumper={{ goButton: false }}
/>,
);
expect(wrapper.find('button').exists()).toBe(false);
expect(
wrapper.find('.rc-pagination-options-quick-jumper-button').exists(),
).toBe(false);
});
});

3 comments on commit 3094ee1

@decimoseptimo
Copy link

@decimoseptimo decimoseptimo commented on 3094ee1 Jun 29, 2020

Choose a reason for hiding this comment

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

@afc163 You forgot to add the styles for button (e.g. the left and right buttons are empty, missing the left and right arrows).

As in:

        .rc-pagination-prev button:after {
          content: "\203A";
          display: block;
        }
        .rc-pagination-next button:after {
          content: "\2039";
          display: block;
        }

@afc163
Copy link
Member Author

@afc163 afc163 commented on 3094ee1 Jun 30, 2020

Choose a reason for hiding this comment

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

@decimoseptimo PR welcome

@decimoseptimo
Copy link

@decimoseptimo decimoseptimo commented on 3094ee1 Jun 30, 2020 via email

Choose a reason for hiding this comment

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

Please sign in to comment.