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(pagination): initial value of pageSize #1015

Merged
merged 2 commits into from
Oct 4, 2019

Conversation

andrew-frueh
Copy link
Collaborator

Stops initial value of pageSize from being overwritten to the default. I starred at this one for hours thinking it had something to do with base class inheritance. Turns out it just had to do with the two-way binding being broken; the pageSize setter wasn't emitting on a change.

closes #776

stops initial value of pageSize from being overwritten to the default

closes HealthCatalyst#776
@@ -52,6 +52,7 @@ export class BasePaginationComponent extends Initializable implements OnInit {
set pageSize(value: number) {
this._pageSize = coerceNumberProperty(value);
this._pageSizeUpdated();
this._emitPageEvent(this.pageNumber);
Copy link
Contributor

Choose a reason for hiding this comment

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

🤦‍♂ I can't believe it was that simple!

Copy link
Contributor

Choose a reason for hiding this comment

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

@andrew-frueh looks like you broke some tests though... might want to look into that

corrects behavior of pageNumberChange and pageSizeChange
@andrew-frueh
Copy link
Collaborator Author

Thank goodness for those unit tests @joeskeen! Turns out that because the two-way binding was broken, we never noticed that the page EventEmitter wasn't emitting on pageSize changes. And pageNumberChange and pageSizeChange would emit on any page event regardless of whether their specific value changed or not. So behavior of both of those have been corrected - which gets the unit tests to passing.

Thanks for being so thorough when you wrote those!!

@andrew-frueh andrew-frueh merged commit da0cd4b into HealthCatalyst:dev Oct 4, 2019
@andrew-frueh andrew-frueh deleted the pagesize-binding branch October 4, 2019 21:26
@benjanderson
Copy link
Contributor

🎉 This PR is included in version 6.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

(pagination) page size is set by component before value is able to be bound
3 participants