Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: improve disable condition and tooltip tile on TxSummary buttons #1969

Merged
merged 8 commits into from
May 12, 2023
9 changes: 7 additions & 2 deletions src/components/transactions/ExecuteTxButton/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import Track from '@/components/common/Track'
import { TX_LIST_EVENTS } from '@/services/analytics/events/txList'
import { ReplaceTxHoverContext } from '../GroupedTxListItems/ReplaceTxHoverProvider'
import CheckWallet from '@/components/common/CheckWallet'
import { useSafeSDK } from '@/hooks/coreSDK/safeCoreSDK'
import { getTxButtonTooltip } from '@/components/transactions/utils'

const ExecuteTxButton = ({
txSummary,
Expand All @@ -26,9 +28,12 @@ const ExecuteTxButton = ({
const txNonce = isMultisigExecutionInfo(txSummary.executionInfo) ? txSummary.executionInfo.nonce : undefined
const isPending = useIsPending(txSummary.id)
const { setSelectedTxId } = useContext(ReplaceTxHoverContext)
const safeSDK = useSafeSDK()

const isNext = txNonce !== undefined && txNonce === safe.nonce
const isDisabled = !isNext || isPending
const isDisabled = !isNext || isPending || !safeSDK

const tooltipTitle = getTxButtonTooltip('Execute', { isNext, nonce: safe.nonce, isPending, hasSafeSDK: !!safeSDK })

const onClick = (e: SyntheticEvent) => {
e.stopPropagation()
Expand All @@ -49,7 +54,7 @@ const ExecuteTxButton = ({
{(isOk) => (
<Track {...TX_LIST_EVENTS.EXECUTE}>
{compact ? (
<Tooltip title="Execute" arrow placement="top">
<Tooltip title={tooltipTitle} arrow placement="top">
<span>
<IconButton
onClick={onClick}
Expand Down
10 changes: 8 additions & 2 deletions src/components/transactions/RejectTxButton/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import ErrorIcon from '@/public/images/notifications/error.svg'
import Track from '@/components/common/Track'
import { TX_LIST_EVENTS } from '@/services/analytics/events/txList'
import CheckWallet from '@/components/common/CheckWallet'
import { useSafeSDK } from '@/hooks/coreSDK/safeCoreSDK'
import { getTxButtonTooltip } from '@/components/transactions/utils'

const NewTxModal = dynamic(() => import('@/components/tx/modals/NewTxModal'))

Expand All @@ -23,7 +25,11 @@ const RejectTxButton = ({
}): ReactElement | null => {
const [open, setOpen] = useState<boolean>(false)
const txNonce = isMultisigExecutionInfo(txSummary.executionInfo) ? txSummary.executionInfo.nonce : undefined
const isDisabled = useIsPending(txSummary.id)
const isPending = useIsPending(txSummary.id)
const safeSDK = useSafeSDK()
const isDisabled = isPending || !safeSDK

const tooltipTitle = getTxButtonTooltip('Replace', { isPending, hasSafeSDK: !!safeSDK })

const onClick = (e: SyntheticEvent) => {
e.stopPropagation()
Expand All @@ -36,7 +42,7 @@ const RejectTxButton = ({
{(isOk) => (
<Track {...TX_LIST_EVENTS.REJECT}>
{compact ? (
<Tooltip title="Replace" arrow placement="top">
<Tooltip title={tooltipTitle} arrow placement="top">
<span>
<IconButton onClick={onClick} color="error" size="small" disabled={!isOk || isDisabled}>
<SvgIcon component={ErrorIcon} inheritViewBox fontSize="small" />
Expand Down
11 changes: 8 additions & 3 deletions src/components/transactions/SignTxButton/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import CheckIcon from '@mui/icons-material/Check'
import Track from '@/components/common/Track'
import { TX_LIST_EVENTS } from '@/services/analytics/events/txList'
import CheckWallet from '@/components/common/CheckWallet'
import { useSafeSDK } from '@/hooks/coreSDK/safeCoreSDK'
import { getTxButtonTooltip } from '@/components/transactions/utils'

const SignTxButton = ({
txSummary,
Expand All @@ -24,21 +26,24 @@ const SignTxButton = ({
const wallet = useWallet()
const isSignable = isSignableBy(txSummary, wallet?.address || '')
const isPending = useIsPending(txSummary.id)
const safeSDK = useSafeSDK()

const isDisabled = !isSignable || isPending || !safeSDK

const tooltipTitle = getTxButtonTooltip('Confirm', { isPending, hasSafeSDK: !!safeSDK })

const onClick = (e: SyntheticEvent) => {
e.stopPropagation()
setOpen(true)
}

const isDisabled = !isSignable || isPending

return (
<>
<CheckWallet>
{(isOk) => (
<Track {...TX_LIST_EVENTS.CONFIRM}>
{compact ? (
<Tooltip title="Confirm" arrow placement="top">
<Tooltip title={tooltipTitle} arrow placement="top">
<span>
<IconButton onClick={onClick} color="primary" disabled={!isOk || isDisabled} size="small">
<CheckIcon fontSize="small" />
Expand Down
38 changes: 38 additions & 0 deletions src/components/transactions/__tests__/utils.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { getTxButtonTooltip } from '../utils'

describe('transactions utils', () => {
describe('getTooltipTitle', () => {
const disabledPropsBase = { isPending: false, hasSafeSDK: true }

it('should return the enabledTitle if no disabled conditions', () => {
const enabledTitle = 'Execute'

expect(getTxButtonTooltip(enabledTitle, disabledPropsBase)).toBe('Execute')
expect(true).toBe(true)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
expect(true).toBe(true)

})

it('should return the not isNext message', () => {
const enabledTitle = 'Confirm'
const nonce = 2
const disabledProps = { ...disabledPropsBase, isNext: false, nonce }

expect(getTxButtonTooltip(enabledTitle, disabledProps)).toBe('Transaction 2 must be executed first')
})

it('should return the "is tx pending" message', () => {
const enabledTitle = 'Execute'
const disabledProps = { ...disabledPropsBase, isPending: true }

expect(getTxButtonTooltip(enabledTitle, disabledProps)).toBe('Pending transaction must first succeed')
expect(true).toBe(true)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
expect(true).toBe(true)

})

it('should return the Safe SDK not initialized message', () => {
const enabledTitle = 'Execute'
const disabledProps = { ...disabledPropsBase, hasSafeSDK: false }

expect(getTxButtonTooltip(enabledTitle, disabledProps)).toBe('Waiting for the SDK to initialize')
expect(true).toBe(true)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
expect(true).toBe(true)

})
})
})
27 changes: 27 additions & 0 deletions src/components/transactions/utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
type DisabledPropsType =
| {
isNext: false
nonce: number
isPending: boolean
hasSafeSDK: boolean
}
| {
isNext?: boolean
nonce?: number
isPending: boolean
hasSafeSDK: boolean
}

type EnabledTitleType = 'Execute' | 'Replace' | 'Confirm'

export const getTxButtonTooltip = (enabledTitle: EnabledTitleType, disabledProps: DisabledPropsType) => {
const { isPending, hasSafeSDK, isNext, nonce } = disabledProps

return isNext === false
? `Transaction ${nonce} must be executed first`
: isPending
? 'Pending transaction must first succeed'
: !hasSafeSDK
? 'Waiting for the SDK to initialize'
: enabledTitle
}