-
-
Notifications
You must be signed in to change notification settings - Fork 338
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
Implement display options for the custom Money type #1105
Comments
Can you give an example of how this should work? If the |
Installing os locale package seems to be the best option, there are very few alternate options like parsing our own from a dataset(https://lh.2xlibre.net/locales/) which would anyway end up creating a subset of the locale |
@silentninja Examples of these are given in the issue, e.g. "Whenever the "Format Type" setting is changed to a currency code, this should be set to the appropriate currency symbol for that code."
I agree with this, mostly. I think if the user is using a predefined format and then after that, they change another display option, we should allow them to do so but also change the "format type" to custom at that point. Does that make sense?
Do you mean the standard library |
Answering a comment on #1207, as this issue is a better place for that conversation
For now, we support inferring display options for |
Moved @seancolsen comment from #1207 as this issue is a better place to track the information
|
Copied from the other spot: (my comment)
If we come to an agreement on that, then we shouldn't be storing it as a display option at all.
And so on. Display options are not an appropriate place to hold data. |
Copied over from #1207
I agree with this, and I think we should probably make the currency unit a database option. Unrelated, I still see value in having a mathesar ui specific endpoint that can suggest display options as I mentioned in my previous comment (#1207 (comment)). But this is not at all a priority and can be taken up for future consideration. |
As far as storing that as a database option, there are some limitations, due to other constraints. We're avoiding certain type definition methods, because they require root access to the underlying machine and aren't available in many cloud environments (e.g., it's impossible to define a custom type that takes a type option in RDS). For context, here are the ways to do money as I see it:
Given the options listed, we decided to go with the |
@mathemancer I think we had a discussion around this and came to a conclusion to use display options. Our options are quite limited until we have the composite money type implemented from #417 |
Sure, but we agreed that it's a display option for a In fact, if we want to store which currency the column is as data, we'd want to store simply which currency it is, and then still leave how to display that given currency to other non-database layers. |
We did decide to treat currency as a display option; it is not data. I don't think we should be revisiting our money implementation right now, we've already had a lot of discussions about it and I think it's better to ship something imperfect and see what users think. Is there a problem with implementing this issue? I read through the thread and couldn't find one. |
I don't think we should do very heavy inference of currency symbol or other display options. I'd certainly like to avoid invalidating a column if it has |
We won't be invalidating a column, rather we fuzzy match the symbol fetched with the currency symbols we know and suggest it to the users. At most, it will be a wrong suggestion, which can be corrected during the preview. |
@silentninja is correct. Since we're showing it as a display option, the user can change the currency or formatting at any time. I don't see any issues with proceeding with this issue as specced. |
@seancolsen @pavish @kgodey Would it be better to set just the |
@silentninja No, because (unlike Numbers), our design (see here) allows for customization of all of those other details. |
Problem
We need to implement support for display options for the custom Money type.
Solution
The custom money type supports the following display options. We need to represent them in the API, validate them, and also set them automatically on inference of Money types.
Format Type
This stores the currency that the Money type should be displayed as. We will store a pre-set number of currencies that the frontend can pick from, or we will allow users to use "Custom" formatting that doesn't obey the rules of a specific currency.
Options
Symbol
Users can set a custom currency symbol here.
Options
Symbol Location
Location of the currency symbol.
Options
Decimal Symbol
Location of the decimal symbol.
Options
Digit Grouping
How the digits are grouped.
Options
[0]
)[3]
)[3, 0]
)[3, 2]
)Whenever the "Format Type" setting is changed to a currency code, this should be set to the appropriate grouping for that code.
Note: These representations are based on .NET docs.
Digit Grouping Symbol
What symbol is used for digit grouping
Options
Whenever the "Format Type" setting is changed to a currency code, this should be set to the appropriate grouping for that code.
Expected Outcomes
Additional context
The text was updated successfully, but these errors were encountered: