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 #6252 ([mobile UX] Eur symbol displayed below the price of the item) #6383

Merged

Conversation

andrewpbrett
Copy link
Contributor

What? Why?

Closes #6252

The formatting string for the currency display put an extra space in between the amount and the currency symbol, which resulted in it wrapping onto a second line.

What should we test?

See the issue; you can change the instance config to put the currency symbol after the amount and a price with four digits should remain all on one line.

Release notes

Fixed a visual bug where the price for an item would split across two lines on mobile devices.

Changelog Category: User facing changes

Dependencies

Documentation updates

@andrewpbrett andrewpbrett changed the title fix #6252 fix #6252 ([mobile UX] Eur symbol displayed below the price of the item) Nov 16, 2020
@andrewpbrett andrewpbrett self-assigned this Nov 16, 2020
@filipefurtad0 filipefurtad0 self-assigned this Nov 21, 2020
@filipefurtad0 filipefurtad0 added pr-staged-es pr-staged-uk staging.openfoodnetwork.org.uk pr-staged-fr staging.coopcircuits.fr and removed pr-staged-es pr-staged-uk staging.openfoodnetwork.org.uk labels Nov 21, 2020
@filipefurtad0
Copy link
Contributor

Hey @andrewpbrett ,

I checked this bug again in staging. I've set different price "lengths" in order to mimic other weaker currencies.

  • On mobile (Android), it looks like this:

Chrome
image

Firefox
image

  • Turns out, this bug is observable as well in desktop as well. If one inserts an item into the cart, a second row is shown, see below:

I took a series of pics, for different screen sizes - see top right corner of the pic (Firefox 82, Ubuntu 20.04):

image

image

image

image

image

image

image

I staged this PR, but somehow can't quite observe any changes:

Chrome
image

Firefox
image

This video shows the behavior after staging this PR. Below is Firefox 82, and below Chrome 86:

currency_down_before_

Perhaps this PR could still be improved and extended to other number sizes? Happy to get feedback Andy, in case there is a testcase I'm missing 👍

@filipefurtad0 filipefurtad0 added feedback-needed and removed pr-staged-fr staging.coopcircuits.fr labels Nov 23, 2020
@Matt-Yorkley
Copy link
Contributor

Matt-Yorkley commented Nov 23, 2020

Maybe we would be better off solving this with CSS? I think there's a few ways to control line-wrapping behavior with CSS rules.

@filipefurtad0
Copy link
Contributor

Thanks for feedback on this one @Matt-Yorkley ,
Moving back to In Dev for now 👍

@sauloperez
Copy link
Contributor

I could take a look myself once I'm done with #6366. You guys are pretty busy with many other things and I think this fits in my rather limited OFN hours per week.

@andrewpbrett
Copy link
Contributor Author

I took another look and found a quick solution - sorry @sauloperez :)

I tested on my iPhone SE (the original, so 4" screen) and it looks like this should work for up to about 7 digits (depending slightly on what those digits are). Anything larger than that can accommodate even more digits before running over into the add to cart button.

@RachL
Copy link
Contributor

RachL commented Nov 30, 2020

@andrewpbrett FYI we are using browserstack to test on various devices and OS: you should have access to the credentials in bitwarden, let me know if it does not work 👍 1:

@filipefurtad0
Copy link
Contributor

Hey @andrewpbrett ,

checked the following devices, after staging this PR:

Android 10 Samsung S10, Chrome

image

Android 10 Samsung S20, Firefox

image

IOS 14 IPhone 12 - Safari

image

Desktop Ubuntu 20.04 - Firefox 83 (responsiveness, identical to Chrome 86)

_euro_resize_after

I also did some regression tests, by setting the Euro sign before the price. Interestingly, I couldn't set the Euro symbol, to appear before the price on mobile - worked in desktop though and it looked good as well. Ready to go!

@andrewpbrett andrewpbrett merged commit 65f6f1f into openfoodfoundation:master Dec 2, 2020
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.

[mobile UX] Eur symbol displayed below the price of the item
5 participants