fix input clearing ux#106
Conversation
![constructor-claude-bedrock[bot]](https://avatars.githubusercontent.com/u/9828194?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This PR fixes the UX behavior where the Input field was always cleared after submission, adding logic to retain the pre-filled value when the user submits it unchanged — the implementation is mostly correct but has two subtle bugs that could cause unexpected behavior.
Inline comments: 4 discussions added
Overall Assessment:
| if (value.trim()) { | ||
| onSubmit(value.trim()); | ||
| setValue(''); | ||
| if (value.trim() !== providedValue) { |
There was a problem hiding this comment.
Important Issue: value is trimmed before comparison but providedValue is not. If providedValue has any leading/trailing whitespace (e.g. " blue? "), value.trim() will never equal it, causing the field to clear on every submit even when the user typed nothing new. Conversely, if the user pads the pre-filled text with spaces, value.trim() would equal an un-padded providedValue, preventing the expected clear.
Fix: trim both sides:
if (value.trim() !== providedValue?.trim()) {| expect(button).toBeDisabled(); | ||
| }); | ||
|
|
||
| it('retains value after submit when it matches providedValue (default mode)', () => { |
There was a problem hiding this comment.
Suggestion: The test description says "default mode" but the behavior being tested is actually specific to the case where value prop is provided. Consider renaming to 'retains value after submit when input matches the provided value prop' to make the condition explicit and avoid ambiguity with "default" (no value prop at all).
| expect(input.value).toBe('Does this come in blue?'); | ||
| }); | ||
|
|
||
| it('clears value after submit when it differs from providedValue', () => { |
There was a problem hiding this comment.
Suggestion: The new tests don't cover the scenario where no value prop is supplied (i.e. providedValue is undefined). In that case, value.trim() !== providedValue is always true, so the clear path is always taken — but this is incidental rather than explicit. Adding a test like 'clears value after submit when no value prop is provided' would make that contract explicit and guard against regressions.
1 participant
Pull Request Checklist
Before you submit a pull request, please make sure you have to following:
PR Type
What kind of change does this PR introduce?