Skip to content
This repository has been archived by the owner on Sep 20, 2023. It is now read-only.

Issue #36 : add approx price #188

Merged
merged 5 commits into from
May 18, 2018
Merged

Conversation

inkredabull
Copy link
Contributor

@inkredabull inkredabull commented May 17, 2018

Checklist:

  • No Tests
  • No README change

Description:

  • opted for least-effort inclusion of approximate price with Cryptonator API
  • no tests; maybe someday?

screen shot 2018-05-16 at 5 50 29 pm

Ref

@DanielVF
Copy link
Member

Cool!

I'm wondering if it might look better formatted as "Approximately $6,041.79 USD". The parenthesis, tilde, and three digit cents add a lot of visual noise.

@inkredabull
Copy link
Contributor Author

I agree with you in principle @DanielVF but what I thought would be a slam-dunk for a "good first issue" had me sink a bunch of time in the last three weeks around rebasing, conflict resolution, infra changes, and testing.

On the flip side, ABL ("Always Be Learning") ;-)

Didn't realize the project was changing so quickly when I joined; If there's consensus around the need for an improved UX beyond what it is now, can adjust but would love this to be merged as-is and will just figure on buffering more going forward.

@DanielVF
Copy link
Member

Micah, maybe Aure could take a pass at how we want to display alternate prices, both on the search page, and the listing detail pages.

I changed this to use two decimal places.

screen shot 2018-05-17 at 3 38 30 pm

@wanderingstan
Copy link
Collaborator

For reference, this is how MetaMask shows ETH/USD combinations:
image
image

Not that MetaMask is a great design inspiration, but it is what a lot of our users will see as the final price that they have to sign off on.

Following convention of space after `if`, as it's not a function.
@DanielVF
Copy link
Member

Here's another design:

screen shot 2018-05-18 at 12 47 39 pm

@auregimon
Copy link

I like the first option @DanielVF

ETH and USD to the left of it, same font size and USD in lighter gray. Try putting a pipe | in between the two if it looks weird.

@joshfraser joshfraser merged commit 471a8bd into develop May 18, 2018
@joshfraser joshfraser deleted the issue/36-add-approx-price-no-tests branch May 18, 2018 17:25
@DanielVF
Copy link
Member

DanielVF commented May 18, 2018

And here's the final design:

image

Note an important design change - we're now focusing on the fiat price first, with the ETH (or other crypto currency / token) taking a secondary visual weight.

@matthewliu
Copy link
Member

aure and i somehow missed this issue and the discussion around it. this is not going to fly. the current UI is very misleading as the USD will constantly fluctuate. unless we are using a stablecoin, we need to price in a primary denomination that doesn't change values over time. the USD needs to have very clear "approximation" denoted.

@wanderingstan
Copy link
Collaborator

wanderingstan commented Aug 20, 2018

@matthewliu , am I correct in understanding that you would prefer (A) to have the eth/token price first, with approx dollar afterward (with dollar more clearly marked as appprox), or (B) take out dollars entirely, and potentially re-introduce dollars after more design thought?

@matthewliu
Copy link
Member

A. I like the dollars but having it as primary is misleading because it would be changing minute-by-minute.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants