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

Added Event Forwarding To Pagination Component #185

Merged

Conversation

mabentley85
Copy link
Contributor

Added Event Forwarding to the Pagination component by adding handlers to the Next & Previous Page buttons and the Page Size and Page select fields.

These handlers will dispatch one event containing page and pageSize details. This was done to prevent duplicate dispatches when the pageSize changes, as the page is always set to 1 when pageSize is changed.

Question: Is there a better way to handle select value changes than onBlur? Using onChange results in duplicate triggers since they are binded to the page and pageSize variables.

@mabentley85 mabentley85 requested a review from metonym as a code owner May 28, 2020 19:54
@metonym
Copy link
Collaborator

metonym commented May 29, 2020

Hi @mabentley85 – thank you for the PR. I think it would be useful to dispatch page and pageSize values.

Using on:blur is not the expected behavior because it will not fire immediately like on:change. To address the "double firing" from on:change, try using the afterUpdate to dispatch the values.

@mabentley85
Copy link
Contributor Author

@metonym Thanks for the hint, I refactored my changes to make use of the afterUpdate function.

Copy link
Collaborator

@metonym metonym left a comment

Choose a reason for hiding this comment

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

Much better – this is a lot more concise.

See my comment about converting the value to a number.

src/components/Pagination/Pagination.svelte Outdated Show resolved Hide resolved
@mabentley85
Copy link
Contributor Author

@metonym Made the requested change

@metonym metonym merged commit 5e20571 into carbon-design-system:master May 30, 2020
@metonym
Copy link
Collaborator

metonym commented May 30, 2020

Thanks again @mabentley85 for the submission!

Released in v0.7.0 – changelog notes

@mabentley85 mabentley85 deleted the pagination-event-forwarding branch June 1, 2020 17:40
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