upcoming: [M3-8340] - Update Manage Image Regions Drawer based on UX …

…feedback (#10674) * update to use new ux * Added changeset: Update Manage Image Regions Drawer based on UX feedback * fix obj regression * update tests to reflect new UX * add fallback * fix placeholder behvaior --------- Co-authored-by: Banks Nussman <[email protected]>
linode · Jul 15, 2024 · c71cdbc · c71cdbc
1 parent 90b6625
commit c71cdbc
Show file tree

Hide file tree

Showing 10 changed files with 71 additions and 46 deletions.
diff --git a/packages/manager/.changeset/pr-10674-upcoming-features-1720716452894.md b/packages/manager/.changeset/pr-10674-upcoming-features-1720716452894.md
@@ -0,0 +1,5 @@
+---
+"@linode/manager": Upcoming Features
+---
+
+Update Manage Image Regions Drawer based on UX feedback ([#10674](https://github.com/linode/manager/pull/10674))
diff --git a/packages/manager/cypress/e2e/core/images/manage-image-regions.spec.ts b/packages/manager/cypress/e2e/core/images/manage-image-regions.spec.ts
@@ -139,7 +139,7 @@ describe('Manage Image Regions', () => {
     // mock the updated paginated response
     mockGetCustomImages([updatedImage]);
 
-    // Click outside of the Region Multi-Select to commit the selection to the list
+    // Click outside of the Region Multi-Select to close the popover
     ui.drawer
       .findByTitle(`Manage Regions for ${image.label}`)
       .click()
@@ -170,7 +170,7 @@ describe('Manage Image Regions', () => {
         // Verify the image isn't shown in the list after being removed
         cy.findByText(region1.label).should('not.exist');
 
-        // Verify the count is now 2
+        // Verify the count is now 3
         cy.findByText('Image will be available in these regions (3)').should(
           'be.visible'
         );
@@ -181,21 +181,9 @@ describe('Manage Image Regions', () => {
           .should('be.visible')
           .should('be.enabled')
           .click();
-
-        // "Unsaved" regions should transition to "pending replication" because
-        // they are now returned by the API
-        cy.findAllByText('pending replication').should('be.visible');
-
-        // The save button should become disabled now that changes have been saved
-        ui.button.findByTitle('Save').should('be.disabled');
-
-        // The save button should become disabled now that changes have been saved
-        ui.button.findByTitle('Save').should('be.disabled');
-
-        cy.findByLabelText('Close drawer').click();
       });
 
-    ui.toast.assertMessage('Image regions successfully updated.');
+    ui.toast.assertMessage(`${image.label}'s regions successfully updated.`);
 
     cy.findByText(image.label)
       .closest('tr')
@@ -206,8 +194,24 @@ describe('Manage Image Regions', () => {
         // Verify the first region is rendered
         cy.findByText(region2.label + ',').should('be.visible');
 
-        // Verify the regions count is now "+2"
         cy.findByText('+2').should('be.visible').should('be.enabled');
+
+        // Verify the regions count is now "+2" and open the drawer
+        cy.findByText('+2').should('be.visible').should('be.enabled').click();
+      });
+
+    ui.drawer
+      .findByTitle(`Manage Regions for ${image.label}`)
+      .click()
+      .within(() => {
+        // "Unsaved" regions should transition to "pending replication" because
+        // they are now returned by the API
+        cy.findAllByText('pending replication').should('be.visible');
+
+        // The save button should be disabled
+        ui.button.findByTitle('Save').should('be.disabled');
+
+        cy.findByLabelText('Close drawer').click();
       });
   });
 });
diff --git a/packages/manager/src/components/Autocomplete/Autocomplete.tsx b/packages/manager/src/components/Autocomplete/Autocomplete.tsx
@@ -108,7 +108,7 @@ export const Autocomplete = <
           label={label}
           loading={loading}
           noMarginTop={noMarginTop}
-          placeholder={placeholder || 'Select an option'}
+          placeholder={placeholder ?? 'Select an option'}
           required={textFieldProps?.InputProps?.required}
           tooltipText={textFieldProps?.tooltipText}
           {...params}

diff --git a/packages/manager/src/components/RegionSelect/RegionMultiSelect.tsx b/packages/manager/src/components/RegionSelect/RegionMultiSelect.tsx
@@ -1,4 +1,3 @@
-import { Region } from '@linode/api-v4';
 import CloseIcon from '@mui/icons-material/Close';
 import React from 'react';
 
@@ -24,6 +23,7 @@ import type {
   DisableRegionOption,
   RegionMultiSelectProps,
 } from './RegionSelect.types';
+import type { Region } from '@linode/api-v4';
 
 interface LabelComponentProps {
   region: Region;
@@ -67,7 +67,7 @@ export const RegionMultiSelect = React.memo((props: RegionMultiSelectProps) => {
     selectedIds,
     sortRegionOptions,
     width,
-    onClose,
+    ...rest
   } = props;
 
   const {
@@ -115,7 +115,7 @@ export const RegionMultiSelect = React.memo((props: RegionMultiSelectProps) => {
             return getRegionCountryGroup(option);
           }}
           onChange={(_, selectedOptions) =>
-            onChange(selectedOptions.map((region) => region.id))
+            onChange(selectedOptions?.map((region) => region.id) ?? [])
           }
           renderOption={(props, option, { selected }) => {
             if (!option.site_type) {
@@ -155,7 +155,6 @@ export const RegionMultiSelect = React.memo((props: RegionMultiSelectProps) => {
             InputProps: {
               required,
             },
-            placeholder: selectedRegions.length > 0 ? '' : placeholder,
             tooltipText: helperText,
           }}
           autoHighlight
@@ -172,7 +171,7 @@ export const RegionMultiSelect = React.memo((props: RegionMultiSelectProps) => {
           options={regionOptions}
           placeholder={placeholder ?? 'Select Regions'}
           value={selectedRegions}
-          onClose={onClose}
+          {...rest}
         />
       </StyledAutocompleteContainer>
       {selectedRegions.length > 0 && SelectedRegionsList && (

diff --git a/packages/manager/src/components/RegionSelect/RegionSelect.types.ts b/packages/manager/src/components/RegionSelect/RegionSelect.types.ts
@@ -68,7 +68,7 @@ export interface RegionSelectProps<
 
 export interface RegionMultiSelectProps
   extends Omit<
-    EnhancedAutocompleteProps<Region, false>,
+    EnhancedAutocompleteProps<Region, true>,
     'label' | 'onChange' | 'options'
   > {
   SelectedRegionsList?: React.ComponentType<{

diff --git a/packages/manager/src/features/Images/ImagesLanding/ImageRegions/ImageRegionRow.test.tsx b/packages/manager/src/features/Images/ImagesLanding/ImageRegions/ImageRegionRow.test.tsx
@@ -39,4 +39,19 @@ describe('ImageRegionRow', () => {
 
     expect(onRemove).toHaveBeenCalled();
   });
+
+  it('disables the remove button if disableRemoveButton is true', async () => {
+    const { getByLabelText } = renderWithTheme(
+      <ImageRegionRow
+        disableRemoveButton
+        onRemove={vi.fn()}
+        region="us-east"
+        status="creating"
+      />
+    );
+
+    const removeButton = getByLabelText('Remove us-east');
+
+    expect(removeButton).toBeDisabled();
+  });
 });
diff --git a/packages/manager/src/features/Images/ImagesLanding/ImageRegions/ImageRegionRow.tsx b/packages/manager/src/features/Images/ImagesLanding/ImageRegions/ImageRegionRow.tsx
@@ -15,13 +15,14 @@ import type { Status } from 'src/components/StatusIcon/StatusIcon';
 type ExtendedImageRegionStatus = 'unsaved' | ImageRegionStatus;
 
 interface Props {
+  disableRemoveButton?: boolean;
   onRemove: () => void;
   region: string;
   status: ExtendedImageRegionStatus;
 }
 
 export const ImageRegionRow = (props: Props) => {
-  const { onRemove, region, status } = props;
+  const { disableRemoveButton, onRemove, region, status } = props;
 
   const { data: regions } = useRegionsQuery();
 
@@ -40,6 +41,7 @@ export const ImageRegionRow = (props: Props) => {
         />
         <IconButton
           aria-label={`Remove ${region}`}
+          disabled={disableRemoveButton}
           onClick={onRemove}
           sx={{ p: 0.5 }}
         >

diff --git a/...es/manager/src/features/Images/ImagesLanding/ImageRegions/ManageImageRegionsForm.test.tsx b/...es/manager/src/features/Images/ImagesLanding/ImageRegions/ManageImageRegionsForm.test.tsx
@@ -96,9 +96,6 @@ describe('ManageImageRegionsDrawer', () => {
     // Select new region
     await userEvent.click(await findByText('us-west', { exact: false }));
 
-    // Close the Region Multi-Select to that selections are committed to the list
-    await userEvent.type(regionSelect, '{escape}');
-
     expect(getByText('Place, CA')).toBeVisible();
     expect(getByText('unsaved')).toBeVisible();
 

diff --git a/packages/manager/src/features/Images/ImagesLanding/ImageRegions/ManageImageRegionsForm.tsx b/packages/manager/src/features/Images/ImagesLanding/ImageRegions/ManageImageRegionsForm.tsx
@@ -1,7 +1,7 @@
 import { yupResolver } from '@hookform/resolvers/yup';
 import { updateImageRegionsSchema } from '@linode/validation';
 import { useSnackbar } from 'notistack';
-import React, { useState } from 'react';
+import React from 'react';
 import { useForm } from 'react-hook-form';
 
 import { ActionsPanel } from 'src/components/ActionsPanel/ActionsPanel';
@@ -32,8 +32,6 @@ export const ManageImageRegionsForm = (props: Props) => {
   const { data: regions } = useRegionsQuery();
   const { mutateAsync } = useUpdateImageRegionsMutation(image?.id ?? '');
 
-  const [selectedRegions, setSelectedRegions] = useState<string[]>([]);
-
   const {
     formState: { errors, isDirty, isSubmitting },
     handleSubmit,
@@ -50,9 +48,14 @@ export const ManageImageRegionsForm = (props: Props) => {
     try {
       await mutateAsync(data);
 
-      enqueueSnackbar('Image regions successfully updated.', {
-        variant: 'success',
-      });
+      enqueueSnackbar(
+        `${image?.label ?? 'Image'}'s regions successfully updated.`,
+        {
+          variant: 'success',
+        }
+      );
+
+      onClose();
     } catch (errors) {
       for (const error of errors) {
         if (error.field) {
@@ -79,22 +82,19 @@ export const ManageImageRegionsForm = (props: Props) => {
         for details on managing your Linux system's disk space.
       </Typography>
       <RegionMultiSelect
-        onClose={() => {
-          setValue('regions', [...values.regions, ...selectedRegions], {
+        onChange={(regionIds) =>
+          setValue('regions', regionIds, {
             shouldDirty: true,
             shouldValidate: true,
-          });
-          setSelectedRegions([]);
-        }}
-        regions={(regions ?? []).filter(
-          (r) => !values.regions.includes(r.id) && r.site_type === 'core'
-        )}
+          })
+        }
         currentCapability={undefined}
         errorText={errors.regions?.message}
         label="Add Regions"
-        onChange={setSelectedRegions}
-        placeholder="Select Regions"
-        selectedIds={selectedRegions}
+        placeholder="Select regions or type to search"
+        regions={regions?.filter((r) => r.site_type === 'core') ?? []}
+        renderTags={() => null}
+        selectedIds={values.regions}
       />
       <Typography sx={{ mb: 1, mt: 2 }}>
         Image will be available in these regions ({values.regions.length})
@@ -127,6 +127,7 @@ export const ManageImageRegionsForm = (props: Props) => {
                   (regionItem) => regionItem.region === regionId
                 )?.status ?? 'unsaved'
               }
+              disableRemoveButton={values.regions.length <= 1}
               key={regionId}
               region={regionId}
             />

diff --git a/...manager/src/features/ObjectStorage/AccessKeyLanding/AccessKeyRegions/AccessKeyRegions.tsx b/...manager/src/features/ObjectStorage/AccessKeyLanding/AccessKeyRegions/AccessKeyRegions.tsx
@@ -29,13 +29,15 @@ export const AccessKeyRegions = (props: Props) => {
 
   return (
     <RegionMultiSelect
-      onChange={onChange}
+      placeholder={
+        selectedRegion.length > 0 ? '' : 'Select regions or type to search'
+      }
       currentCapability="Object Storage"
       disabled={disabled}
       errorText={errorText}
       isClearable={false}
       label="Regions"
-      placeholder="Select Regions or type to search"
+      onChange={onChange}
       regions={regions ?? []}
       required={required}
       selectedIds={selectedRegion}