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.

Sign up for GitHub

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

Jump to bottom

Feature/high gas warn #2306

Merged
merged 28 commits into from
Mar 31, 2021
Merged

Feature/high gas warn #2306

merged 28 commits into from
Mar 31, 2021
+145 −49

Conversation

sethkfman
Copy link
Contributor

@sethkfman sethkfman commented Feb 26, 2021 •
edited
Loading

Description

This feature will warn users if they have entered a gas price that is higher than the recommend "Fast" price during the entire transaction flow.

Checklist

  • [NA] There is a related GitHub issue
  • Tests are included if applicable
  • Any added code is fully documented

@sethkfman sethkfman requested a review from a team as a code owner February 26, 2021 16:36
@sethkfman sethkfman changed the title Feature/high gas warn WIP - Feature/high gas warn Feb 26, 2021
@sethkfman sethkfman added needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) needs-qa Any New Features that needs a full manual QA prior to being added to a release. labels Mar 2, 2021
@sethkfman sethkfman changed the title WIP - Feature/high gas warn Feature/high gas warn Mar 2, 2021
@sethkfman sethkfman removed the needs-qa Any New Features that needs a full manual QA prior to being added to a release. label Mar 2, 2021
@sethkfman sethkfman added the needs-qa Any New Features that needs a full manual QA prior to being added to a release. label Mar 2, 2021
@ibrahimtaveras00 ibrahimtaveras00 added QA'd - Issues Found QA has been complete, however issues have been discovered that need to be addressed and removed needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) labels Mar 18, 2021
@omnat
Copy link
Contributor

omnat commented Mar 19, 2021

Are we showing the warning if the user chooses 1.5 times the 'Fast' gas setting?
Was discussed async here, but unsure if was visible enough.

@sethkfman
Copy link
Contributor Author

@omnat We are showing the warning if it is anything over the "Fast" value. Does it need to be changed to 1.5x?
CC: @jakehaugen & @ibrahimtaveras00

@sethkfman
moved the position of errors int he ApproveTransactionReview and redu…
6abaaa9 
…ced padding to make sure details are visisble
@sethkfman sethkfman requested a review from estebanmino March 22, 2021 20:57
estebanmino
estebanmino approved these changes Mar 23, 2021
View reviewed changes
Copy link
Contributor

@estebanmino estebanmino left a comment

Choose a reason for hiding this comment

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

lgtm!

@sethkfman sethkfman added the needs-qa Any New Features that needs a full manual QA prior to being added to a release. label Mar 23, 2021
@ibrahimtaveras00 ibrahimtaveras00 added QA in Progress QA has started on the feature. and removed needs-qa Any New Features that needs a full manual QA prior to being added to a release. QA in Progress QA has started on the feature. labels Mar 24, 2021
ibrahimtaveras00
ibrahimtaveras00 approved these changes Mar 25, 2021
View reviewed changes
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.

Fixes look good, QA Passed 👍🏽

@ibrahimtaveras00 ibrahimtaveras00 added QA Passed A successful QA run through has been done and removed QA'd - Issues Found QA has been complete, however issues have been discovered that need to be addressed labels Mar 25, 2021
@omnat omnat added the design-review Any feature that needs feedback from the design team label Mar 29, 2021
@sethkfman sethkfman merged commit 33949f2 into develop Mar 31, 2021
@sethkfman sethkfman deleted the feature/high-gas-warn branch March 31, 2021 15:47
rickycodes pushed a commit that referenced this pull request Jan 31, 2022
@sethkfman
Feature/high gas warn (#2306)
fa6334d 
* added high fee error warning to custom gas view

* updated text layout to wrap and center error text

* updated strings for high gas message

* adding high gas warning to dapp flow

* minor ui tweaks and dapp flow updates and reducer modification

* remove console log statements

* updated jest snapshots

* bug fix when swap merged into branch

* updated snapshots

* PR comments and feedback

* unit test snapshot updated

* added color to gasInput style

* updated snapshot and removed console log

* fixed spelling error in high_gas_price warning

* added high gas warning to swaps views

* added gas warning into ApproveTransactionReview from deeplink

* remove console logs

* moved the position of errors int he ApproveTransactionReview and reduced padding to make sure details are visisble

* PR feedback on location of warning

* set the gas warning threshold to 1.5x the fast price

* reset gas warning when preset slow to fast toggles are selected
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ibrahimtaveras00 ibrahimtaveras00 ibrahimtaveras00 approved these changes

@estebanmino estebanmino estebanmino approved these changes

Assignees

@rickycodes rickycodes

@andrepimenta andrepimenta

@estebanmino estebanmino

Labels
design-review Any feature that needs feedback from the design team next release QA Passed A successful QA run through has been done
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@sethkfman @omnat @estebanmino @ibrahimtaveras00 @rickycodes @andrepimenta