From dd1ab1ca72a308107801e93afc1f3a590ca058d8 Mon Sep 17 00:00:00 2001 From: Thomas Heartman Date: Mon, 29 Nov 2021 15:18:12 +0100 Subject: [PATCH] Autofocus dialog form fields, allow form submissions via pressing enter inside the form (#524) * chore: add prettier as a dev dependency The project has a .prettierrc, so seems to depend on that for its formatting, but there was no prettier installed with the node modules. * chore: add autofocus to all clearly defined first inputs on dialogs. * fix: wrap the disable env input in a form and give it autofocus. * fix: submit form when pressing enter * fix: only autofocus the submit button if there is no other content. When multiple (enabled) elements have the autofocus attribute, the browser picks the last element in the tree. This means that if there is a form with a text input with autofocus and a submit button with autofocus, the button will win, causing the user to have to tab back up. Only doing this if there are no children will cause some changes, however: Dialogs with textual children will no longer focus the accept-button when appearing. However, dialogs such as the create new api token dialog will give the focus to the first input field instead of to the create button. * fix: add formId prop to dialog element; adapt behavior If the component receives a form id, it will treat the primary button as the submit button for that form. To stop a full page reload, we call the `preventDefault` on the submit event before calling the handler. * chore: remove redundant spacing in component. * fix: hook environment disable form up with the new form id system. * chore: Update existing modal forms to pass in formId * fix: Type the dialog event wrapper * fix: change 'allows' => 'allow' because the noun is pluralized. * fix: add autofocus to js add-tag-dialog-component. I've got a feeling this component isn't in use anymore, though, as the exact same text appears in a TS-version of this component. * fix: add autofocus to add user form. This seems to only be used as the main piece of a modal, so adding autofocus seems pretty safe here, but I could be wrong. * fix: Update snapshot test after changing wording. * fix: add autofocus to update user form * fix: add autofocus to the create toggle form. This is a little besides the task's actual point. However! This form is only ever used on the page where it's the only bit of content. I'd argue that when the user navigates to this form, it's because they want to create a feature. Thus, adding autofocus to the first field makes a lot of sense to me. * refactor: set button type to 'undefined' when it isn't 'submit' This allows Material to use their default type based on whatever heuristics they use. It's most likely going to be 'button' for the foreseeable future, but in the event that they change it, passing undefined instead should future-proof this a bit. * fix: set type to button when formId is not present Co-authored-by: Fredrik Strand Oseberg --- frontend/package.json | 1 + .../ApiTokenCreate/ApiTokenCreate.tsx | 5 +++++ .../component/common/Dialogue/Dialogue.tsx | 16 +++++++++++--- .../EditEnvironment/EditEnvironment.tsx | 5 ++++- .../EnvironmentDeleteConfirm.tsx | 18 ++++++++++----- .../feature/FeatureCreate/FeatureCreate.tsx | 1 + .../AddTagDialog/AddTagDialog.tsx | 8 +++++-- .../AddFeatureVariant/AddFeatureVariant.tsx | 10 ++++++++- .../feature/add-tag-dialog-component.jsx | 7 ++++-- .../feature/variant/AddVariant/AddVariant.jsx | 10 ++++++++- .../EnvironmentDisableConfirm.tsx | 22 ++++++++++++------- .../tag-type-create-component-test.js.snap | 6 ++--- .../tag-types/form-tag-type-component.js | 2 +- .../src/page/admin/users/AddUser/AddUser.tsx | 4 ++++ .../users/AddUser/AddUserForm/AddUserForm.jsx | 4 +++- .../admin/users/update-user-component.jsx | 4 ++++ frontend/yarn.lock | 5 +++++ 17 files changed, 99 insertions(+), 29 deletions(-) diff --git a/frontend/package.json b/frontend/package.json index 1e32072215..83faec94bf 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -73,6 +73,7 @@ "lodash.clonedeep": "4.5.0", "lodash.flow": "3.5.0", "node-fetch": "2.6.6", + "prettier": "^2.4.1", "react": "17.0.2", "react-dnd": "14.0.4", "react-dnd-html5-backend": "14.0.2", diff --git a/frontend/src/component/api-token/ApiTokenCreate/ApiTokenCreate.tsx b/frontend/src/component/api-token/ApiTokenCreate/ApiTokenCreate.tsx index 3ae313f3e3..0624936db1 100644 --- a/frontend/src/component/api-token/ApiTokenCreate/ApiTokenCreate.tsx +++ b/frontend/src/component/api-token/ApiTokenCreate/ApiTokenCreate.tsx @@ -144,6 +144,8 @@ const ApiTokenCreate = ({ { key: 'ADMIN', label: 'Admin', title: 'Admin API token' }, ]; + const formId = 'create-api-token-form'; + return ( submit()} @@ -152,8 +154,10 @@ const ApiTokenCreate = ({ primaryButtonText="Create" secondaryButtonText="Cancel" title="New API token" + formId={formId} >
= ({ @@ -35,8 +36,15 @@ const Dialogue: React.FC = ({ secondaryButtonText, maxWidth = 'sm', fullWidth = false, + formId, }) => { const styles = useStyles(); + const handleClick = formId + ? (e: React.SyntheticEvent) => { + e.preventDefault(); + onClick(e); + } + : onClick; return ( = ({ condition={Boolean(onClick)} show={ @@ -77,7 +87,7 @@ const Dialogue: React.FC = ({ condition={Boolean(onClose)} show={ } /> diff --git a/frontend/src/component/environments/EditEnvironment/EditEnvironment.tsx b/frontend/src/component/environments/EditEnvironment/EditEnvironment.tsx index 90c24a872c..4940910c1f 100644 --- a/frontend/src/component/environments/EditEnvironment/EditEnvironment.tsx +++ b/frontend/src/component/environments/EditEnvironment/EditEnvironment.tsx @@ -75,6 +75,8 @@ const EditEnvironment = ({ setType(env.type); }; + const formId = 'edit-environment-form'; + return (

@@ -92,7 +95,7 @@ const EditEnvironment = ({

{env.name}

- + Danger. Deleting this environment will result in removing all @@ -59,12 +62,15 @@ const EnvironmentDeleteConfirm = ({ environment in the textfield below: {env?.name}

- + + + ); }; diff --git a/frontend/src/component/feature/FeatureCreate/FeatureCreate.tsx b/frontend/src/component/feature/FeatureCreate/FeatureCreate.tsx index 127be7e938..a482e37bd8 100644 --- a/frontend/src/component/feature/FeatureCreate/FeatureCreate.tsx +++ b/frontend/src/component/feature/FeatureCreate/FeatureCreate.tsx @@ -110,6 +110,7 @@ const FeatureCreate = () => {
{ } }; + const formId = 'add-tag-form'; + return ( <> { onClick={onSubmit} disabledPrimaryButton={loading} onClose={onCancel} + formId={formId} > <> - Tags allows you to group features together + Tags allow you to group features together -
+
setValue('type', e.target.value)} diff --git a/frontend/src/component/feature/FeatureView2/FeatureVariants/FeatureVariantsList/AddFeatureVariant/AddFeatureVariant.tsx b/frontend/src/component/feature/FeatureView2/FeatureVariants/FeatureVariantsList/AddFeatureVariant/AddFeatureVariant.tsx index 43cd726101..d1c854cf91 100644 --- a/frontend/src/component/feature/FeatureView2/FeatureVariants/FeatureVariantsList/AddFeatureVariant/AddFeatureVariant.tsx +++ b/frontend/src/component/feature/FeatureView2/FeatureVariants/FeatureVariantsList/AddFeatureVariant/AddFeatureVariant.tsx @@ -212,6 +212,8 @@ const AddVariant = ({ const isFixWeight = data.weightType === weightTypes.FIX; + const formId = 'add-feature-variant-form'; + return ( - +

{error.general}