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

fix(table-pager): add flexible width when active page number is large #694

Merged

Conversation

gcruchon
Copy link
Contributor

@gcruchon gcruchon commented Oct 6, 2020

Related issue

Reference to the issue

#689

Description of the issue

When the table had 4-digit number of pages, the active page was truncated and blue bullet not centered

Person(s) for reviewing proposed changes

@samuel-gomez @guillaumechervet @youf-olivier @LeGrandKhan

@gcruchon gcruchon force-pushed the fix/table-pager-large-number branch 2 times, most recently from 558bf5d to c7dc725 Compare October 6, 2020 13:01
arnaudforaison
arnaudforaison previously approved these changes Oct 6, 2020
Copy link
Contributor

@arnaudforaison arnaudforaison left a comment

Choose a reason for hiding this comment

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

LGTM

@gcruchon
Copy link
Contributor Author

gcruchon commented Oct 6, 2020

Hmmmm I rebased after @samuel-gomez merged PR and lost your approval, @arnaudforaison. Sorry.

@youf-olivier
Copy link
Contributor

Le rebase était vraiment obligatoire ?

@gcruchon
Copy link
Contributor Author

gcruchon commented Oct 6, 2020

That's my religion, @youf-olivier, I always rebase before submitting a PR.

To me:

  • it keeps the history much much much cleaner and more understandable (history being "passive documentation")
  • much safer (even more after a 450+ file PR just merged before mine)

@youf-olivier
Copy link
Contributor

De mémoire ca change rien, le merge se débrouille tout seul, mais t'as surement raison.

@gcruchon
Copy link
Contributor Author

gcruchon commented Oct 6, 2020

Rebase will make sure commits are one after another... while merges may look like this :-)
merge-toolkit

@gcruchon
Copy link
Contributor Author

gcruchon commented Oct 6, 2020

Anyway, it's your move to merge it now, I don't have the write-access on this repo :)

@youf-olivier youf-olivier merged commit 7810d89 into AxaFrance:master Oct 6, 2020
@samuel-gomez-axa samuel-gomez-axa mentioned this pull request Oct 7, 2020
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.

None yet

3 participants