change: [M3-7813] - Allow the disabling of the TypeToConfirm input #10251

abailly-akamai · 2024-03-04T15:20:38Z

Description 📝

We currently don't have the possibility of disabling the TypeToConfirm input (only its submit button).

In the case of a TypeToConfirm Modal where an action needs to be taken before being abled to delete the entity, we want to be able to disable the input in order for the user to not fill and then to realize they still can't delete it.

This is the case for the PlacementGroupDeleteModal for which we need to delete all attached Linodes in order to delete the PlacementGroup

Changes 🔄

Add new disableTypeToConfirmInput prop to allow disabling the input
Improve naming conventions
Improve test coverage

Preview 📷

Include a screenshot or screen recording of the change

How to test 🧪

Prerequisites

Have both the "Placement Group" feature flag and MSW turned on

Verification steps

Navigate to http://localhost:3000/placement-groups/delete/1
Confirm TypeToConfirm input is disabled
Modify PG factory to have an empty linodes array
Confirm TypeToConfirm input is enabled

As an Author I have considered 🤔

Check all that apply

Commit message and pull request title format standards

Note: Remove this section before opening the pull request
Make sure your PR title and commit message on squash and merge are as shown below

<commit type>: [JIRA-ticket-number] - <description>

Commit Types:

feat: New feature for the user (not a part of the code, or ci, ...).
fix: Bugfix for the user (not a fix to build something, ...).
change: Modifying an existing visual UI instance. Such as a component or a feature.
refactor: Restructuring existing code without changing its external behavior or visual UI. Typically to improve readability, maintainability, and performance.
test: New tests or changes to existing tests. Does not change the production code.
upcoming: A new feature that is in progress, not visible to users yet, and usually behind a feature flag.

Example: feat: [M3-1234] - Allow user to view their login history

abailly-akamai · 2024-03-04T15:29:58Z

packages/manager/src/components/TypeToConfirmDialog/TypeToConfirmDialog.tsx

@@ -64,8 +66,9 @@ export const TypeToConfirmDialog = (props: CombinedProps) => {
  const [confirmText, setConfirmText] = React.useState('');

  const { data: preferences } = usePreferences();
-  const disabled =
+  const isPrimaryButtonDisabled =


Just changing the const name to make things clearer what element is meant to be disabled by those conditions

Is there ever a case where we would want isPrimaryButtonDisabled to be false when the disableTypeToConfirmInput prop is true?

I’m imagining a scenario where a user who has disabled type-to-confirm in their preferences is able to proceed with some action while a user who has type-to-confirm enabled gets blocked by the disabled input field, but I might not be understanding the logic fully

hmmm - trying to wrap my head around this - I don't think so? It feels to me like if the user has type-to-confirm disabled, we'd just need to rely on API errors as it is currently? Hope that answers this concern - what's implemented here really merely a visual helper so I am trying to think of it in the simplest way possible. Do you have any other suggestion as to how handling it?

You're changes look good.

On a side note (for another ticket), I find it odd that I can't manually disable the "Delete" button in the dialog. I would imagine that if there's still Linodes assigned to the PG and type-to-confirm is disabled, the Delete button should be disabled and we shouldn't rely on the API to deny the user.

Yup, i hear that. This component was breaking a few things so I fixed things in 53ffd9f

Mostly, the issue is that the disabled prop was broken. Please see my comments on this commit

abailly-akamai · 2024-03-04T15:32:38Z

packages/manager/src/features/PlacementGroups/PlacementGroupsDeleteModal.tsx

-      inputProps={{
-        disabled: isDisabled,
-      }}
+      disableTypeToConfirmInput={isDisabled}


Making this update for the Placement Group delete modal since we want to prevent the use trying to type to confirm considering it will not enable the submit button

jdamore-linode · 2024-03-04T17:01:06Z

packages/manager/src/components/TypeToConfirmDialog/TypeToConfirmDialog.tsx

+  /**
+   * Chidlren are rendered above the TypeToConfirm input
+   */
  children?: React.ReactNode;
+  /**
+   * Props to be allow disabling the input
+   */
+  disableTypeToConfirmInput?: boolean;
+  /**
+   * The entity being confirmed
+   */
  entity: EntityInfo;


++ for all these awesome doc comments!

jaalah-akamai · 2024-03-05T14:27:04Z

packages/manager/src/components/TypeToConfirmDialog/TypeToConfirmDialog.tsx

@@ -64,8 +66,9 @@ export const TypeToConfirmDialog = (props: CombinedProps) => {
  const [confirmText, setConfirmText] = React.useState('');

  const { data: preferences } = usePreferences();
-  const disabled =
+  const isPrimaryButtonDisabled =


You're changes look good.

On a side note (for another ticket), I find it odd that I can't manually disable the "Delete" button in the dialog. I would imagine that if there's still Linodes assigned to the PG and type-to-confirm is disabled, the Delete button should be disabled and we shouldn't rely on the API to deny the user.

github-actions · 2024-03-05T18:50:19Z

Coverage Report: ✅
Base Coverage: 81.29%
Current Coverage: 81.31%

abailly-akamai · 2024-03-05T19:01:27Z

packages/manager/src/components/TypeToConfirmDialog/TypeToConfirmDialog.tsx

+  /**
+   * Props to be allow disabling the submit button
+   */
+  disableTypeToConfirmSubmit?: boolean;


this is to replace the disabled prop (which I omitted from the types) so the nomenclature is very clear as to what element we're looking to disabled

abailly-akamai · 2024-03-05T19:02:07Z

packages/manager/src/components/TypeToConfirmDialog/TypeToConfirmDialog.tsx

@@ -68,12 +72,13 @@ interface TypeToConfirmDialogProps {

 type CombinedProps = TypeToConfirmDialogProps &
  ConfirmationDialogProps &
-  Partial<TypeToConfirmProps>;
+  Partial<Omit<TypeToConfirmProps, 'disabled'>>;


This leads to confusion since we may want both the input & and the button to be disabled for different reasons

abailly-akamai · 2024-03-05T19:02:34Z

packages/manager/src/features/Account/CloseAccountDialog.tsx

@@ -103,7 +87,6 @@ const CloseAccountDialog = ({ closeDialog, open }: Props) => {
        subType: 'CloseAccount',
        type: 'AccountSetting',
      }}
-      disabled={!canSubmit}


this has 0 effect

abailly-akamai · 2024-03-05T19:03:36Z

packages/manager/src/features/Account/CloseAccountDialog.tsx

-      setCanSubmit(false);
-    }
-  }, [inputtedUsername, profile]);
-


I dunno why this code was here, but it was trying to do the same thing the TypeToConfirmDialog does by default (I fixed it cause the e2e was failing)

abailly-akamai · 2024-03-05T19:04:08Z

...nager/src/features/Kubernetes/KubernetesClusterDetail/DeleteKubernetesClusterDialog.test.tsx


    await findByTestId('textfield-input');

    const input = getByTestId('textfield-input');
    fireEvent.change(input, { target: { value: 'this-cluster' } });

-    expect(button).toHaveAttribute('aria-disabled', 'false');


this was a false positive!

jaalah-akamai · 2024-03-05T19:19:33Z

This is much better - 🚢

abailly-akamai · 2024-03-05T21:03:32Z

It appears the e2e failures (volume) are unrelated

…eature/namespace-create * 'develop' of https://github.com/vrajesh73/manager: (89 commits) fix: [M3-7269] - Display parent email in user menu when no company name is available for restricted parent user (linode#10248) fix: [M3-7817] - Show correct status of Child Account Enabled column for parent users (linode#10233) upcoming: [M3-7616] - Add Placement Groups Events and Notifications (linode#10221) upcoming: [M3-7816-v2] - Adjust logic for when to show Switch Account button (linode#10266) fix: [M3-7831] - Persisting error messages in ACLB delete dialogs (linode#10254) upcoming: [M3-7842] - Update Assign Linode Drawer and improve query skipping (linode#10263) upcoming: [M3-7704] - Disable Cloning, Private IP, Backups for edge regions (linode#10222) test: Fix test flake for Images landing page test (linode#10267) fix: [M3-7824] - ACLB TCP Rule Creation and other fixes (linode#10264) refactor: [M3-7687] - Linodes Restricted User Experience 2/2 (linode#10227) test: Resolve OBJ create and delete E2E test flake (linode#10245) upcoming: [M3-7723] - Placement Group feature flag as object (linode#10256) chore(deps): Bump sanitize-html from 2.11.0 to 2.12.1 (linode#10247) change: [M3-7813] - Allow the disabling of the TypeToConfirm input (linode#10251) upcoming: [M3-7839] - Change Business Partner to Parent User (linode#10259) upcoming: [M3-7835] - Adjust user table column count (linode#10252) upcoming: [M3 7738] - Update Placement Group Create & Edit Drawers (linode#10205) refactor: [M3-7437] - Use `@lukemorales/query-key-factory` for Profile Queries (linode#10241) fix: React Query `updateInPaginatedStore` helper function not working as expected (linode#10249) test: [M3-7497] - Add tests for child user verification banner (linode#10204) ... # Conflicts: # packages/manager/src/MainContent.tsx # packages/manager/src/dev-tools/FeatureFlagTool.tsx

Allow disabling type to confirm input

c8a7399

abailly-akamai self-assigned this Mar 4, 2024

abailly-akamai added 2 commits March 4, 2024 10:27

PlacementGroupDelete update

492fab4

Improve coverage

9de33b8

abailly-akamai commented Mar 4, 2024

View reviewed changes

abailly-akamai marked this pull request as ready for review March 4, 2024 16:38

abailly-akamai requested a review from a team as a code owner March 4, 2024 16:38

abailly-akamai requested review from bnussman-akamai and jaalah-akamai and removed request for a team March 4, 2024 16:38

jdamore-linode reviewed Mar 4, 2024

View reviewed changes

jaalah-akamai approved these changes Mar 5, 2024

View reviewed changes

feedback and update interface

53ffd9f

abailly-akamai commented Mar 5, 2024

View reviewed changes

Improve coverage

d02007f

bnussman-akamai approved these changes Mar 6, 2024

View reviewed changes

abailly-akamai merged commit d4bb18e into linode:develop Mar 6, 2024
17 of 18 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

change: [M3-7813] - Allow the disabling of the TypeToConfirm input #10251

change: [M3-7813] - Allow the disabling of the TypeToConfirm input #10251

abailly-akamai commented Mar 4, 2024 •

edited

Loading

abailly-akamai Mar 4, 2024

jdamore-linode Mar 4, 2024

abailly-akamai Mar 4, 2024

jaalah-akamai Mar 5, 2024

abailly-akamai Mar 5, 2024

abailly-akamai Mar 4, 2024

jdamore-linode Mar 4, 2024

jaalah-akamai Mar 5, 2024

github-actions bot commented Mar 5, 2024 •

edited

Loading

abailly-akamai Mar 5, 2024

abailly-akamai Mar 5, 2024

abailly-akamai Mar 5, 2024

abailly-akamai Mar 5, 2024

abailly-akamai Mar 5, 2024

jaalah-akamai commented Mar 5, 2024

abailly-akamai commented Mar 5, 2024

change: [M3-7813] - Allow the disabling of the TypeToConfirm input #10251

change: [M3-7813] - Allow the disabling of the TypeToConfirm input #10251

Conversation

abailly-akamai commented Mar 4, 2024 • edited Loading

Description 📝

Changes 🔄

Preview 📷

How to test 🧪

Prerequisites

Verification steps

As an Author I have considered 🤔

Commit message and pull request title format standards

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Mar 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jaalah-akamai commented Mar 5, 2024

abailly-akamai commented Mar 5, 2024

abailly-akamai commented Mar 4, 2024 •

edited

Loading

github-actions bot commented Mar 5, 2024 •

edited

Loading