-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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
feat(paginator): Add functionality to jump to first and last page #9603
feat(paginator): Add functionality to jump to first and last page #9603
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, great PR.
Will leave final review to @andrewseguin
src/lib/paginator/paginator.html
Outdated
[disabled]="!hasPreviousPage()" | ||
*ngIf="showFirstLastButtons"> | ||
<div class="mat-paginator-first"></div> | ||
<div class="mat-paginator-decrement"></div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: indentation with these 2 divs is inconsistent with the rest
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job, thanks for adding this feature and following the right styles/structure! We would definitely welcome any more quality contributions with any other bugs and features in the library :)
Just one nit about the demo app, and Will's indent comment.
After responding to those two comments, just add a comment and we can add the merge: ready
label
@andrewseguin did you forget to leave your nit comment? I only see one comment above (you mentioned "two") and no rebases have been performed. (random guy reading through PRs everyday) |
Thanks very much @andrewseguin! Good spot @willshowell I'll have fixed that now. @andrewseguin what is the nit for the demo app and how should I fix it? I have brought the branch up to speed, however, I believe it is still blocked by #9597? PS Thanks very much @andrewseguin for guiding me, you've made it really easy to submit the PR |
e3b27dc
to
c6fdd9e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops, looks like my review didn't get sent out, just one small comment about the demo
src/demo-app/table/table-demo.html
Outdated
(toggleColorColumn)="toggleColorColumn()"> | ||
</table-header-demo> | ||
|
||
<mat-table [dataSource]="dataSource" [trackBy]="userTrackBy"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the data source for this table doesn't update with the paginator, we can go ahead and just remove it. Maybe replace it with a JSON text of the latest paginator output?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, if you can fit in a similar change as #9629, that'd be helpful. Right now the inputs require a boolean or "true" when they really should be set to true just by being present as attributes |
28fa11e
to
25be196
Compare
Completed the nits, however I believe it's still blocked by #9597 |
Add two buttons which allow you to jump to the first and last page. Hidden by default and can be toggled with showHideFirstButtons Fixes angular#9278
25be196
to
fa07f14
Compare
Any thoughts on why the travis-ci failed? |
No issues, just a flaky test. I re-ran and you're good to go. Next steps will be running this against some internal tests at Google to make sure nothing breaks. I don't expect any issues. We'll do the same with #9597. Ideally we can get them in this week |
Awesome! |
@andrewseguin please add release label for this pr. |
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Add two buttons which allow you to jump to the first and last page.
Hidden by default and can be toggled with showHideFirstButtons
Fixes #9278, contains the following fixes and is therefore blocked by: #9597 and #9604 (and therefore also #9584)