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

StripeDialog: Put the money icon in the right place depending on the locale #1663

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

tintou
Copy link
Member

@tintou tintou commented Sep 9, 2021

In French we would write "5 $" and never "$ 5".

…locale.

In French we would write "5 $" and never "$ 5".
@danirabbit
Copy link
Member

I'm conflicted on this because we always put icons which describe the purpose of the entry in the primary position and it really messes up the layout to move it. It's more of a modifier to the entry itself than to number inside of the entry

@tintou
Copy link
Member Author

tintou commented Sep 15, 2021

I feel like here the exception is that the icon is actually a label (using the same weight and font than a normal label) so it really looks weird for me as a french speaker.

@danirabbit
Copy link
Member

danirabbit commented Sep 15, 2021

It's a different weight and font and spacing also :p

Screenshot from 2021-09-15 10 52 16

Copy link
Contributor

@cassidyjames cassidyjames left a comment

Choose a reason for hiding this comment

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

I'm kind of indifferent on the design; if the French person expects it on the right, I'm inclined to listen even if it looks funky to us. But I'm unable to confirm this actually uses the locale setting; I've set my locale to use French formats and rebooted, but the dialog in this branch still has it on the left.

@tintou
Copy link
Member Author

tintou commented Sep 16, 2021

@cassidyjames we're not using LC_MONETARY and I really don't know any software that actually use that, we just rely on the language

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.

None yet

5 participants