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 estimated fee #5885

Merged
merged 5 commits into from
Jun 30, 2024
Merged

fix estimated fee #5885

merged 5 commits into from
Jun 30, 2024

Conversation

greg-schrammel
Copy link
Contributor

@greg-schrammel greg-schrammel commented Jun 22, 2024

Fixes APP-1556
Fixes APP-1637

What changed (plus any additional context for devs)

Screen recordings / screenshots

What to test

Copy link

linear bot commented Jun 24, 2024

Copy link
Member

@brunobar79 brunobar79 left a comment

Choose a reason for hiding this comment

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

Added a couple comments and there are conflicts. Once all those are fixed I'd love for @ibrahimtaveras00 to take a look to this one too

// which is great when refetching for the same chainId, but we don't want to keep the previous data
// when fetching for a different chainId
return {
data: data && data.chainId === chainId ? data.gasLimit : initialData?.gasLimit,
Copy link
Member

@jinchung jinchung Jun 26, 2024

Choose a reason for hiding this comment

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

leaving a note for the future: when we'd like to upgrade to react-query v5 where we can replace this with using placeholderData which gets access to previous values in v5 (currently it does not, which is why we need to do this particular pattern in order to get previous data while fetching if you stay switch assets while on the same chain)

Copy link
Member

@jinchung jinchung left a comment

Choose a reason for hiding this comment

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

🌮

@jinchung jinchung self-requested a review June 26, 2024 23:44
Copy link
Member

@jinchung jinchung left a comment

Choose a reason for hiding this comment

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

RESOLVED:
Blocking PR for now - Finding weirdness in the custom gas panel - confirming if this is also happening in latest develop

Separate issue that is also happening in develop: once you update custom gas and then come back to the panel, tip resets to 0

Uploading Simulator Screen Recording - iPhone 15 Pro - 2024-06-27 at 08.43.01.mp4…

Copy link
Contributor

@ibrahimtaveras00 ibrahimtaveras00 left a comment

Choose a reason for hiding this comment

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

@jinchung jinchung force-pushed the fix-estimated-fee branch from f23abdb to a2508e6 Compare June 29, 2024 02:32
Copy link

linear bot commented Jun 29, 2024

Copy link
Member

@brunobar79 brunobar79 left a comment

Choose a reason for hiding this comment

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

LGTM

@brunobar79 brunobar79 dismissed stale reviews from jinchung and ibrahimtaveras00 June 30, 2024 00:50

Issues resolved

@brunobar79 brunobar79 merged commit a543f85 into develop Jun 30, 2024
6 checks passed
@brunobar79 brunobar79 deleted the fix-estimated-fee branch June 30, 2024 00:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants