CUI-5: Prevent implicit form submission on Enter for non-text elements#1014
Open
JeanMarcMilletScality wants to merge 2 commits into
Open
CUI-5: Prevent implicit form submission on Enter for non-text elements#1014JeanMarcMilletScality wants to merge 2 commits into
JeanMarcMilletScality wants to merge 2 commits into
Conversation
Contributor
Hello jeanmarcmilletscality,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Contributor
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
Peer approvals must include at least 1 approval from the following list: |
Comment on lines
+275
to
+289
| const TEXT_INPUT_TYPES = ['text', 'search', 'url', 'tel', 'password', 'email', 'number']; | ||
|
|
||
| const preventImplicitSubmission = ( | ||
| event: KeyboardEvent<HTMLFormElement>, | ||
| ) => { | ||
| if (event.key !== 'Enter') return; | ||
| const target = event.target as HTMLElement; | ||
| const isTextInput = | ||
| target instanceof HTMLInputElement && | ||
| TEXT_INPUT_TYPES.includes(target.type) && | ||
| !target.readOnly; | ||
| if (!isTextInput && !(target instanceof HTMLButtonElement)) { | ||
| event.preventDefault(); | ||
| } | ||
| }; |
Contributor
There was a problem hiding this comment.
I'm really unsure about this proposal. What if we have a select with search field (because we have more than 8 items) ? Then pressing enter on the search field trigger the submission.
Are we trying to solve a real problem in this PR ? What is described as a pb in here is a standard behavior I think.
Contributor
Branches have divergedThis pull request's source branch To avoid any integration risks, please re-synchronize them using one of the
Note: If you choose to rebase, you may have to ask me to rebuild |
The Form component now intercepts Enter keydown events and only allows form submission when the focused element is a writable text input or a button. This prevents custom widgets like Select from accidentally triggering form submission when users press Enter to interact with them.
Move the fix from the Form to the Select. The Select wraps react-select in a display: contents div with an onKeyDown that preventDefaults Enter, so Enter never escapes to trigger the browser's implicit submission — regardless of whether the inner input is readonly or a writable search field, and regardless of which form container the Select is in. The previous Form-level fix only blocked Enter on readonly inputs, so a searchable Select (>8 options) still submitted the form. It also did nothing when the Select was used inside a consumer's own <form>. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
JeanMarcMilletScality
force-pushed
the
bugfix/CUI-5-select-enter-prevents-default
branch
from
3886ecb to
fb383df
Compare
Contributor
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
Peer approvals must include at least 1 approval from the following list: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Pressing Enter on a
Selectinside any<form>triggered the browser's implicit form submission instead of being consumed by the combobox. Fixed by wrapping theSelect's react-select instance in adisplay: contentsdiv with anonKeyDownhandler that callspreventDefault()on Enter. The fix lives in the widget itself, so it works regardless of which form container theSelectis in.Ticket: https://scality.atlassian.net/browse/CUI-5
Changes
Selectv2.component.tsx: wrap react-select in adisplay: contentsdiv that preventDefaults Enterselectv2.test.tsx: add regression tests for non-searchable and searchable Select inside a plain<form>Form.test.tsx: keep the "plain text input still submits" test (Form's own contract); drop the Select-inside-Form test, which now lives next to the component that owns the behaviourDetails: why this solution, alternatives considered, references
Root cause
The browser's implicit submission mechanism submits a
<form>when Enter is pressed on a text-like input. React-select renders an internal<input>(readonly for non-searchable, writable for searchable) which the browser treats as a regular text input. React-select does not callpreventDefault()on Enter when the menu is closed, so the browser proceeds with implicit submission.Why fix this in the Select component
Per the WHATWG HTML spec, implicit submission is designed for text fields. The WAI-ARIA Combobox Pattern defines Enter on a combobox as "accept the autocomplete suggestion / pick the highlighted option"; it never delegates Enter to the surrounding form. A combobox is a composite widget that consumes Enter itself.
Putting the fix on the
<form>would mean every form container has to know about every composite widget that consumes Enter (Select today, DatePicker / Autocomplete / custom dropdowns tomorrow), and it would only work when theSelectis used inside our<Form>component — not when consumers wrap it in their own<form>. The widget owns its keyboard behaviour, so the fix belongs in the widget.Implementation
A
display: contentsdiv wraps react-select. The div sits in the DOM and event-flow but generates no box of its own — no layout impact, no width/margin/flex changes. ItsonKeyDownruns on the bubble phase, after react-select's own handler. When react-select consumes Enter for option selection it already callspreventDefault()internally; our handler is redundant in that case (harmless). When react-select does not consume Enter (menu closed, or search field with no highlighted option), our handler callspreventDefault()and the browser skips implicit submission.Alternatives considered
1. Form-level handler with a text-input whitelist — the first version of this PR. Only blocked Enter on readonly inputs, so a searchable Select (>8 options) still submitted the form. It also did nothing when the
Selectwas used inside a consumer's own<form>. Rejected after review feedback from @JBWatenbergScality flagged the searchable-Select regression.2.
preventDefault()inside react-select'sonKeyDownprop — not viable. React-select checksevent.defaultPreventedand skips all keyboard handling if set, breaking menu opening and option selection.3.
stopPropagation()inside react-select'sonKeyDownprop — not viable. Implicit form submission is a browser default action, not triggered by event bubbling.stopPropagation()cannot prevent default actions.