Skip to content

fix: swapper warning acks#7948

Closed
0xApotheosis wants to merge 1 commit intoreleasefrom
fix-warning-acks
Closed

fix: swapper warning acks#7948
0xApotheosis wants to merge 1 commit intoreleasefrom
fix-warning-acks

Conversation

@0xApotheosis
Copy link
Copy Markdown
Member

@0xApotheosis 0xApotheosis commented Oct 16, 2024

Description

Fixes a regression from #7912 related to the acknowledgment component.

Updates the SharedTradeInput component to take an optional inputAcknowledgementParentComponent, which can be used to pass one or more parent acknowledgement wrappers.

⚠️ Still in draft as there is a weird height bug.

Issue (if applicable)

Closes #7946

Risk

Medium

What protocols, transaction types, wallets or contract interactions might be affected by this PR?

None

Testing

Trigger a warning acknowledgement and ensure it renders as it does in prod (not spanning the full width).

Engineering

☝️

Operations

  • 🏁 My feature is behind a flag and doesn't require operations testing (yet)

☝️

Screenshots (if applicable)

Screenshot 2024-10-16 at 14 28 36

@0xApotheosis 0xApotheosis changed the base branch from develop to release October 16, 2024 05:16
@0xApotheosis
Copy link
Copy Markdown
Member Author

0xApotheosis commented Oct 16, 2024

I think this guy is mounting and unmounting on input change:

<WithLazyMount
shouldUse={Boolean(receiveAddress) && disableThorNativeSmartContractReceive === false}
shouldForceManualAddressEntry={disableThorNativeSmartContractReceive}
component={RecipientAddress}
description={
disableThorTaprootReceiveAddress
? translate('trade.disableThorTaprootReceive')
: undefined
}
/>

@0xApotheosis
Copy link
Copy Markdown
Member Author

As far as I can tell onSubmit is too reactive in TradeInput.tsx, which is causing inputAcknowledgementParentComponent to re-render, which causes SharedTradeInput to re-render...

@0xApotheosis
Copy link
Copy Markdown
Member Author

Closing in favour of #7953

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Attention Warning in trade modal spans across whole page instead of just the modal

1 participant