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

{WIP} Add pagination component #485

Closed
wants to merge 5 commits into from
Closed

Conversation

emplums
Copy link
Contributor

@emplums emplums commented May 9, 2018

Fixes: #421
Adds pagination component into primer 🎉

/cc @primer/ds-core

@emplums
Copy link
Contributor Author

emplums commented May 9, 2018

@primer/ds-core not sure if we should include updating the text to use $text-size-small (1px smaller) and the primer spacing system here... it makes quite a different in the overall look of the component and feels a bit off 🤔

em/a/span padding: 7px 12px -> 8px 16px ($spacer-2, $spacer-3)
em/a/span font-size: 12px -> 13px ($font-size-small)
paginate-container margin-top: 20px -> 24px ($spacer-4)
paginate-container margin-bottom: 15px -> 16px ($spacer-3)

Before:
image

After:
image

@emplums emplums requested a review from jonrohan May 9, 2018 22:13
@emplums
Copy link
Contributor Author

emplums commented May 9, 2018

Also - I merged dev into this branch so I could get storybook running - is that ok or should I revert that commit?

This reverts commit 710cc7c, reversing
changes made to 6511252.
@emplums emplums changed the base branch from release-10.5.0 to dev May 9, 2018 22:24
@whytrall
Copy link

whytrall commented May 9, 2018

Hey! Check out my comment at #421 (comment).

Again, I'm not sure if it's a good idea to add a new module instead of modifying button group and adding button-group-pagination.

@emplums emplums closed this May 9, 2018
@emplums
Copy link
Contributor Author

emplums commented May 9, 2018

Closing in favor of #487... messed up my commit history a bit!

@shawnbot shawnbot deleted the task/add-pagination-component branch October 17, 2018 20:42
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.

2 participants