From 1df1aee0104ece342c4665a932a5e8c8b8c54336 Mon Sep 17 00:00:00 2001
From: Gao Sun <gao@silverhand.io>
Date: Wed, 9 Mar 2022 12:24:33 +0800
Subject: [PATCH] refactor(console): improve create app form ux (#340)

* refactor(console): improve create app form ux

* refactor(console): update i18n
---
 .../src/components/RadioGroup/Radio.tsx       | 24 +++++++++--
 .../components/RadioGroup/index.module.scss   |  7 +++
 .../src/components/RadioGroup/index.tsx       | 43 +++++++++++--------
 .../components/CreateForm/index.module.scss   |  6 +++
 .../components/CreateForm/index.tsx           | 14 ++++--
 packages/phrases/src/locales/en.ts            |  1 +
 packages/phrases/src/locales/zh-cn.ts         |  1 +
 7 files changed, 70 insertions(+), 26 deletions(-)

diff --git a/packages/console/src/components/RadioGroup/Radio.tsx b/packages/console/src/components/RadioGroup/Radio.tsx
index 56aade654..752f4120f 100644
--- a/packages/console/src/components/RadioGroup/Radio.tsx
+++ b/packages/console/src/components/RadioGroup/Radio.tsx
@@ -1,5 +1,5 @@
 import classNames from 'classnames';
-import React, { ReactNode } from 'react';
+import React, { KeyboardEventHandler, ReactNode, useCallback } from 'react';
 
 import * as styles from './index.module.scss';
 
@@ -22,12 +22,28 @@ export type Props = {
   children?: ReactNode;
   isChecked?: boolean;
   onClick?: () => void;
+  tabIndex?: number;
 };
 
-const Radio = ({ value, title, name, children, isChecked, onClick }: Props) => {
+const Radio = ({ value, title, name, children, isChecked, onClick, tabIndex }: Props) => {
+  const handleKeyPress: KeyboardEventHandler<HTMLDivElement> = useCallback(
+    (event) => {
+      if ([' ', 'Enter'].includes(event.key)) {
+        onClick?.();
+        event.preventDefault();
+      }
+    },
+    [onClick]
+  );
+
   return (
-    <div className={classNames(styles.radio, isChecked && styles.checked)} onClick={onClick}>
-      <input readOnly type="radio" name={name} value={value} checked={isChecked} />
+    <div
+      className={classNames(styles.radio, isChecked && styles.checked)}
+      tabIndex={tabIndex}
+      onClick={onClick}
+      onKeyPress={handleKeyPress}
+    >
+      <input readOnly disabled type="radio" name={name} value={value} checked={isChecked} />
       <div className={classNames(styles.headline, !children && styles.center)}>
         <div className={styles.title}>{title}</div>
         <Check />
diff --git a/packages/console/src/components/RadioGroup/index.module.scss b/packages/console/src/components/RadioGroup/index.module.scss
index 53d6189d0..9ed27919e 100644
--- a/packages/console/src/components/RadioGroup/index.module.scss
+++ b/packages/console/src/components/RadioGroup/index.module.scss
@@ -15,6 +15,7 @@
   > .radio {
     position: relative;
     flex: 1;
+    min-width: 180px;
     max-width: 220px;
     padding: _.unit(5);
     display: flex;
@@ -24,6 +25,12 @@
     outline: none;
     user-select: none;
     cursor: pointer;
+    transition: border 0.2s ease-in-out;
+
+    &:focus,
+    &:hover {
+      border-color: var(--color-primary);
+    }
 
     &:not(:first-child) {
       margin-left: _.unit(8);
diff --git a/packages/console/src/components/RadioGroup/index.tsx b/packages/console/src/components/RadioGroup/index.tsx
index 9a88becfe..ec9673df3 100644
--- a/packages/console/src/components/RadioGroup/index.tsx
+++ b/packages/console/src/components/RadioGroup/index.tsx
@@ -1,4 +1,4 @@
-import React, { Children, cloneElement, isValidElement, ReactNode } from 'react';
+import React, { Children, cloneElement, forwardRef, isValidElement, ReactNode } from 'react';
 
 import Radio, { Props as RadioProps } from './Radio';
 import * as styles from './index.module.scss';
@@ -7,28 +7,33 @@ type Props = {
   name: string;
   children: ReactNode;
   value: string;
+  // https://github.com/yannickcr/eslint-plugin-react/issues/2856
+  // eslint-disable-next-line react/require-default-props
   onChange?: (value: string) => void;
 };
 
-const RadioGroup = ({ name, children, value, onChange }: Props) => {
-  return (
-    <div className={styles.radioGroup} tabIndex={0}>
-      {Children.map(children, (child) => {
-        if (!isValidElement(child) || child.type !== Radio) {
-          return child;
-        }
+const RadioGroup = forwardRef<HTMLDivElement, Props>(
+  ({ name, children, value, onChange }: Props, reference) => {
+    return (
+      <div ref={reference} className={styles.radioGroup}>
+        {Children.map(children, (child) => {
+          if (!isValidElement(child) || child.type !== Radio) {
+            return child;
+          }
 
-        return cloneElement<RadioProps>(child, {
-          name,
-          isChecked: value === child.props.value,
-          onClick: () => {
-            onChange?.(child.props.value);
-          },
-        });
-      })}
-    </div>
-  );
-};
+          return cloneElement<RadioProps>(child, {
+            name,
+            isChecked: value === child.props.value,
+            onClick: () => {
+              onChange?.(child.props.value);
+            },
+            tabIndex: 0,
+          });
+        })}
+      </div>
+    );
+  }
+);
 
 export default RadioGroup;
 export { default as Radio } from './Radio';
diff --git a/packages/console/src/pages/Applications/components/CreateForm/index.module.scss b/packages/console/src/pages/Applications/components/CreateForm/index.module.scss
index ff0dae4a6..85818660e 100644
--- a/packages/console/src/pages/Applications/components/CreateForm/index.module.scss
+++ b/packages/console/src/pages/Applications/components/CreateForm/index.module.scss
@@ -29,3 +29,9 @@
   margin-top: _.unit(8);
   text-align: right;
 }
+
+.error {
+  font: var(--font-body-2);
+  color: var(--color-error);
+  margin-top: _.unit(2);
+}
diff --git a/packages/console/src/pages/Applications/components/CreateForm/index.tsx b/packages/console/src/pages/Applications/components/CreateForm/index.tsx
index 95a6bcbe3..978d92039 100644
--- a/packages/console/src/pages/Applications/components/CreateForm/index.tsx
+++ b/packages/console/src/pages/Applications/components/CreateForm/index.tsx
@@ -27,9 +27,14 @@ type Props = {
 };
 
 const CreateForm = ({ onClose }: Props) => {
-  const { handleSubmit, control, register } = useForm<FormData>();
   const {
-    field: { onChange, value, name },
+    handleSubmit,
+    control,
+    register,
+    formState: { errors },
+  } = useForm<FormData>();
+  const {
+    field: { onChange, value, name, ref },
   } = useController({ name: 'type', control, rules: { required: true } });
   const { t } = useTranslation(undefined, { keyPrefix: 'admin_console' });
 
@@ -51,7 +56,7 @@ const CreateForm = ({ onClose }: Props) => {
       </div>
       <form className={styles.form} onSubmit={onSubmit}>
         <FormField title="admin_console.applications.select_application_type">
-          <RadioGroup name={name} value={value} onChange={onChange}>
+          <RadioGroup ref={ref} name={name} value={value} onChange={onChange}>
             {Object.values(ApplicationType).map((value) => (
               <Radio key={value} title={t(`${applicationTypeI18nKey[value]}.title`)} value={value}>
                 <TypeDescription
@@ -61,6 +66,9 @@ const CreateForm = ({ onClose }: Props) => {
               </Radio>
             ))}
           </RadioGroup>
+          {errors.type?.type === 'required' && (
+            <div className={styles.error}>{t('applications.no_application_type_selected')}</div>
+          )}
         </FormField>
         <FormField
           isRequired
diff --git a/packages/phrases/src/locales/en.ts b/packages/phrases/src/locales/en.ts
index 5b2f05d53..d29972b28 100644
--- a/packages/phrases/src/locales/en.ts
+++ b/packages/phrases/src/locales/en.ts
@@ -52,6 +52,7 @@ const translation = {
       application_name: 'Application Name',
       application_description: 'Application Description',
       select_application_type: 'Select an Application Type',
+      no_application_type_selected: 'You have to select an application type to proceed.',
       client_id: 'Client ID',
       type: {
         native: {
diff --git a/packages/phrases/src/locales/zh-cn.ts b/packages/phrases/src/locales/zh-cn.ts
index d8b52b760..8652b54f2 100644
--- a/packages/phrases/src/locales/zh-cn.ts
+++ b/packages/phrases/src/locales/zh-cn.ts
@@ -54,6 +54,7 @@ const translation = {
       application_name: 'Application Name',
       application_description: 'Application Description',
       select_application_type: 'Select an application type',
+      no_application_type_selected: 'You have to select an application type to proceed.',
       client_id: 'Client ID',
       type: {
         native: {