Warning: Type of PR label mismatch

To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.

• Required label: Any label starting with [Type].
• Labels found: [Package] Components.

Read more about Type labels in Gutenberg. Don't worry if you don't have the required permissions to add labels; the PR reviewer should be able to help with the task.

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: aduth <aduth@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Size Change: +10 B (0%)

Total Size: 6.89 MB

_{compressed-size-action}

I'm investigating the test failure. Based on the test behavior, it makes sense that the invalid value would not be affected until loss of focus, but then this seems to me the purpose of isPressEnterToChange, which RangeControl does not use.

Flaky tests detected in 489c4bd.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/22971836094
📝 Reported issues:

• [Flaky Test] Custom Completer: should allow newlines after multiple option completions #48535 in /test/e2e/specs/editor/various/autocomplete-and-mentions.spec.js
• [Flaky Test] should undo bold #48558 in /test/e2e/specs/editor/various/undo.spec.js

I'm investigating the test failure. Based on the test behavior, it makes sense that the invalid value would not be affected until loss of focus, but then this seems to me the purpose of isPressEnterToChange, which RangeControl does not use.

So it might be right that RangeControl ought to be using isPressEnterToChange, but the actual issue here is different, and has to do with types of how props.value and draft.value are represented. Previously, it would be comparing based on draft values (both the "current" and "last known" values). After these changes, we're comparing props.value and draft.value. These should both be typed as string | undefined and therefore it's fine to compare, but in practice this isn't always the case.

For example, see how NumberControl bypasses type-checking and passes a number instead of a string:

This worked previously because it was always based on the draft value which comes from input.value, a string. But after these changes, we now have a scenario where it tries to compare a prop value (numeric -1.1) with the draft value (string '-1.1'), which aren't the same.

Ultimately, this feels like it's an issue with how NumberControl passes its value as the wrong type.

Okay, after testing this in the browser and seeing that the behavior asserted in the tests was the behavior I was seeing in the browser, I discovered that updating the test to use userEvent passes. I think it may be that because we were emitting raw change events with values that would never actually in a real browser (like, for example, being able to enter a value of -1.1 if the min is -1).

Separately, it's still true that NumberControl should be passing a value in the correct type string | undefined and stop passing a number. I updated this in b7b4174.

The fact that NumberControl is misusing the InputControl value prop like this does have me concerned if there are other components or third-party usage that might be doing the same. We could maybe be a bit more defensive here and coerce the non-undefined prop value to a string in the same way we might expect it to be reflected in draft.value from input.value.

I'll mark this as ready for review, but it's not quite as urgent to land given that #76400 now includes a solution that doesn't directly rely on this fix.

The failing test in ColorPicker is another example of a knock-on effect 😕 Likely due to timing of internal debouncing. It might be back to the drawing board for how best to approach this problem in a way that retroactively handles all the components that depend on the current behavior.