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

Slimmer pagination look / feel #380

Merged
merged 4 commits into from
Feb 9, 2018
Merged

Conversation

snide
Copy link
Contributor

@snide snide commented Feb 8, 2018

A slimmer pagination style requested by nearly everyone. cc @uboness

TODO

  • Make a full react component and use refs to properly assign focus state so we don't have the weird jumping with large pagination sets. Will do in later PR.
  • Compressed format

@snide snide mentioned this pull request Feb 8, 2018
@snide snide requested a review from cchaos February 9, 2018 00:04
@uboness
Copy link
Contributor

uboness commented Feb 9, 2018

looks great.... how does it look like when there are 20 pages and you're now at page 10?

Also, it would be nice if you can pass in a showPageNumbers property that will enable to only show the arrows - this can be useful with using pagination in limited small places (e.g. dashboard panels)

@snide
Copy link
Contributor Author

snide commented Feb 9, 2018

@uboness Good idea on the small one. Should be able to make that happen pretty easily.

@cchaos
Copy link
Contributor

cchaos commented Feb 9, 2018

What about keeping the underline for active page?

screen shot 2018-02-09 at 13 31 17 pm

@snide
Copy link
Contributor Author

snide commented Feb 9, 2018

@cchaos that works for me. I can make that change. If you have a minute, feel free to review the code as well. There's not much to this PR so it should be fine, but I think I'm gonna merge it (before doing the ref changes) and then do that stuff in a followup PR.

@cchaos
Copy link
Contributor

cchaos commented Feb 9, 2018

Visually the other thing I saw was that not all focus states had rounded corners:

screen capture on 2018-02-09 at 13-37-20

@snide
Copy link
Contributor Author

snide commented Feb 9, 2018

@cchaos Good spot, fixed.

Copy link
Contributor

@cchaos cchaos left a comment

Choose a reason for hiding this comment

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

Code LGTM

@snide snide merged commit 693365b into elastic:master Feb 9, 2018
@snide snide deleted the style/pagination branch February 9, 2018 18:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants