Pagination: onClick don't work for ellipsisItem

[sposterkil]

Feature Request

Problem description

Pagination.Item type="ellipsisItem"s do not support the onClick handler. This prevents me from easily using the ellipsis button as a "jump several pages" button. An onClick handler can be attached to the content of the Pagination.Item, but this requires CSS overrides to have the entire button be clickable.

Proposed solutions

1. Pagination.Items fire their onClick handlers even if they are ellipsisItems.
   Upsides: Maximum consistency and simplicity - onClick simply works as normal on everything inside a Pagination.
   Downsides: This is a breaking change, in that anyone depending on the previous behavior (for instance, using a common props shorthand for custom ellipsisItem and next/prevItem) will have to go change their code to not supply that handler for ellipsisItems.
2. Pagination.Items offer a second onEllipsisClick prop that is onClick for ellipsisItems only.
   Upsides: Maintains backwards compatibility, nobody has to change any code.
   Downsides: Less consistent, requires you to look at the docs to understand how to handle a click on an ellipsisItem.

MVP

For solution 1, replace handleClick in addons/Pagination/PaginationItem.js with this code:

handleClick = (e) => {
  _.invoke(this.props, 'onClick', e, this.props)
}

For solution 2, replace it with this code:

handleClick = (e) => {
  const { type } = this.props
  if (type === "ellipsisItem") {
    _.invoke(this.props, 'onEllipsisClick', e, this.props)
  } else {
    _.invoke(this.props, 'onClick', e, this.props)
  }
}

...update the propTypes to this:

static propTypes = {
  /** A pagination item can be active. */
  active: PropTypes.bool,

  /** A pagination item can be disabled. */
  disabled: PropTypes.bool,

  /**
   * Called on click for non-ellipsisItem items.
   *
   * @param {SyntheticEvent} event - React's original SyntheticEvent.
   * @param {object} data - All props.
   */
  onClick: PropTypes.func,

  /**
   * Called on on click if the type is "ellipsisItem".
   *
   * @param {SyntheticEvent} event - React's original SyntheticEvent.
   * @param {object} data - All props.
   */
  onEllipsisClick: PropTypes.func,

  /**
   * Called on key down.
   *
   * @param {SyntheticEvent} event - React's original SyntheticEvent.
   * @param {object} data - All props.
   */
  onKeyDown: PropTypes.func,

  /** A pagination should have a type. */
  type: PropTypes.oneOf([
    'ellipsisItem',
    'firstItem',
    'prevItem',
    'pageItem',
    'nextItem',
    'lastItem',
  ]),
}

...and replicate that new proptype into the type definition for PaginationItem.

I'll open a PR for whichever of these solutions the maintainers prefer, if they agree this is a worthwhile change 😄

[welcome]

👋 Thanks for opening your first issue here! If you're reporting a 🐞 bug, please make sure you've completed all the fields in the issue template so we can best help.

We get a lot of issues on this repo, so please be patient and we will get back to you as soon as we can.

[layershifter]

Actually, it's a design issue, the check for type === "ellipsisItem" should be placed inside Pagination component.

<Pagination
  defaultActivePage={5}
  ellipsisItem={{
    content: '...',
    onClick: (e, props) => console.log('ellipsisItem click', props),
  }}
  totalPages={10}
/>

In this case you will be able to pass your own handler. props object will contain all items props as our regular callback, so will be able to get props.value, too 👍

PR is coming.