Merge pull request #645 from gadget-inc/mill/removeFrameFromAutoTable

Removed deprecated Polaris `Frame` usage from AutoTable
gadget-inc · Sep 17, 2024 · a6e5588 · a6e5588
2 parents 41ab357 + b4ccb39
commit a6e5588
Show file tree

Hide file tree

Showing 5 changed files with 127 additions and 145 deletions.
diff --git a/packages/react/.changeset/eight-pans-beg.md b/packages/react/.changeset/eight-pans-beg.md
@@ -0,0 +1,5 @@
+---
+"@gadgetinc/react": patch
+---
+
+Removed deprecated Polaris `Frame` usage from AutoTable
diff --git a/packages/react/.changeset/fair-files-repair.md b/packages/react/.changeset/fair-files-repair.md
@@ -0,0 +1,5 @@
+---
+"@gadgetinc/react": patch
+---
+
+Fixed bug in `AutoForm` where dynamically changing findBy object props did not cause the form to re-render
diff --git a/packages/react/cypress/component/auto/table/AutoTableBulkActions.cy.tsx b/packages/react/cypress/component/auto/table/AutoTableBulkActions.cy.tsx
@@ -115,16 +115,17 @@ describe("AutoTable - Bulk actions", () => {
     cy.wait("@getWidgets").its("request.body.variables").should("deep.equal", { first: 50 }); // No search value
 
     cy.contains(ActionSuccessMessage);
+    cy.get("button").contains("Close").click();
 
     // Now ensure that error response appears in the modal
     selectRecordIds(["20", "21", "22"]);
+
     openBulkAction("Delete");
 
     mockBulkDeleteWidgets(bulkDeleteFailureResponse, "bulkDeleteWidgets2");
 
     cy.get("button").contains("Run").click();
     cy.wait("@bulkDeleteWidgets2");
-
     cy.contains(ActionErrorMessage);
   });
 
@@ -152,6 +153,7 @@ describe("AutoTable - Bulk actions", () => {
       cy.wait("@getWidgets").its("request.body.variables").should("deep.equal", { first: 50 }); // No search value
 
       cy.contains(ActionSuccessMessage);
+      cy.get("button").contains("Close").click();
     });
   });
 });

diff --git a/packages/react/src/auto/polaris/PolarisAutoBulkActionModal.tsx b/packages/react/src/auto/polaris/PolarisAutoBulkActionModal.tsx
@@ -1,4 +1,4 @@
-import { Button, ButtonGroup, Modal, Spinner, Text, Toast } from "@shopify/polaris";
+import { Banner, Button, ButtonGroup, Modal, Spinner, Text } from "@shopify/polaris";
 import React, { useCallback, useEffect, useMemo } from "react";
 import type { TableRow } from "../../use-table/types.js";
 import { useBulkAction } from "../../useBulkAction.js";
@@ -12,32 +12,7 @@ export const PolarisAutoBulkActionModal = (props: {
   selectedRows: TableRow[];
   clearSelection: () => void;
 }) => {
-  const [toastMessage, setToastMessage] = React.useState<string | undefined>(undefined);
-
-  return (
-    <>
-      {toastMessage && (
-        <Toast
-          content={toastMessage}
-          onDismiss={() => setToastMessage(undefined)}
-          duration={4500}
-          error={toastMessage.includes(ActionErrorMessage)}
-        />
-      )}
-      <BulkActionModal {...props} setToastMessage={setToastMessage} />
-    </>
-  );
-};
-
-const BulkActionModal = (props: {
-  model: any;
-  modelActionDetails?: ModelActionDetails;
-  ids: string[];
-  selectedRows: TableRow[];
-  clearSelection: () => void;
-  setToastMessage: (message: string) => void;
-}) => {
-  const { model, modelActionDetails, ids, selectedRows, clearSelection, setToastMessage } = props;
+  const { model, modelActionDetails, ids, selectedRows, clearSelection } = props;
 
   const [showModal, setShowModal] = React.useState(!!modelActionDetails);
   const [actionName, setActionName] = React.useState(modelActionDetails?.apiIdentifier);
@@ -55,23 +30,24 @@ const BulkActionModal = (props: {
 
   const modalTitle = useMemo(() => humanizeCamelCase(actionName ?? ""), [actionName]);
 
-  const closeModal = useCallback(() => setShowModal(false), [setShowModal]);
+  const closeAndClear = useCallback(() => {
+    setShowModal(false);
+    clearSelection();
+  }, [setShowModal, clearSelection]);
 
   if (!actionIsLoaded || !isBulkGadgetAction) {
     return null;
   }
 
   return (
     <>
-      <Modal onClose={() => setShowModal(false)} title={modalTitle} open={showModal}>
+      <Modal onClose={closeAndClear} title={modalTitle} open={showModal}>
         <GadgetBulkActionModalContent
           model={model}
           modelActionDetails={modelActionDetails}
           actionLabel={modalTitle.replace("Bulk ", "")}
           ids={ids}
-          close={closeModal}
-          clearSelection={clearSelection}
-          setToastMessage={setToastMessage}
+          close={closeAndClear}
         />
       </Modal>
     </>
@@ -87,10 +63,8 @@ const GadgetBulkActionModalContent = (props: {
   actionLabel: string;
   ids: string[];
   close: () => void;
-  clearSelection: () => void;
-  setToastMessage: (message: string) => void;
 }) => {
-  const { model, modelActionDetails, actionLabel: actionName, ids, close, clearSelection, setToastMessage } = props;
+  const { model, modelActionDetails, actionLabel: actionName, ids, close } = props;
 
   const [hasRun, setHasRun] = React.useState(false);
 
@@ -100,25 +74,26 @@ const GadgetBulkActionModalContent = (props: {
 
   const hasError = !!(error || (data && (data as any).success === false));
 
+  const actionResultBanner = useMemo(
+    () =>
+      hasError ? (
+        <Banner title={`${actionName}${ActionErrorMessage}`} tone="critical" />
+      ) : (
+        <Banner title={`${actionName}${ActionSuccessMessage}`} tone="success" />
+      ),
+    [hasError]
+  );
+
   const runAction = useCallback(() => {
     void runBulkAction(ids);
     setHasRun(true);
-  }, [runBulkAction, ids, clearSelection, close]);
-
-  // Automatically close the modal if the action is successful
-  useEffect(() => {
-    if (!fetching && hasRun) {
-      clearSelection();
-      close();
-      setToastMessage(hasError ? `${actionName}${ActionErrorMessage}` : `${actionName}${ActionSuccessMessage}`);
-    }
-  }, [fetching, hasRun, hasError, clearSelection, close, setToastMessage, actionName]);
+  }, [runBulkAction, ids]);
 
   return (
     <>
       <Modal.Section>
         {fetching && <CenteredSpinner />}
-        {!fetching && !hasRun && <RunActionConfirmationText count={ids.length} />}
+        {!fetching && (hasRun ? actionResultBanner : <RunActionConfirmationText count={ids.length} />)}
       </Modal.Section>
       <Modal.Section>
         <div style={{ float: "right", paddingBottom: "16px" }}>

diff --git a/packages/react/src/auto/polaris/PolarisAutoTable.tsx b/packages/react/src/auto/polaris/PolarisAutoTable.tsx
@@ -6,13 +6,11 @@ import {
   Box,
   DataTable,
   EmptySearchResult,
-  Frame,
   IndexFilters,
   IndexTable,
   SkeletonBodyText,
   useSetIndexFiltersMode,
 } from "@shopify/polaris";
-
 import pluralize from "pluralize";
 import type { ReactNode } from "react";
 import React, { useCallback, useMemo } from "react";
@@ -153,105 +151,102 @@ const PolarisAutoTableComponent = <
 
   return (
     <AutoTableContext.Provider value={[methods, refresh]}>
-      <Frame>
-        <BlockStack>
-          <PolarisAutoBulkActionModal
-            model={props.model}
-            modelActionDetails={selectedModelActionDetails}
-            ids={selection.recordIds}
-            selectedRows={selectedRows}
-            clearSelection={selection.clearAll}
+      <BlockStack>
+        <PolarisAutoBulkActionModal
+          model={props.model}
+          modelActionDetails={selectedModelActionDetails}
+          ids={selection.recordIds}
+          selectedRows={selectedRows}
+          clearSelection={selection.clearAll}
+        />
+        {searchable && (
+          <IndexFilters
+            mode={mode}
+            setMode={setMode}
+            appliedFilters={[]}
+            filters={[]}
+            onClearAll={() => undefined}
+            tabs={[]}
+            canCreateNewView={false}
+            selected={1}
+            loading={fetching}
+            cancelAction={{ onAction: () => search.clear() }}
+            disabled={!!error}
+            // Search
+            queryValue={search.value}
+            onQueryChange={search.set}
+            onQueryClear={search.clear}
           />
+        )}
 
-          {searchable && (
-            <IndexFilters
-              mode={mode}
-              setMode={setMode}
-              appliedFilters={[]}
-              filters={[]}
-              onClearAll={() => undefined}
-              tabs={[]}
-              canCreateNewView={false}
-              selected={1}
-              loading={fetching}
-              cancelAction={{ onAction: () => search.clear() }}
-              disabled={!!error}
-              // Search
-              queryValue={search.value}
-              onQueryChange={search.set}
-              onQueryClear={search.clear}
-            />
-          )}
+        {error && (
+          <Box paddingBlockStart="200" paddingBlockEnd="1000">
+            <Banner title={error.message} tone="critical" />
+          </Box>
+        )}
 
-          {error && (
-            <Box paddingBlockStart="200" paddingBlockEnd="1000">
-              <Banner title={error.message} tone="critical" />
-            </Box>
-          )}
-
-          <IndexTable
-            selectedItemsCount={selection.recordIds.length}
-            {...disablePaginatedSelectAllButton}
-            onSelectionChange={selection.onSelectionChange}
-            {...polarisTableProps}
-            promotedBulkActions={promotedBulkActions.length ? promotedBulkActions : undefined}
-            bulkActions={bulkActions.length ? bulkActions : undefined}
-            resourceName={resourceName}
-            emptyState={props.emptyState ?? <EmptySearchResult title={`No ${resourceName.plural} yet`} description={""} withIllustration />}
-            loading={fetching}
-            hasMoreItems={page.hasNextPage}
-            itemCount={
-              error
-                ? 1 // Don't show the empty state if there's an error
-                : rows?.length ?? 0
-            }
-            pagination={
-              paginate
-                ? {
-                    hasNext: page.hasNextPage,
-                    hasPrevious: page.hasPreviousPage,
-                    onNext: page.goToNextPage,
-                    onPrevious: page.goToPreviousPage,
-                  }
-                : undefined
-            }
-            sortDirection={gadgetToPolarisDirection(sort.direction)}
-            sortColumnIndex={columns ? getColumnIndex(columns, sort.column) : undefined}
-            onSort={(headingIndex) => handleColumnSort(headingIndex)}
-            selectable={props.selectable === undefined ? bulkActionOptions.length !== 0 : props.selectable}
-            lastColumnSticky={props.lastColumnSticky}
-            hasZebraStriping={props.hasZebraStriping}
-            condensed={props.condensed}
-          >
-            {rows &&
-              columns &&
-              rows.map((row, index) => {
-                const rawRecord = rawRecords?.[index];
-                return (
-                  <IndexTable.Row
-                    key={row.id as string}
-                    id={row.id as string}
-                    position={index}
-                    onClick={onClick ? onClickCallback(row, rawRecord) : undefined}
-                    selected={selection.recordIds.includes(row.id as string)}
-                  >
-                    {columns.map((column) => (
-                      <IndexTable.Cell key={column.identifier}>
-                        <div style={{ maxWidth: "200px" }}>
-                          {column.type == "CustomRenderer" ? (
-                            (row[column.identifier] as ReactNode)
-                          ) : (
-                            <PolarisAutoTableCellRenderer column={column} value={row[column.identifier] as ColumnValueType} />
-                          )}
-                        </div>
-                      </IndexTable.Cell>
-                    ))}
-                  </IndexTable.Row>
-                );
-              })}
-          </IndexTable>
-        </BlockStack>
-      </Frame>
+        <IndexTable
+          selectedItemsCount={selection.recordIds.length}
+          {...disablePaginatedSelectAllButton}
+          onSelectionChange={selection.onSelectionChange}
+          {...polarisTableProps}
+          promotedBulkActions={promotedBulkActions.length ? promotedBulkActions : undefined}
+          bulkActions={bulkActions.length ? bulkActions : undefined}
+          resourceName={resourceName}
+          emptyState={props.emptyState ?? <EmptySearchResult title={`No ${resourceName.plural} yet`} description={""} withIllustration />}
+          loading={fetching}
+          hasMoreItems={page.hasNextPage}
+          itemCount={
+            error
+              ? 1 // Don't show the empty state if there's an error
+              : rows?.length ?? 0
+          }
+          pagination={
+            paginate
+              ? {
+                  hasNext: page.hasNextPage,
+                  hasPrevious: page.hasPreviousPage,
+                  onNext: page.goToNextPage,
+                  onPrevious: page.goToPreviousPage,
+                }
+              : undefined
+          }
+          sortDirection={gadgetToPolarisDirection(sort.direction)}
+          sortColumnIndex={columns ? getColumnIndex(columns, sort.column) : undefined}
+          onSort={(headingIndex) => handleColumnSort(headingIndex)}
+          selectable={props.selectable === undefined ? bulkActionOptions.length !== 0 : props.selectable}
+          lastColumnSticky={props.lastColumnSticky}
+          hasZebraStriping={props.hasZebraStriping}
+          condensed={props.condensed}
+        >
+          {rows &&
+            columns &&
+            rows.map((row, index) => {
+              const rawRecord = rawRecords?.[index];
+              return (
+                <IndexTable.Row
+                  key={row.id as string}
+                  id={row.id as string}
+                  position={index}
+                  onClick={onClick ? onClickCallback(row, rawRecord) : undefined}
+                  selected={selection.recordIds.includes(row.id as string)}
+                >
+                  {columns.map((column) => (
+                    <IndexTable.Cell key={column.identifier}>
+                      <div style={{ maxWidth: "200px" }}>
+                        {column.type == "CustomRenderer" ? (
+                          (row[column.identifier] as ReactNode)
+                        ) : (
+                          <PolarisAutoTableCellRenderer column={column} value={row[column.identifier] as ColumnValueType} />
+                        )}
+                      </div>
+                    </IndexTable.Cell>
+                  ))}
+                </IndexTable.Row>
+              );
+            })}
+        </IndexTable>
+      </BlockStack>
     </AutoTableContext.Provider>
   );
 };