Browse Source

fix(project-create): Improve project creation UX (#54436)

Add default value to the alert threshold field.
Add descriptive tooltip to disabled submit button.

Fixes #53791
ArthurKnaus 1 year ago
parent
commit
eafe720633

+ 13 - 12
static/app/views/projectInstall/createProject.spec.tsx

@@ -490,29 +490,30 @@ describe('CreateProject', function () {
     it('should enabled the submit button if and only if all the required information has been filled', async function () {
       render(<CreateProject />);
 
-      const createProjectButton = screen.getByRole('button', {name: 'Create Project'});
+      // We need to query for the submit button every time we want to access it
+      // as re-renders can create new DOM nodes
+      const getSubmitButton = () => screen.getByRole('button', {name: 'Create Project'});
 
-      await userEvent.click(screen.getByText(/When there are more than/));
-      expect(createProjectButton).toBeDisabled();
-
-      await userEvent.type(screen.getByTestId('range-input'), '2');
-      expect(screen.getByTestId('range-input')).toHaveValue(2);
-      expect(createProjectButton).toBeDisabled();
+      expect(getSubmitButton()).toBeDisabled();
 
+      // Selecting the platform pre-fills the project name
       await userEvent.click(screen.getByTestId('platform-apple-ios'));
-      expect(createProjectButton).toBeEnabled();
+      expect(getSubmitButton()).toBeEnabled();
+
+      await userEvent.click(screen.getByText(/When there are more than/));
+      expect(getSubmitButton()).toBeEnabled();
 
       await userEvent.clear(screen.getByTestId('range-input'));
-      expect(createProjectButton).toBeDisabled();
+      expect(getSubmitButton()).toBeDisabled();
 
       await userEvent.type(screen.getByTestId('range-input'), '2712');
-      expect(createProjectButton).toBeEnabled();
+      expect(getSubmitButton()).toBeEnabled();
 
       await userEvent.clear(screen.getByTestId('range-input'));
-      expect(createProjectButton).toBeDisabled();
+      expect(getSubmitButton()).toBeDisabled();
 
       await userEvent.click(screen.getByText("I'll create my own alerts later"));
-      expect(createProjectButton).toBeEnabled();
+      expect(getSubmitButton()).toBeEnabled();
     });
   });
 });

+ 32 - 14
static/app/views/projectInstall/createProject.tsx

@@ -18,6 +18,7 @@ import {SupportedLanguages} from 'sentry/components/onboarding/frameworkSuggesti
 import PlatformPicker, {Platform} from 'sentry/components/platformPicker';
 import {useProjectCreationAccess} from 'sentry/components/projects/useProjectCreationAccess';
 import TeamSelector from 'sentry/components/teamSelector';
+import {Tooltip} from 'sentry/components/tooltip';
 import {IconAdd} from 'sentry/icons';
 import {t, tct} from 'sentry/locale';
 import ProjectsStore from 'sentry/stores/projectsStore';
@@ -260,12 +261,27 @@ function CreateProject() {
   const canCreateTeam = organization.access.includes('project:admin');
   const isOrgMemberWithNoAccess = accessTeams.length === 0 && !canCreateTeam;
 
-  const canSubmitForm =
-    !inFlight &&
-    (team || isOrgMemberWithNoAccess) &&
-    canCreateProject &&
-    projectName !== '' &&
-    (!shouldCreateCustomRule || conditions?.every?.(condition => condition.value));
+  const isMissingTeam = !isOrgMemberWithNoAccess && !team;
+  const isMissingProjectName = projectName === '';
+  const isMissingAlertThreshold =
+    shouldCreateCustomRule && !conditions?.every?.(condition => condition.value);
+
+  const formErrorCount = [
+    isMissingTeam,
+    isMissingProjectName,
+    isMissingAlertThreshold,
+  ].filter(value => value).length;
+
+  const canSubmitForm = !inFlight && canCreateProject && formErrorCount === 0;
+
+  let submitTooltipText: string = t('Please select a team');
+  if (formErrorCount > 1) {
+    submitTooltipText = t('Please fill out all the required fields');
+  } else if (isMissingProjectName) {
+    submitTooltipText = t('Please provide a project name');
+  } else if (isMissingAlertThreshold) {
+    submitTooltipText = t('Please provide an alert threshold');
+  }
 
   const alertFrequencyDefaultValues = useMemo(() => {
     if (!autoFill) {
@@ -358,14 +374,16 @@ function CreateProject() {
           </div>
         )}
         <div>
-          <Button
-            type="submit"
-            data-test-id="create-project"
-            priority="primary"
-            disabled={!canSubmitForm}
-          >
-            {t('Create Project')}
-          </Button>
+          <Tooltip title={submitTooltipText} disabled={formErrorCount === 0}>
+            <Button
+              type="submit"
+              data-test-id="create-project"
+              priority="primary"
+              disabled={!canSubmitForm}
+            >
+              {t('Create Project')}
+            </Button>
+          </Tooltip>
         </div>
       </CreateProjectForm>
     </Fragment>

+ 2 - 2
static/app/views/projectInstall/issueAlertOptions.spec.tsx

@@ -105,14 +105,14 @@ describe('IssueAlertOptions', function () {
     );
   });
 
-  it('should not pre-fill threshold value after a valid server response', () => {
+  it('should pre-fill threshold value after a valid server response', () => {
     MockApiClient.addMockResponse({
       url: URL,
       body: TestStubs.MOCK_RESP_VERBOSE,
     });
 
     render(<IssueAlertOptions {...props} />);
-    expect(screen.getByTestId('range-input')).toHaveValue(null);
+    expect(screen.getByTestId('range-input')).toHaveValue(10);
   });
 
   it('should provide fallthroughType with issue action for issue-alert-fallback-targeting', async () => {

+ 2 - 4
static/app/views/projectInstall/issueAlertOptions.tsx

@@ -46,8 +46,6 @@ const METRIC_CONDITION_MAP = {
   [MetricValues.USERS]: UNIQUE_USER_FREQUENCY_CONDITION,
 } as const;
 
-const DEFAULT_PLACEHOLDER_VALUE = '10';
-
 type StateUpdater = (updatedData: RequestDataFragment) => void;
 type Props = DeprecatedAsyncComponent['props'] & {
   onChange: StateUpdater;
@@ -125,7 +123,7 @@ class IssueAlertOptions extends DeprecatedAsyncComponent<Props, State> {
       alertSetting: this.props.alertSetting ?? RuleAction.ALERT_ON_EVERY_ISSUE.toString(),
       metric: this.props.metric ?? MetricValues.ERRORS,
       interval: this.props.interval ?? '',
-      threshold: this.props.threshold ?? '',
+      threshold: this.props.threshold ?? '10',
     };
   }
 
@@ -161,7 +159,7 @@ class IssueAlertOptions extends DeprecatedAsyncComponent<Props, State> {
           type="number"
           min="0"
           name=""
-          placeholder={DEFAULT_PLACEHOLDER_VALUE}
+          placeholder="10"
           value={this.state.threshold}
           onChange={threshold =>
             this.setStateAndUpdateParents({threshold: threshold.target.value})