refactor: [M3-8720] - One ImageSelect to rule them all (#11058)

* Initial commit - console errors and low hanging fruits * Renaming and moving things * Small clanup * One ImageSelect to rule them all * One ImageSelect to rule them all * fixing e2e tests * save progress * cleanup * cleanup * post rebase fix * fix the console errors with autocomplete * sorting and ordering * Added changeset: Consolidate ImageSelect components * feedback @bnussman-akamai * fix rebase gone wrong * feedback @coliu-akamai
linode · Oct 29, 2024 · 0cbb976 · 0cbb976
1 parent 0818a7e
commit 0cbb976
Show file tree

Hide file tree

Showing 33 changed files with 626 additions and 1,205 deletions.
diff --git a/packages/manager/.changeset/pr-11058-tech-stories-1729621320539.md b/packages/manager/.changeset/pr-11058-tech-stories-1729621320539.md
@@ -0,0 +1,5 @@
+---
+"@linode/manager": Tech Stories
+---
+
+Consolidate ImageSelect components ([#11058](https://github.com/linode/manager/pull/11058))
diff --git a/packages/manager/cypress/e2e/core/linodes/rebuild-linode.spec.ts b/packages/manager/cypress/e2e/core/linodes/rebuild-linode.spec.ts
@@ -131,13 +131,13 @@ describe('rebuild linode', () => {
       openRebuildDialog(linode.label);
       findRebuildDialog(linode.label).within(() => {
         // "From Image" should be selected by default; no need to change the value.
-        ui.select.findByText('From Image').should('be.visible');
-
-        ui.select
-          .findByText('Choose an image')
+        ui.autocomplete
+          .findByLabel('From Image')
           .should('be.visible')
-          .click()
-          .type(`${image}{enter}`);
+          .should('have.value', 'From Image');
+
+        ui.autocomplete.findByLabel('Images').should('be.visible').click();
+        ui.autocompletePopper.findByTitle(image).should('be.visible').click();
 
         // Type to confirm.
         cy.findByLabelText('Linode Label').type(linode.label);
@@ -186,10 +186,9 @@ describe('rebuild linode', () => {
 
       openRebuildDialog(linode.label);
       findRebuildDialog(linode.label).within(() => {
-        ui.select.findByText('From Image').click();
-
-        ui.select
-          .findItemByText('From Community StackScript')
+        ui.autocomplete.findByLabel('From Image').should('be.visible').click();
+        ui.autocompletePopper
+          .findByTitle('From Community StackScript')
           .should('be.visible')
           .click();
 
@@ -203,13 +202,8 @@ describe('rebuild linode', () => {
           cy.get(`[id="${stackScriptId}"][type="radio"]`).click();
         });
 
-        ui.select
-          .findByText('Choose an image')
-          .scrollIntoView()
-          .should('be.visible')
-          .click();
-
-        ui.select.findItemByText(image).should('be.visible').click();
+        ui.autocomplete.findByLabel('Images').should('be.visible').click();
+        ui.autocompletePopper.findByTitle(image).should('be.visible').click();
 
         cy.findByLabelText('Linode Label')
           .should('be.visible')
@@ -229,7 +223,7 @@ describe('rebuild linode', () => {
    */
   it('rebuilds a linode from Account StackScript', () => {
     cy.tag('method:e2e');
-    const image = 'Alpine';
+    const image = 'Alpine 3.18';
     const region = 'us-east';
 
     // Create a StackScript to rebuild a Linode.
@@ -265,10 +259,9 @@ describe('rebuild linode', () => {
 
       openRebuildDialog(linode.label);
       findRebuildDialog(linode.label).within(() => {
-        ui.select.findByText('From Image').should('be.visible').click();
-
-        ui.select
-          .findItemByText('From Account StackScript')
+        ui.autocomplete.findByLabel('From Image').should('be.visible').click();
+        ui.autocompletePopper
+          .findByTitle('From Account StackScript')
           .should('be.visible')
           .click();
 
@@ -280,13 +273,8 @@ describe('rebuild linode', () => {
           cy.get(`[id="${stackScript.id}"][type="radio"]`).click();
         });
 
-        ui.select
-          .findByText('Choose an image')
-          .scrollIntoView()
-          .should('be.visible')
-          .click();
-
-        ui.select.findItemByText(image).should('be.visible').click();
+        ui.autocomplete.findByLabel('Images').should('be.visible').click();
+        ui.autocompletePopper.findByTitle(image).should('be.visible').click();
 
         cy.findByLabelText('Linode Label')
           .should('be.visible')
@@ -319,14 +307,13 @@ describe('rebuild linode', () => {
 
     cy.visitWithLogin(`/linodes/${mockLinode.id}?rebuild=true`);
     findRebuildDialog(mockLinode.label).within(() => {
-      ui.select.findByText('From Image').should('be.visible');
-      ui.select
-        .findByText('Choose an image')
+      ui.autocomplete.findByLabel('From Image').should('be.visible');
+      ui.autocomplete
+        .findByLabel('Images')
         .should('be.visible')
         .click()
-        .type(`${image}`);
-
-      ui.select.findItemByText(image).should('be.visible').click();
+        .type(image);
+      ui.autocompletePopper.findByTitle(image).should('be.visible').click();
 
       assertPasswordComplexity(rootPassword, 'Good');
 

diff --git a/packages/manager/cypress/e2e/core/stackscripts/create-stackscripts.spec.ts b/packages/manager/cypress/e2e/core/stackscripts/create-stackscripts.spec.ts
@@ -20,7 +20,7 @@ import { cleanUp } from 'support/util/cleanup';
 import { createTestLinode } from 'support/util/linodes';
 import { interceptGetAllImages } from 'support/intercepts/images';
 import type { Image } from '@linode/api-v4';
-import { getFilteredImagesForImageSelect } from 'src/components/ImageSelectv2/utilities';
+import { getFilteredImagesForImageSelect } from 'src/components/ImageSelect/utilities';
 
 // StackScript fixture paths.
 const stackscriptBasicPath = 'stackscripts/stackscript-basic.sh';
@@ -80,9 +80,9 @@ const fillOutStackscriptForm = (
       .type(description);
   }
 
-  cy.findByText('Target Images').click().type(`${targetImage}`);
-
-  cy.findByText(`${targetImage}`).should('be.visible').click();
+  ui.autocomplete.findByLabel('Target Images').should('be.visible').click();
+  ui.autocompletePopper.findByTitle(targetImage).should('be.visible').click();
+  ui.autocomplete.findByLabel('Target Images').click(); // Close autocomplete popper
 
   // Insert a script.
   inputStackScript(script);

diff --git a/packages/manager/cypress/e2e/core/stackscripts/update-stackscripts.spec.ts b/packages/manager/cypress/e2e/core/stackscripts/update-stackscripts.spec.ts
@@ -55,9 +55,9 @@ const fillOutStackscriptForm = (
       .type(description);
   }
 
-  cy.findByText('Target Images').click().type(targetImage);
-
-  cy.findByText(targetImage).should('be.visible').click();
+  ui.autocomplete.findByLabel('Target Images').should('be.visible').click();
+  ui.autocompletePopper.findByTitle(targetImage).should('be.visible').click();
+  ui.autocomplete.findByLabel('Target Images').click(); // Close autocomplete popper
 
   // Insert a script with invalid UDF data.
   cy.get('[data-qa-textfield-label="Script"]')

diff --git a/packages/manager/src/components/Dialog/Dialog.tsx b/packages/manager/src/components/Dialog/Dialog.tsx
@@ -40,64 +40,67 @@ export interface DialogProps extends _DialogProps {
  * > A modal can only be closed by taking direct action, clicking on a button or the “X” button, or using the `esc` key.
  *
  */
-export const Dialog = (props: DialogProps) => {
-  const theme = useTheme();
-  const {
-    children,
-    className,
-    error,
-    fullHeight,
-    fullWidth,
-    maxWidth = 'md',
-    onClose,
-    subtitle,
-    title,
-    titleBottomBorder,
-    ...rest
-  } = props;
+export const Dialog = React.forwardRef(
+  (props: DialogProps, ref: React.Ref<HTMLDivElement>) => {
+    const theme = useTheme();
+    const {
+      children,
+      className,
+      error,
+      fullHeight,
+      fullWidth,
+      maxWidth = 'md',
+      onClose,
+      subtitle,
+      title,
+      titleBottomBorder,
+      ...rest
+    } = props;
 
-  const titleID = convertForAria(title);
+    const titleID = convertForAria(title);
 
-  return (
-    <StyledDialog
-      aria-labelledby={titleID}
-      data-qa-dialog
-      data-qa-drawer
-      data-testid="drawer"
-      fullHeight={fullHeight}
-      fullWidth={fullWidth}
-      maxWidth={(fullWidth && maxWidth) ?? undefined}
-      onClose={onClose}
-      role="dialog"
-      title={title}
-      {...rest}
-    >
-      <Box
-        sx={{
-          alignItems: 'center',
-        }}
+    return (
+      <StyledDialog
+        aria-labelledby={titleID}
+        data-qa-dialog
+        data-qa-drawer
+        data-testid="drawer"
+        fullHeight={fullHeight}
+        fullWidth={fullWidth}
+        maxWidth={(fullWidth && maxWidth) ?? undefined}
+        onClose={onClose}
+        ref={ref}
+        role="dialog"
+        title={title}
+        {...rest}
       >
-        <DialogTitle
-          id={titleID}
-          onClose={() => onClose && onClose({}, 'backdropClick')}
-          subtitle={subtitle}
-          title={title}
-        />
-        {titleBottomBorder && <StyledHr />}
-        <DialogContent
+        <Box
           sx={{
-            overflowX: 'hidden',
-            paddingBottom: theme.spacing(3),
+            alignItems: 'center',
           }}
-          className={className}
         >
-          {error && <Notice text={error} variant="error" />}
-          {children}
-        </DialogContent>
-      </Box>
-    </StyledDialog>
-  );
-};
+          <DialogTitle
+            id={titleID}
+            onClose={() => onClose && onClose({}, 'backdropClick')}
+            subtitle={subtitle}
+            title={title}
+          />
+          {titleBottomBorder && <StyledHr />}
+          <DialogContent
+            sx={{
+              overflowX: 'hidden',
+              paddingBottom: theme.spacing(3),
+            }}
+            className={className}
+          >
+            {error && <Notice text={error} variant="error" />}
+            {children}
+          </DialogContent>
+        </Box>
+      </StyledDialog>
+    );
+  }
+);
 
 const StyledDialog = styled(_Dialog, {
   shouldForwardProp: omittedProps(['fullHeight', 'title']),

diff --git a/...ents/ImageSelectv2/ImageOptionv2.test.tsx → ...mponents/ImageSelect/ImageOption.test.tsx b/...ents/ImageSelectv2/ImageOptionv2.test.tsx → ...mponents/ImageSelect/ImageOption.test.tsx
@@ -3,44 +3,47 @@ import React from 'react';
 import { imageFactory } from 'src/factories';
 import { renderWithTheme } from 'src/utilities/testHelpers';
 
-import { ImageOptionv2 } from './ImageOptionv2';
+import { ImageOption } from './ImageOption';
 
-describe('ImageOptionv2', () => {
+describe('ImageOption', () => {
   it('renders the image label', () => {
     const image = imageFactory.build({ eol: null });
 
     const { getByText } = renderWithTheme(
-      <ImageOptionv2 image={image} isSelected={false} listItemProps={{}} />
+      <ImageOption image={image} isSelected={false} listItemProps={{}} />
     );
 
     expect(getByText(image.label)).toBeVisible();
   });
+
   it('renders an OS icon', () => {
     const image = imageFactory.build();
 
     const { getByTestId } = renderWithTheme(
-      <ImageOptionv2 image={image} isSelected={false} listItemProps={{}} />
+      <ImageOption image={image} isSelected={false} listItemProps={{}} />
     );
 
     expect(getByTestId('os-icon')).toBeVisible();
   });
+
   it('renders a metadata (cloud-init) icon if the flag is on and the image supports cloud-init', () => {
     const image = imageFactory.build({ capabilities: ['cloud-init'] });
 
     const { getByLabelText } = renderWithTheme(
-      <ImageOptionv2 image={image} isSelected={false} listItemProps={{}} />,
+      <ImageOption image={image} isSelected={false} listItemProps={{}} />,
       { flags: { metadata: true } }
     );
 
     expect(
       getByLabelText('This image supports our Metadata service via cloud-init.')
     ).toBeVisible();
   });
+
   it('renders a distributed icon if image has the "distributed-sites" capability', () => {
     const image = imageFactory.build({ capabilities: ['distributed-sites'] });
 
     const { getByLabelText } = renderWithTheme(
-      <ImageOptionv2 image={image} isSelected={false} listItemProps={{}} />
+      <ImageOption image={image} isSelected={false} listItemProps={{}} />
     );
 
     expect(
@@ -54,7 +57,7 @@ describe('ImageOptionv2', () => {
     const image = imageFactory.build({ deprecated: true });
 
     const { getByText } = renderWithTheme(
-      <ImageOptionv2 image={image} isSelected={false} listItemProps={{}} />
+      <ImageOption image={image} isSelected={false} listItemProps={{}} />
     );
 
     expect(getByText(`${image.label} (deprecated)`)).toBeVisible();
@@ -67,9 +70,19 @@ describe('ImageOptionv2', () => {
     });
 
     const { getByText } = renderWithTheme(
-      <ImageOptionv2 image={image} isSelected={false} listItemProps={{}} />
+      <ImageOption image={image} isSelected={false} listItemProps={{}} />
     );
 
     expect(getByText(`${image.label} (deprecated)`)).toBeVisible();
   });
+
+  it('should show the selected icon when isSelected is true', () => {
+    const image = imageFactory.build();
+
+    const { getByTestId } = renderWithTheme(
+      <ImageOption image={image} isSelected={true} listItemProps={{}} />
+    );
+
+    expect(getByTestId('DoneIcon')).toBeVisible();
+  });
 });