CUI-5: Prevent implicit form submission on Enter for non-text elements#1014
Draft
JeanMarcMilletScality wants to merge 1 commit intodevelopment/1.0from
Draft
CUI-5: Prevent implicit form submission on Enter for non-text elements#1014JeanMarcMilletScality wants to merge 1 commit intodevelopment/1.0from
JeanMarcMilletScality wants to merge 1 commit intodevelopment/1.0from
Conversation
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.
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.
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 aFormtriggers form submission instead of interacting with the widget. Fixed by adding apreventImplicitSubmissionhandler on the Form's<form>element that only allows Enter to submit when the focused element is a writable text input or a button.Ticket: https://scality.atlassian.net/browse/CUI-5
Changes
preventImplicitSubmissioninForm.component.tsx(bothPageFormandTabForm)Form.test.tsxverifying Enter on a Select does not submit, and Enter on a text input still submitsDetails: why this solution, alternatives considered, references
Root cause
The browser's implicit submission mechanism submits a
<form>when Enter is pressed on any input element. React-select uses an internalreadonlytext input, which the browser treats as a regular text input. React-select does not callpreventDefault()when the menu is closed, and itsonKeyDownprop cannot be used withpreventDefault()because react-select checksevent.defaultPreventedand skips all keyboard handling if set.Why fix this in the Form component
Per the WHATWG HTML spec, implicit submission is designed for text fields. The WAI-ARIA Combobox Pattern defines Enter on a combobox as accepting an autocomplete suggestion — it does not define Enter as triggering form submission. Since the combobox pattern does not delegate Enter to the form, implicit submission from a combobox is unexpected behavior. Keyboard operability is also an accessibility requirement (WCAG 2.1 SC 2.1.1).
Since the Form component owns the
<form>element, it is the right place to control submission behavior. Individual widgets should not need to know they are inside a form, and this fix applies to all non-text elements uniformly.Alternatives considered
1.
preventDefault()in the Select'sonKeyDownprop — Not viable. React-select checksevent.defaultPreventedand skips all keyboard handling if set, breaking menu opening and option selection.2.
stopPropagation()in the Select'sonKeyDownprop — Not viable. Implicit form submission is a browser default action, not triggered by event bubbling.stopPropagation()cannot prevent default actions.3. Wrapper
<div>withonKeyDown+preventDefault()around the Select — Works, but adds an unnecessary DOM node and places form submission logic inside the Select, which is the wrong abstraction.