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

Create pending transaction state #710

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

zerts
Copy link
Contributor

@zerts zerts commented Feb 4, 2025

No description provided.

Copy link

github-actions bot commented Feb 4, 2025

📦 build.zip [updated at Feb 18, 3:52:51 PM UTC]

@zerts zerts force-pushed the task/extension-show-tx-status-on-last-screen-of-native-flow-WLT-5816 branch from e80be16 to 946a303 Compare February 4, 2025 16:32
? transactionReceiptToActionStatus({ receipt: ethersv5Receipt })
: null;

return localStatus || nodeStatus || 'pending';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it ok to use pending as a fallback option?

@zerts zerts requested a review from everdimension February 5, 2025 13:25
@zerts zerts marked this pull request as ready for review February 5, 2025 13:25
import { sendRpcRequest } from 'src/shared/custom-rpc/rpc-request';
import { wait } from 'src/shared/wait';

export async function getTransactionReceipt({
Copy link
Member

Choose a reason for hiding this comment

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

TransactionPoller is already performing this work
I think we should be able to tap into it from the UI

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was thinking about this, but didn't get the easy solution. Will think again

Comment on lines +244 to +255
<UIText kind="headline/hero">{pendingTitle}</UIText>
<AnimatedDots />
</HStack>
<UIText
kind="headline/hero"
className={cn(
styles.title,
status !== 'confirmed' && styles.hidden
)}
>
{confirmedTitle}
</UIText>
Copy link
Member

Choose a reason for hiding this comment

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

I know it's minor but I don't like that text selection is messed up in this area because of overlapping text elements with zero opacity
Can we fix this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point!

Comment on lines 207 to 213
<PageColumn
style={{
width: 'clamp(320px, 100vw, 450px)',
marginInline: 'auto',
position: 'relative',
}}
>
Copy link
Member

Choose a reason for hiding this comment

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

What do you think about making this whole element appear with a quick fade-in? I think currently it feels a bit off that it appears kinda unexpectedly but then has fluid animations within

return (
<PageColumn>
<NavigationTitle urlBar="none" title="Send Success" />
Copy link
Member

Choose a reason for hiding this comment

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

Did this get lost?

@@ -43,10 +56,12 @@

.title.hidden {
opacity: 0;
user-select: none;
Copy link
Member

Choose a reason for hiding this comment

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

Does this work? I'd think it should be pointer-events: none instead

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought, user-select is the one that should be used here. As it prevents text from being selected
I can change to pointer-events too. Not sure there is a difference in this case

Copy link
Member

Choose a reason for hiding this comment

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

I meant that the visible text should still be selectable :)

@zerts zerts requested a review from everdimension February 18, 2025 15:51
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.

2 participants