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

Improve copying price strings for bulk items #306

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

daviderickson
Copy link

When using the evaluate module and clicking on another
person's price listing to copy the price, if the price
is a fraction it currently copies as a decimal number, ie:
0.125. However if you list it as such, listing services will
round it to 0.5. Further, depending on the fraction, it
can be hard to reason about as a user on how many that
really represents per unit of currency.

This patch modifies the copied text to instead contain
the pervasive fractional numerator/denominator formatting for
any price that has a denominator greater than 1.

Description

Replication Steps

  1. ctl-D on a bulk currency item
  2. Find a listing that is a fraction
  3. Click the listing
  4. You should now get pricing text with a fraction ie X/Y rather than .025

Screenshots/Video

How Has This Been Tested?

  • ran npm run ng:lint
  • ran npm run format
  • ran npm run ng:test
  • added unit tests
  • manually tested

When using the evaluate module and clicking on another
person's price listing to copy the price, if the price
is a fraction it currently copies as a decimal number, ie:
0.125.  However if you list it as such, listing services will
round it to 0.5.  Further, depending on the fraction, it
can be hard to reason about as a user on how many that
really represents per unit of currency.

This patch modifies the copied text to instead contain
the pervasive fractional numerator/denominator formatting for
any price that has a denominator greater than 1.
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.

1 participant