From 12230108ca279a7d48d7bc135499cb86f78fff61 Mon Sep 17 00:00:00 2001 From: Jeremy Pople <jeremy@ethyca.com> Date: Thu, 15 Aug 2024 09:10:59 -0500 Subject: [PATCH 1/4] changes to D&D action buttons --- .../ActionButton.tsx | 27 ++++++------ .../DetectionItemActions.tsx | 43 ++++++++----------- .../DiscoveryItemActions.tsx | 23 +++++----- .../tables/DiscoveryFieldBulkActions.tsx | 8 ++-- .../tables/DiscoveryTableBulkActions.tsx | 8 ++-- 5 files changed, 51 insertions(+), 58 deletions(-) diff --git a/clients/admin-ui/src/features/data-discovery-and-detection/ActionButton.tsx b/clients/admin-ui/src/features/data-discovery-and-detection/ActionButton.tsx index 3d1e7fb112..1dd04ba43a 100644 --- a/clients/admin-ui/src/features/data-discovery-and-detection/ActionButton.tsx +++ b/clients/admin-ui/src/features/data-discovery-and-detection/ActionButton.tsx @@ -1,31 +1,32 @@ -import { Button, ButtonProps, Text } from "fidesui"; +import { Button, ButtonProps } from "fidesui"; import { ReactElement } from "react"; +interface ActionButtonProps extends ButtonProps { + title: string; + icon?: ReactElement; +} + const ActionButton = ({ title, icon, onClick, - disabled, + isDisabled, + isLoading, variant = "outline", colorScheme = undefined, -}: { - title: string; - icon?: ReactElement; - onClick: () => void; - disabled?: boolean; -} & Pick<ButtonProps, "variant" | "colorScheme">) => ( +}: ActionButtonProps) => ( <Button size="xs" onClick={onClick} - isDisabled={disabled} + isDisabled={isDisabled} variant={variant} colorScheme={colorScheme} data-testid={`action-${title}`} + isLoading={isLoading} + loadingText={title} + leftIcon={icon} > - {icon} - <Text marginLeft={icon && 1} fontWeight="semibold" fontSize={12}> - {title} - </Text> + {title} </Button> ); export default ActionButton; diff --git a/clients/admin-ui/src/features/data-discovery-and-detection/DetectionItemActions.tsx b/clients/admin-ui/src/features/data-discovery-and-detection/DetectionItemActions.tsx index 2e838797d9..6003026a6b 100644 --- a/clients/admin-ui/src/features/data-discovery-and-detection/DetectionItemActions.tsx +++ b/clients/admin-ui/src/features/data-discovery-and-detection/DetectionItemActions.tsx @@ -1,11 +1,4 @@ -import { - ButtonSpinner, - CheckIcon, - HStack, - ViewIcon, - ViewOffIcon, -} from "fidesui"; -import { useState } from "react"; +import { CheckIcon, HStack, ViewIcon, ViewOffIcon } from "fidesui"; import { useAlert } from "~/features/common/hooks"; import { DiffStatus, StagedResource } from "~/types/api"; @@ -27,12 +20,16 @@ interface DetectionItemActionProps { const DetectionItemAction = ({ resource }: DetectionItemActionProps) => { const resourceType = findResourceType(resource); - const [confirmResourceMutation] = useConfirmResourceMutation(); - const [muteResourceMutation] = useMuteResourceMutation(); - const [unmuteResourceMutation] = useUnmuteResourceMutation(); + const [confirmResourceMutation, { isLoading: confirmIsLoading }] = + useConfirmResourceMutation(); + const [muteResourceMutation, { isLoading: muteIsLoading }] = + useMuteResourceMutation(); + const [unmuteResourceMutation, { isLoading: unmuteIsLoading }] = + useUnmuteResourceMutation(); const { successAlert } = useAlert(); - const [isProcessingAction, setIsProcessingAction] = useState(false); + const anyActionIsLoading = + confirmIsLoading || muteIsLoading || unmuteIsLoading; const { diff_status: diffStatus, child_diff_statuses: childDiffStatus } = resource; @@ -60,7 +57,6 @@ const DetectionItemAction = ({ resource }: DetectionItemActionProps) => { title="Monitor" icon={<MonitorOnIcon />} onClick={async () => { - setIsProcessingAction(true); await confirmResourceMutation({ staged_resource_urn: resource.urn, monitor_config_id: resource.monitor_config_id!, @@ -69,9 +65,9 @@ const DetectionItemAction = ({ resource }: DetectionItemActionProps) => { "Data discovery has started. The results may take some time to appear in the “Data discovery“ tab.", `${resource.name || "The resource"} is now being monitored.`, ); - setIsProcessingAction(false); }} - disabled={isProcessingAction} + isDisabled={anyActionIsLoading} + isLoading={confirmIsLoading} /> )} {showUnmuteAction && ( @@ -79,7 +75,6 @@ const DetectionItemAction = ({ resource }: DetectionItemActionProps) => { title="Monitor" icon={isSchemaType ? <MonitorOnIcon /> : <ViewIcon />} onClick={async () => { - setIsProcessingAction(true); await unmuteResourceMutation({ staged_resource_urn: resource.urn, }); @@ -87,9 +82,9 @@ const DetectionItemAction = ({ resource }: DetectionItemActionProps) => { "Data discovery has started. The results may take some time to appear in the “Data discovery“ tab.", `${resource.name || "The resource"} is now being monitored.`, ); - setIsProcessingAction(false); }} - disabled={isProcessingAction} + isDisabled={anyActionIsLoading} + isLoading={unmuteIsLoading} /> )} {showConfirmAction && ( @@ -97,7 +92,6 @@ const DetectionItemAction = ({ resource }: DetectionItemActionProps) => { title="Confirm" icon={<CheckIcon />} onClick={async () => { - setIsProcessingAction(true); await confirmResourceMutation({ staged_resource_urn: resource.urn, monitor_config_id: resource.monitor_config_id!, @@ -106,9 +100,9 @@ const DetectionItemAction = ({ resource }: DetectionItemActionProps) => { `These changes have been added to a Fides dataset. To view, navigate to "Manage datasets".`, `Table changes confirmed`, ); - setIsProcessingAction(false); }} - disabled={isProcessingAction} + isDisabled={anyActionIsLoading} + isLoading={confirmIsLoading} /> )} {/* Positive Actions (Monitor, Confirm) goes first. Negative actions such as ignore should be last */} @@ -117,17 +111,14 @@ const DetectionItemAction = ({ resource }: DetectionItemActionProps) => { title="Ignore" icon={isSchemaType ? <MonitorOffIcon /> : <ViewOffIcon />} onClick={async () => { - setIsProcessingAction(true); await muteResourceMutation({ staged_resource_urn: resource.urn, }); - setIsProcessingAction(false); }} - disabled={isProcessingAction} + isDisabled={anyActionIsLoading} + isLoading={muteIsLoading} /> )} - - {isProcessingAction ? <ButtonSpinner position="static" /> : null} </HStack> ); }; diff --git a/clients/admin-ui/src/features/data-discovery-and-detection/DiscoveryItemActions.tsx b/clients/admin-ui/src/features/data-discovery-and-detection/DiscoveryItemActions.tsx index 44e470c70a..bd434b471c 100644 --- a/clients/admin-ui/src/features/data-discovery-and-detection/DiscoveryItemActions.tsx +++ b/clients/admin-ui/src/features/data-discovery-and-detection/DiscoveryItemActions.tsx @@ -1,5 +1,4 @@ -import { ButtonSpinner, CheckIcon, HStack, ViewOffIcon } from "fidesui"; -import { useState } from "react"; +import { CheckIcon, HStack, ViewOffIcon } from "fidesui"; import { useAlert } from "~/features/common/hooks"; import { DiscoveryMonitorItem } from "~/features/data-discovery-and-detection/types/DiscoveryMonitorItem"; @@ -16,10 +15,12 @@ interface DiscoveryItemActionsProps { } const DiscoveryItemActions = ({ resource }: DiscoveryItemActionsProps) => { - const [promoteResourceMutation] = usePromoteResourceMutation(); - const [muteResourceMutation] = useMuteResourceMutation(); + const [promoteResourceMutation, { isLoading: promoteIsLoading }] = + usePromoteResourceMutation(); + const [muteResourceMutation, { isLoading: muteIsLoading }] = + useMuteResourceMutation(); - const [isProcessingAction, setIsProcessingAction] = useState(false); + const anyActionIsLoading = promoteIsLoading || muteIsLoading; const { diff_status: diffStatus, @@ -55,7 +56,6 @@ const DiscoveryItemActions = ({ resource }: DiscoveryItemActionsProps) => { title="Confirm" icon={<CheckIcon />} onClick={async () => { - setIsProcessingAction(true); await promoteResourceMutation({ staged_resource_urn: resource.urn, }); @@ -63,9 +63,9 @@ const DiscoveryItemActions = ({ resource }: DiscoveryItemActionsProps) => { `These changes have been added to a Fides dataset. To view, navigate to "Manage datasets".`, `Table changes confirmed`, ); - setIsProcessingAction(false); }} - disabled={isProcessingAction} + isDisabled={anyActionIsLoading} + isLoading={promoteIsLoading} /> )} {showMuteAction && ( @@ -73,17 +73,14 @@ const DiscoveryItemActions = ({ resource }: DiscoveryItemActionsProps) => { title="Ignore" icon={<ViewOffIcon />} onClick={async () => { - setIsProcessingAction(true); await muteResourceMutation({ staged_resource_urn: resource.urn, }); - setIsProcessingAction(false); }} - disabled={isProcessingAction} + isDisabled={anyActionIsLoading} + isLoading={muteIsLoading} /> )} - - {isProcessingAction ? <ButtonSpinner position="static" /> : null} </HStack> ); }; diff --git a/clients/admin-ui/src/features/data-discovery-and-detection/tables/DiscoveryFieldBulkActions.tsx b/clients/admin-ui/src/features/data-discovery-and-detection/tables/DiscoveryFieldBulkActions.tsx index 33a998a33a..4ad052a0b0 100644 --- a/clients/admin-ui/src/features/data-discovery-and-detection/tables/DiscoveryFieldBulkActions.tsx +++ b/clients/admin-ui/src/features/data-discovery-and-detection/tables/DiscoveryFieldBulkActions.tsx @@ -16,7 +16,7 @@ const DiscoveryFieldBulkActions = ({ const [muteResourceMutationTrigger, { isLoading: isMuteLoading }] = useMuteResourcesMutation(); - const isLoading = isPromoteLoading || isMuteLoading; + const anyActionIsLoading = isPromoteLoading || isMuteLoading; const handleIgnoreClicked = async (urns: string[]) => { await muteResourceMutationTrigger({ @@ -42,15 +42,17 @@ const DiscoveryFieldBulkActions = ({ title="Confirm all" icon={<CheckIcon />} onClick={() => handleConfirmClicked([resourceUrn])} - disabled={isLoading} + isDisabled={anyActionIsLoading} + isLoading={isPromoteLoading} variant="solid" colorScheme="primary" /> <ActionButton title="Ignore all" icon={<ViewOffIcon />} - disabled={isLoading} onClick={() => handleIgnoreClicked([resourceUrn])} + isDisabled={anyActionIsLoading} + isLoading={isMuteLoading} /> </ButtonGroup> </Flex> diff --git a/clients/admin-ui/src/features/data-discovery-and-detection/tables/DiscoveryTableBulkActions.tsx b/clients/admin-ui/src/features/data-discovery-and-detection/tables/DiscoveryTableBulkActions.tsx index dab27c8025..88c7f097b5 100644 --- a/clients/admin-ui/src/features/data-discovery-and-detection/tables/DiscoveryTableBulkActions.tsx +++ b/clients/admin-ui/src/features/data-discovery-and-detection/tables/DiscoveryTableBulkActions.tsx @@ -16,7 +16,7 @@ const DiscoveryTableBulkActions = ({ const [muteResourcesMutationTrigger, { isLoading: isMuteLoading }] = useMuteResourcesMutation(); - const isLoading = isPromoteLoading || isMuteLoading; + const anyActionIsLoading = isPromoteLoading || isMuteLoading; const handleConfirmClicked = async (urns: string[]) => { await promoteResourcesMutationTrigger({ @@ -52,14 +52,16 @@ const DiscoveryTableBulkActions = ({ title="Confirm" icon={<CheckIcon />} onClick={() => handleConfirmClicked(selectedUrns)} - disabled={isLoading} + isDisabled={anyActionIsLoading} + isLoading={isPromoteLoading} variant="solid" colorScheme="primary" /> <ActionButton title="Ignore" icon={<ViewOffIcon />} - disabled={isLoading} + isDisabled={anyActionIsLoading} + isLoading={isMuteLoading} onClick={() => handleIgnoreClicked(selectedUrns)} /> </ButtonGroup> From ae5cc8e069a4f9a182128c9b38e88b7fc3178b12 Mon Sep 17 00:00:00 2001 From: Jeremy Pople <jeremy@ethyca.com> Date: Thu, 15 Aug 2024 09:13:00 -0500 Subject: [PATCH 2/4] also update scan button on monitor table --- .../configure-monitor/MonitorConfigActionsCell.tsx | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/clients/admin-ui/src/features/integrations/configure-monitor/MonitorConfigActionsCell.tsx b/clients/admin-ui/src/features/integrations/configure-monitor/MonitorConfigActionsCell.tsx index 9333c926dc..edddca6b5c 100644 --- a/clients/admin-ui/src/features/integrations/configure-monitor/MonitorConfigActionsCell.tsx +++ b/clients/admin-ui/src/features/integrations/configure-monitor/MonitorConfigActionsCell.tsx @@ -28,7 +28,8 @@ const MonitorConfigActionsCell = ({ defaultSuccessMsg: "Monitor deleted successfully", }); - const [executeMonitor] = useExecuteDiscoveryMonitorMutation(); + const [executeMonitor, { isLoading: executeIsLoading }] = + useExecuteDiscoveryMonitorMutation(); const { toastResult: toastExecuteResult } = useQueryResultToast({ defaultErrorMsg: "A problem occurred initiating monitor execution", defaultSuccessMsg: "Monitor execution successfully started", @@ -78,7 +79,11 @@ const MonitorConfigActionsCell = ({ data-testid="delete-monitor-btn" /> </Tooltip> - <ActionButton onClick={handleExecute} title="Scan" /> + <ActionButton + onClick={handleExecute} + title="Scan" + isLoading={executeIsLoading} + /> </ButtonGroup> </> ); From 68805aa3112051d445827d371da161d253dd64d8 Mon Sep 17 00:00:00 2001 From: Jeremy Pople <jeremy@ethyca.com> Date: Thu, 15 Aug 2024 09:28:32 -0500 Subject: [PATCH 3/4] update changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6311552560..8cf19eb295 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,8 @@ The types of changes are: ## [Unreleased](https://github.com/ethyca/fides/compare/2.43.0...main) +### Changed +- Tweaked behavior of loading state on D&D table actions buttons [#5201](https://github.com/ethyca/fides/pull/5201) ## [2.43.0](https://github.com/ethyca/fides/compare/2.42.1...2.43.0) From a496622850fdb70f883c9a0f47586637d473a871 Mon Sep 17 00:00:00 2001 From: Jeremy Pople <jeremy@ethyca.com> Date: Fri, 16 Aug 2024 17:21:30 -0500 Subject: [PATCH 4/4] update button prop handling --- .../data-discovery-and-detection/ActionButton.tsx | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/clients/admin-ui/src/features/data-discovery-and-detection/ActionButton.tsx b/clients/admin-ui/src/features/data-discovery-and-detection/ActionButton.tsx index 1dd04ba43a..712b3c937e 100644 --- a/clients/admin-ui/src/features/data-discovery-and-detection/ActionButton.tsx +++ b/clients/admin-ui/src/features/data-discovery-and-detection/ActionButton.tsx @@ -9,22 +9,16 @@ interface ActionButtonProps extends ButtonProps { const ActionButton = ({ title, icon, - onClick, - isDisabled, - isLoading, variant = "outline", - colorScheme = undefined, + ...props }: ActionButtonProps) => ( <Button size="xs" - onClick={onClick} - isDisabled={isDisabled} variant={variant} - colorScheme={colorScheme} data-testid={`action-${title}`} - isLoading={isLoading} loadingText={title} leftIcon={icon} + {...props} > {title} </Button>