-
Notifications
You must be signed in to change notification settings - Fork 30
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Make percentage input consistent #43
Conversation
This makes the Percentage feature more consistent, such that the input is accepted as a value between 0 and 100. Previously the input was required to be a ratio (between 0 and 1), but it was then displayed as a percentage (effectively `"#{ratio / 100}%"`). With this change, the formatting of the input and later display match.
Thank you for submitting the PR, and for describing the issue in tompave/fun_with_flags#188. As I said there, I closed that issue in order to avoid duplication. We can discuss this here in the PR. |
I see where you're coming from but, to be honest, I think I prefer dealing with floats between Asking users to enter a
I admit that displaying "30%" in the UI might be confusing, but that's what the next bit of UI is for, the one that says: "raw value: So I think I'm going to reject this PR as it is now, sorry. However, I welcome a PR that tidies up the UI to make it easier to understand, provided that the input accepts the same |
In hindsight, I'd agree that cramming all that info into the But I stand by the idea that "percentage" is the right word there. I wouldn't change it to say "ratio" in the UI because the package API uses "percentage", and I wouldn't change the input to accept a Another point to keep in mind is that the current wording hasn't been a problem so far; at least I've not heard anyone complain. That doesn't necessarily mean that things cannot be improved (they always can), but at the very least it means that I must consider consistency and predictability in both the API and the web UI. Therefore, I'm still inclined to reject the PR, sorry. |
Fair enough! We'll continue using our fork. 👍 |
Very well, at least I'm glad you're unblocked! 👍 Thanks for using the (main? 😆) package! |
Thank you for all your work on FWF! |
This makes the Percentage feature more consistent, such that the input is accepted as a value between 0 and 100. Previously the input was required to be a ratio (between 0 and 1), but it was then displayed as a percentage (effectively
"#{ratio / 100}%"
). With this change, the formatting of the input and later display match.Resolves tompave/fun_with_flags#188