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

UnitControl: avoid calling onChange callback twice when unit changes #40796

Merged

Conversation

ciampo
Copy link
Contributor

@ciampo ciampo commented May 3, 2022

What?

Prior to this PR, the onChange callback on UnitControl is called twice when the unit changes.

Why?

Just an error in the component's logic.

This bug was uncovered by the work done in #40790 (comment) while a solution was, at the same time, already being proposed by @stokesman as part of #40568.

This PR extracts the fix from #40568, while also updating the corresponding unit test.

How?

Removed the call to onChange from within the mayUpdateUnit function.

Testing Instructions

  • Updated unit tests pass
  • Pass both the onChange and onUnitChange callbacks to UnitControl. Interact with the component by changing the unit from the dropdown. Make sure that the both callbacks are called, but that onChange is called only once.

@ciampo ciampo self-assigned this May 3, 2022
@ciampo ciampo added [Type] Bug An existing feature does not function as intended [Package] Components /packages/components [Feature] Component System WordPress component system labels May 3, 2022
Copy link
Contributor

@stokesman stokesman left a comment

Choose a reason for hiding this comment

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

The changes make sense. Thanks for breaking this out as its own PR 🙌.

Prior to this PR, the onChange callback on UnitControl is called twice when the unit changes.

It's only called twice for unit changes triggered by typing into the text input. Or at least that's the only case this change pertains to but I think it's a detail that's not too important.

@ciampo ciampo merged commit b89145d into trunk May 3, 2022
@ciampo ciampo deleted the fix/unit-control-on-change-called-twice-when-unit-changes branch May 3, 2022 21:11
@github-actions github-actions bot added this to the Gutenberg 13.2 milestone May 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Component System WordPress component system [Package] Components /packages/components [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants