Skip to content

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.

3 participants