Skip to content

Savings tooltip and column#9666

Closed
danjm wants to merge 18 commits intodevelopfrom
savings-tooltip-and-column
Closed

Savings tooltip and column#9666
danjm wants to merge 18 commits intodevelopfrom
savings-tooltip-and-column

Conversation

@danjm
Copy link
Copy Markdown
Contributor

@danjm danjm commented Oct 21, 2020 •
edited
Loading

This PR adds the final pieces of the latest redesign of the view quote screen: the savings tooltip and modifications the the 3rd column of the select quote popover. This compares to #9629

@danjm danjm changed the title Savings tooltip and column (WIP) Savings tooltip and column Oct 21, 2020
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Oct 21, 2020

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@danjm danjm force-pushed the alternative-savings-fix branch from 47c7324 to 4cbdce7 Compare October 22, 2020 07:05
@danjm danjm force-pushed the savings-tooltip-and-column branch from b58c0a3 to d63c1ec Compare October 22, 2020 08:47
@danjm danjm changed the base branch from alternative-savings-fix to update-fee-card-designs October 22, 2020 08:48
@danjm danjm changed the title (WIP) Savings tooltip and column Savings tooltip and column Oct 22, 2020
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Oct 22, 2020

Builds ready [c80126f]
Page Load Metrics (503 ± 68 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint35103572211
domContentLoaded32976750014268
load33076850314268
domInteractive32876750014268

@brad-decker brad-decker mentioned this pull request Oct 22, 2020
Merged
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Oct 22, 2020

Builds ready [b58db28]
Page Load Metrics (405 ± 48 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint29433642
domContentLoaded29760640310048
load29960840510048
domInteractive29760640310048

brad-decker
brad-decker suggested changes Oct 22, 2020
View reviewed changes
ui/app/pages/swaps/fee-card/index.scss
}

&__savings-tooltip-card {
width: 294.95068359375px;
Copy link
Copy Markdown
Contributor

@brad-decker brad-decker Oct 22, 2020

Choose a reason for hiding this comment

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

this is making the div bigger than its allowed screen size... I also don't think it's strictly necessary to make it look good. not a blocker, but these oddly specific pixel values are prone to breaking later.

@jakehaugen
Copy link
Copy Markdown

jakehaugen commented Oct 23, 2020 •
edited
Loading

Looking great. Gal and I discussed a few small tweaks:

  • Remove verbal explanation of savings.
  • Adjust formatting of line item view (no parent container with border, HR on bottom and top of "Estimated savings" line)
  • Add clarification of calculation below savings calculation (i.e. "Savings are calculated by comparing the best quote to the median quote.")
  • Change negative values to UI4 (dark gray) vs. red.
  • Show MetaMask fee as the token value (if possible, otherwise use the 0.875% -- looks like the % symbol is missing in the screen recording above).

Screen Shot 2020-10-23 at 10 29 08 AM

@jakehaugen
Copy link
Copy Markdown

jakehaugen commented Oct 23, 2020

We'll also need to cover the scenario when there are no savings. If there are no savings:

  • Replace the "Saving ..." text and piggy bank icon with "Using the best quote".
  • Do not show the savings tooltip.
  • In the view all quotes table, do not show the "Overall savings" column, just highlight the best quote with the "Best" tag.

Screen Shot 2020-10-23 at 11 13 28 AM

@danjm danjm force-pushed the savings-tooltip-and-column branch from 8d6a141 to 52e4a5d Compare October 26, 2020 13:47
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Oct 26, 2020

Builds ready [52e4a5d]
Page Load Metrics (382 ± 47 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint298839168
domContentLoaded2936093819747
load2946103829747
domInteractive2936093809747

@danjm
Copy link
Copy Markdown
Contributor Author

danjm commented Oct 26, 2020

The updates to the tooltip designs most recently been requested by @jakehaugen have been added to this PR:
Screenshot from 2020-10-26 11-14-06

@danjm
Copy link
Copy Markdown
Contributor Author

danjm commented Oct 26, 2020

The update to show the "Using best quote message" when there are no savings has been added to #9629

@brad-decker
Copy link
Copy Markdown
Contributor

brad-decker commented Oct 26, 2020

@danjm can you rebase this and i'll do another QA/review round

@danjm danjm force-pushed the savings-tooltip-and-column branch from 52e4a5d to 6847189 Compare October 26, 2020 19:31
@danjm danjm added this to the v8.1.next? milestone Nov 2, 2020
@danjm danjm mentioned this pull request Nov 5, 2020
Merged
@danjm danjm force-pushed the update-fee-card-designs branch from bf1ca7f to b14a14b Compare November 6, 2020 18:42
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Nov 9, 2020

Builds ready [6847189]
Page Load Metrics (399 ± 55 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint30393421
domContentLoaded30364839711555
load30464839911455
domInteractive30264739711555

@danjm danjm force-pushed the update-fee-card-designs branch 2 times, most recently from 5d6c285 to a7d49f2 Compare November 10, 2020 17:36
Base automatically changed from update-fee-card-designs to develop November 13, 2020 17:12
@danjm
Copy link
Copy Markdown
Contributor Author

danjm commented Dec 7, 2020

Closing in favour of #9905

@danjm danjm closed this Dec 7, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Dec 7, 2020
@DDDDDanica DDDDDanica deleted the savings-tooltip-and-column branch June 3, 2025 14:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@brad-decker brad-decker brad-decker requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

@brad-decker brad-decker

Labels

area-swaps

Projects

None yet

Milestone

v8.1.next?

Development

Successfully merging this pull request may close these issues.

5 participants

@danjm @metamaskbot @jakehaugen @brad-decker @jacobc-eth