From 28a448ca00fa287bd46375c9aab4a1a9f195e5e3 Mon Sep 17 00:00:00 2001 From: Usame Algan Date: Tue, 1 Feb 2022 17:57:06 +0100 Subject: [PATCH 1/6] fix: Streamline action button behaviour --- .../components/Transactions/TxList/TxCollapsedActions.tsx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/routes/safe/components/Transactions/TxList/TxCollapsedActions.tsx b/src/routes/safe/components/Transactions/TxList/TxCollapsedActions.tsx index b6ba8bea74..a39c4d0ae7 100644 --- a/src/routes/safe/components/Transactions/TxList/TxCollapsedActions.tsx +++ b/src/routes/safe/components/Transactions/TxList/TxCollapsedActions.tsx @@ -37,6 +37,11 @@ export const TxCollapsedActions = ({ transaction }: TxCollapsedActionsProps): Re const txStatus = useTxStatus(transaction) const isAwaitingEx = isAwaitingExecution(txStatus) + const onExecuteOrConfirm = (event) => { + handleOnMouseLeave() + handleConfirmButtonClick(event) + } + const getTitle = () => { if (isAwaitingEx) { return (transaction.executionInfo as MultisigExecutionInfo)?.nonce === nonce @@ -53,7 +58,7 @@ export const TxCollapsedActions = ({ transaction }: TxCollapsedActionsProps): Re Date: Tue, 1 Feb 2022 18:02:42 +0100 Subject: [PATCH 2/6] fix: Disable queued Tx interaction while replacement Tx is pending --- .../Transactions/TxList/TxHoverProvider.tsx | 11 ++++++++++- .../components/Transactions/TxList/TxQueueRow.tsx | 8 +++++--- .../TxList/hooks/useActionButtonsHandlers.ts | 10 +++++++++- .../safe/components/Transactions/TxList/styled.tsx | 10 +++++++--- 4 files changed, 31 insertions(+), 8 deletions(-) diff --git a/src/routes/safe/components/Transactions/TxList/TxHoverProvider.tsx b/src/routes/safe/components/Transactions/TxList/TxHoverProvider.tsx index 6051467839..1afcd68ccf 100644 --- a/src/routes/safe/components/Transactions/TxList/TxHoverProvider.tsx +++ b/src/routes/safe/components/Transactions/TxList/TxHoverProvider.tsx @@ -3,13 +3,22 @@ import { createContext, ReactElement, ReactNode, useState } from 'react' export const TxHoverContext = createContext<{ activeHover?: string setActiveHover: (activeHover?: string) => void + pendingTx?: string + setPendingTx: (hasPendingTx?: string) => void }>({ activeHover: undefined, setActiveHover: () => {}, + pendingTx: undefined, + setPendingTx: () => {}, }) export const TxHoverProvider = ({ children }: { children: ReactNode }): ReactElement => { const [activeHover, setActiveHover] = useState() + const [pendingTx, setPendingTx] = useState() - return {children} + return ( + + {children} + + ) } diff --git a/src/routes/safe/components/Transactions/TxList/TxQueueRow.tsx b/src/routes/safe/components/Transactions/TxList/TxQueueRow.tsx index 59d46858e2..64e4b250bf 100644 --- a/src/routes/safe/components/Transactions/TxList/TxQueueRow.tsx +++ b/src/routes/safe/components/Transactions/TxList/TxQueueRow.tsx @@ -13,17 +13,18 @@ type TxQueueRowProps = { } export const TxQueueRow = ({ isGrouped = false, transaction }: TxQueueRowProps): ReactElement => { - const { activeHover } = useContext(TxHoverContext) + const { activeHover, pendingTx } = useContext(TxHoverContext) const [tx, setTx] = useState(transaction) + const willBeReplaced = tx.txStatus === LocalTransactionStatus.WILL_BE_REPLACED ? ' will-be-replaced' : '' useEffect(() => { - if (activeHover && activeHover !== transaction.id) { + if ((activeHover && activeHover !== transaction.id) || (pendingTx && pendingTx !== transaction.id)) { setTx((currTx) => ({ ...currTx, txStatus: LocalTransactionStatus.WILL_BE_REPLACED })) return } setTx(transaction) - }, [activeHover, transaction]) + }, [activeHover, transaction, pendingTx]) return ( diff --git a/src/routes/safe/components/Transactions/TxList/hooks/useActionButtonsHandlers.ts b/src/routes/safe/components/Transactions/TxList/hooks/useActionButtonsHandlers.ts index 8ac022c350..50284cee22 100644 --- a/src/routes/safe/components/Transactions/TxList/hooks/useActionButtonsHandlers.ts +++ b/src/routes/safe/components/Transactions/TxList/hooks/useActionButtonsHandlers.ts @@ -1,5 +1,5 @@ import { MultisigExecutionInfo } from '@gnosis.pm/safe-react-gateway-sdk' -import { MouseEvent as ReactMouseEvent, useCallback, useContext, useRef } from 'react' +import { MouseEvent as ReactMouseEvent, useCallback, useContext, useEffect, useRef } from 'react' import { useDispatch, useSelector } from 'react-redux' import { @@ -37,6 +37,14 @@ export const useActionButtonsHandlers = (transaction: Transaction): ActionButton const txStatus = useTxStatus(transaction) const isPending = txStatus === LocalTransactionStatus.PENDING + useEffect(() => { + if (isPending) { + hoverContext.current.setPendingTx(transaction.id) + } else { + hoverContext.current.setPendingTx() + } + }, [isPending, transaction.id]) + const handleConfirmButtonClick = useCallback( (event: ReactMouseEvent) => { event.stopPropagation() diff --git a/src/routes/safe/components/Transactions/TxList/styled.tsx b/src/routes/safe/components/Transactions/TxList/styled.tsx index e5c93dc268..6445534541 100644 --- a/src/routes/safe/components/Transactions/TxList/styled.tsx +++ b/src/routes/safe/components/Transactions/TxList/styled.tsx @@ -92,7 +92,7 @@ export const StyledTransactions = styled.div` &:last-of-type { div { - row-gap: 0px; + row-gap: 0; } } } @@ -144,10 +144,14 @@ const gridColumns = { } const willBeReplaced = css` + .will-be-replaced { + pointer-events: none; + filter: grayscale(1) opacity(0.8) !important; + } .will-be-replaced * { + pointer-events: none; color: gray !important; text-decoration: line-through !important; - filter: grayscale(1) opacity(0.8) !important; } ` @@ -431,7 +435,7 @@ export const OwnerList = styled.ul` margin: 5px; } - span::first-of-type { + span:first-of-type { color: #008c73; font-weight: bold; } From 9510e440bef9bf3c8fdbb8e61fd3d6b9d5206c65 Mon Sep 17 00:00:00 2001 From: Usame Algan Date: Wed, 2 Feb 2022 16:30:30 +0100 Subject: [PATCH 3/6] fix: Use pending txs from redux for check --- .../pendingTransactionsMiddleware.ts | 4 ++-- .../store/selectors/pendingTransactions.ts | 24 +++++++++++++++---- .../Transactions/TxList/TxHoverProvider.tsx | 11 +-------- .../Transactions/TxList/TxQueueRow.tsx | 14 ++++++++--- .../TxList/hooks/useActionButtonsHandlers.ts | 10 +------- 5 files changed, 34 insertions(+), 29 deletions(-) diff --git a/src/logic/safe/store/middleware/pendingTransactionsMiddleware.ts b/src/logic/safe/store/middleware/pendingTransactionsMiddleware.ts index f050d23307..f070d1456b 100644 --- a/src/logic/safe/store/middleware/pendingTransactionsMiddleware.ts +++ b/src/logic/safe/store/middleware/pendingTransactionsMiddleware.ts @@ -9,7 +9,7 @@ import { } from 'src/logic/safe/store/actions/pendingTransactions' import { PENDING_TRANSACTIONS_ID, PendingTransactionPayload } from 'src/logic/safe/store/reducer/pendingTransactions' import { Dispatch } from 'src/logic/safe/store/actions/types' -import { allPendingTxs } from 'src/logic/safe/store/selectors/pendingTransactions' +import { allPendingTxIds } from 'src/logic/safe/store/selectors/pendingTransactions' // Share updated statuses between tabs/windows // Test env and Safari don't support BroadcastChannel @@ -57,7 +57,7 @@ export const pendingTransactionsMiddleware = } const state = getState() - session.setItem(PENDING_TRANSACTIONS_ID, allPendingTxs(state)) + session.setItem(PENDING_TRANSACTIONS_ID, allPendingTxIds(state)) break } default: diff --git a/src/logic/safe/store/selectors/pendingTransactions.ts b/src/logic/safe/store/selectors/pendingTransactions.ts index 47275a314f..95124c28c2 100644 --- a/src/logic/safe/store/selectors/pendingTransactions.ts +++ b/src/logic/safe/store/selectors/pendingTransactions.ts @@ -6,21 +6,35 @@ import { LocalTransactionStatus, Transaction } from 'src/logic/safe/store/models import { PendingTransactionsState, PENDING_TRANSACTIONS_ID } from 'src/logic/safe/store/reducer/pendingTransactions' import { currentChainId } from 'src/logic/config/store/selectors' import { ChainId } from 'src/config/chain' +import { getTransactionByAttribute } from 'src/logic/safe/store/selectors/gatewayTransactions' -export const allPendingTxs = (state: AppReduxState): PendingTransactionsState => { +export const allPendingTxIds = (state: AppReduxState): PendingTransactionsState => { return state[PENDING_TRANSACTIONS_ID] } -const pendingTxsByChain = createSelector( - allPendingTxs, +export const pendingTxIdsByChain = createSelector( + allPendingTxIds, currentChainId, (statuses, chainId): PendingTransactionsState[ChainId] => { return statuses[chainId] }, ) +export const pendingTxByChain = createSelector( + (state: AppReduxState) => state, + pendingTxIdsByChain, + (state: AppReduxState, pendingTxIds: PendingTransactionsState[ChainId]): Transaction | undefined => { + if (!pendingTxIds) { + return + } + + const pendingTxId = Object.keys(pendingTxIds)[0] + return getTransactionByAttribute(state, { attributeValue: pendingTxId, attributeName: 'id' }) + }, +) + export const isTxPending = createSelector( - pendingTxsByChain, + pendingTxIdsByChain, (_: AppReduxState, id: string) => id, (pendingTxs: PendingTransactionsState[ChainId], id: string): boolean => { return pendingTxs ? !!pendingTxs?.[id] : false @@ -28,7 +42,7 @@ export const isTxPending = createSelector( ) export const selectTxStatus = createSelector( - pendingTxsByChain, + pendingTxIdsByChain, (_: AppReduxState, tx: Transaction) => tx, (pendingTxs: PendingTransactionsState[ChainId], tx: Transaction): TransactionStatus => { return !!pendingTxs?.[tx.id] ? LocalTransactionStatus.PENDING : tx.txStatus diff --git a/src/routes/safe/components/Transactions/TxList/TxHoverProvider.tsx b/src/routes/safe/components/Transactions/TxList/TxHoverProvider.tsx index 1afcd68ccf..6051467839 100644 --- a/src/routes/safe/components/Transactions/TxList/TxHoverProvider.tsx +++ b/src/routes/safe/components/Transactions/TxList/TxHoverProvider.tsx @@ -3,22 +3,13 @@ import { createContext, ReactElement, ReactNode, useState } from 'react' export const TxHoverContext = createContext<{ activeHover?: string setActiveHover: (activeHover?: string) => void - pendingTx?: string - setPendingTx: (hasPendingTx?: string) => void }>({ activeHover: undefined, setActiveHover: () => {}, - pendingTx: undefined, - setPendingTx: () => {}, }) export const TxHoverProvider = ({ children }: { children: ReactNode }): ReactElement => { const [activeHover, setActiveHover] = useState() - const [pendingTx, setPendingTx] = useState() - return ( - - {children} - - ) + return {children} } diff --git a/src/routes/safe/components/Transactions/TxList/TxQueueRow.tsx b/src/routes/safe/components/Transactions/TxList/TxQueueRow.tsx index 64e4b250bf..5e5abfde90 100644 --- a/src/routes/safe/components/Transactions/TxList/TxQueueRow.tsx +++ b/src/routes/safe/components/Transactions/TxList/TxQueueRow.tsx @@ -6,6 +6,10 @@ import { NoPaddingAccordion, StyledAccordionSummary } from './styled' import { TxDetails } from './TxDetails' import { TxHoverContext } from './TxHoverProvider' import { TxQueueCollapsed } from './TxQueueCollapsed' +import { useSelector } from 'react-redux' +import { AppReduxState } from 'src/store' +import { isTxPending, pendingTxByChain } from 'src/logic/safe/store/selectors/pendingTransactions' +import { MultisigExecutionInfo } from '@gnosis.pm/safe-react-gateway-sdk' type TxQueueRowProps = { isGrouped?: boolean @@ -13,18 +17,22 @@ type TxQueueRowProps = { } export const TxQueueRow = ({ isGrouped = false, transaction }: TxQueueRowProps): ReactElement => { - const { activeHover, pendingTx } = useContext(TxHoverContext) + const { activeHover } = useContext(TxHoverContext) const [tx, setTx] = useState(transaction) const willBeReplaced = tx.txStatus === LocalTransactionStatus.WILL_BE_REPLACED ? ' will-be-replaced' : '' + const isPending = useSelector((state: AppReduxState) => isTxPending(state, transaction.id)) + const pendingTx = useSelector(pendingTxByChain) + const pendingTxNonce = (pendingTx?.executionInfo as MultisigExecutionInfo)?.nonce + const nonce = (transaction.executionInfo as MultisigExecutionInfo)?.nonce useEffect(() => { - if ((activeHover && activeHover !== transaction.id) || (pendingTx && pendingTx !== transaction.id)) { + if ((activeHover && activeHover !== transaction.id) || (!isPending && nonce === pendingTxNonce)) { setTx((currTx) => ({ ...currTx, txStatus: LocalTransactionStatus.WILL_BE_REPLACED })) return } setTx(transaction) - }, [activeHover, transaction, pendingTx]) + }, [activeHover, transaction, isPending, nonce, pendingTxNonce]) return ( { - if (isPending) { - hoverContext.current.setPendingTx(transaction.id) - } else { - hoverContext.current.setPendingTx() - } - }, [isPending, transaction.id]) - const handleConfirmButtonClick = useCallback( (event: ReactMouseEvent) => { event.stopPropagation() From 532a1121edf4018999e478485d8fe4fc5562023c Mon Sep 17 00:00:00 2001 From: Usame Algan Date: Wed, 2 Feb 2022 17:33:58 +0100 Subject: [PATCH 4/6] fix: Use typeguard for tx nonce check --- .../components/Transactions/TxList/TxQueueRow.tsx | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/routes/safe/components/Transactions/TxList/TxQueueRow.tsx b/src/routes/safe/components/Transactions/TxList/TxQueueRow.tsx index 5e5abfde90..39c4a117d2 100644 --- a/src/routes/safe/components/Transactions/TxList/TxQueueRow.tsx +++ b/src/routes/safe/components/Transactions/TxList/TxQueueRow.tsx @@ -1,7 +1,11 @@ import { AccordionDetails } from '@gnosis.pm/safe-react-components' import { ReactElement, useContext, useEffect, useState } from 'react' -import { LocalTransactionStatus, Transaction } from 'src/logic/safe/store/models/types/gateway.d' +import { + isMultisigExecutionInfo, + LocalTransactionStatus, + Transaction, +} from 'src/logic/safe/store/models/types/gateway.d' import { NoPaddingAccordion, StyledAccordionSummary } from './styled' import { TxDetails } from './TxDetails' import { TxHoverContext } from './TxHoverProvider' @@ -9,7 +13,6 @@ import { TxQueueCollapsed } from './TxQueueCollapsed' import { useSelector } from 'react-redux' import { AppReduxState } from 'src/store' import { isTxPending, pendingTxByChain } from 'src/logic/safe/store/selectors/pendingTransactions' -import { MultisigExecutionInfo } from '@gnosis.pm/safe-react-gateway-sdk' type TxQueueRowProps = { isGrouped?: boolean @@ -22,8 +25,8 @@ export const TxQueueRow = ({ isGrouped = false, transaction }: TxQueueRowProps): const willBeReplaced = tx.txStatus === LocalTransactionStatus.WILL_BE_REPLACED ? ' will-be-replaced' : '' const isPending = useSelector((state: AppReduxState) => isTxPending(state, transaction.id)) const pendingTx = useSelector(pendingTxByChain) - const pendingTxNonce = (pendingTx?.executionInfo as MultisigExecutionInfo)?.nonce - const nonce = (transaction.executionInfo as MultisigExecutionInfo)?.nonce + const pendingTxNonce = isMultisigExecutionInfo(pendingTx?.executionInfo) ? pendingTx?.executionInfo.nonce : undefined + const nonce = isMultisigExecutionInfo(transaction.executionInfo) ? transaction.executionInfo.nonce : undefined useEffect(() => { if ((activeHover && activeHover !== transaction.id) || (!isPending && nonce === pendingTxNonce)) { From ab4be462f273c141b175bda8b308331be289e3b0 Mon Sep 17 00:00:00 2001 From: Usame Algan Date: Thu, 3 Feb 2022 09:20:25 +0100 Subject: [PATCH 5/6] fix: Extend condition --- src/routes/safe/components/Transactions/TxList/TxQueueRow.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/routes/safe/components/Transactions/TxList/TxQueueRow.tsx b/src/routes/safe/components/Transactions/TxList/TxQueueRow.tsx index 39c4a117d2..bb6d916b8e 100644 --- a/src/routes/safe/components/Transactions/TxList/TxQueueRow.tsx +++ b/src/routes/safe/components/Transactions/TxList/TxQueueRow.tsx @@ -27,9 +27,10 @@ export const TxQueueRow = ({ isGrouped = false, transaction }: TxQueueRowProps): const pendingTx = useSelector(pendingTxByChain) const pendingTxNonce = isMultisigExecutionInfo(pendingTx?.executionInfo) ? pendingTx?.executionInfo.nonce : undefined const nonce = isMultisigExecutionInfo(transaction.executionInfo) ? transaction.executionInfo.nonce : undefined + const isReplacementTxPending = !isPending && nonce && pendingTxNonce && nonce === pendingTxNonce useEffect(() => { - if ((activeHover && activeHover !== transaction.id) || (!isPending && nonce === pendingTxNonce)) { + if ((activeHover && activeHover !== transaction.id) || isReplacementTxPending) { setTx((currTx) => ({ ...currTx, txStatus: LocalTransactionStatus.WILL_BE_REPLACED })) return } From 1fe2bcea3fb0c17363bee352519c0b6e69919868 Mon Sep 17 00:00:00 2001 From: Usame Algan Date: Thu, 3 Feb 2022 13:32:12 +0100 Subject: [PATCH 6/6] fix: Include missing dependency in useEffect --- src/routes/safe/components/Transactions/TxList/TxQueueRow.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/routes/safe/components/Transactions/TxList/TxQueueRow.tsx b/src/routes/safe/components/Transactions/TxList/TxQueueRow.tsx index bb6d916b8e..16a6a21d30 100644 --- a/src/routes/safe/components/Transactions/TxList/TxQueueRow.tsx +++ b/src/routes/safe/components/Transactions/TxList/TxQueueRow.tsx @@ -36,7 +36,7 @@ export const TxQueueRow = ({ isGrouped = false, transaction }: TxQueueRowProps): } setTx(transaction) - }, [activeHover, transaction, isPending, nonce, pendingTxNonce]) + }, [activeHover, transaction, isReplacementTxPending]) return (