Money Data Types

[ghislaineguerin]

The specs related to issue #239 are ready for review. Please take a look at the attached document and follow the review guidelines.

Document Link

https://wiki.mathesar.org/en/design/specs/money-data-type

@mathemancer @kgodey @pavish, your feedback is required.
The deadline for feedback is set to a week from now, the first review round will be completed by Monday 9th. Please plan your review so that at least one round of changes can be made and reviewed if needed.

[kgodey]

@ghislaineguerin It looks good to me. It's pretty similar to numbers, so I don't have much to add.

My only feedback is:

• It's hard to parse the header describing multiple groupings, there's a lot going on, it's hard to figure out what's a column name, what's a range, and what's the count of records.
• I'm not sure what this ">" is doing here
  

[ghislaineguerin]

@kgodey

It's hard to parse the header describing multiple groupings, there's a lot going on, it's hard to figure out what's a column name, what's a range, and what's the count of records.

I agree, I've added spacing and some styling changes to better separate the content. However, this might have its limitations once we add more group columns. Usually, this is resolved by truncating the grouping list, or by adding a sub-header for the additional groups. I've posted examples for both below

I'm not sure what this ">" is doing here

If the range starts right after 10,000.00, we'd have to define what that next initial number is, for example, 10,000.01 which wouldn't be precise, so the expression was meant to show that anything above the last range end value would be included. I'm open to suggestions on how we might best communicate that.

[mathemancer]

I caught that, but 50/50 isn't a great record. I wish the concept of closed/open interval notation was more common after school. It would be useful in lots of contexts (such as this one)

[kgodey]

I like the sub-headers for additional groups, it's very clear.

As for the intervals, that makes sense but I think the current representation might be hard to parse. Here are alternatives I can think of:

• We can omit it entirely. If people see one group as "0-4" and another one as "4-8", they'll get it (although they would not know which group a "4" value would go into and that would annoy me personally, but I'm not sure that matters to most people).
• We could go with something like "4+ to 8", which might be easier to parse.
• We could use actual closed/open interval notation, so it would be (0-4] and (4-8], I think. I don't know if this would make sense to users, it would to me, but I have a math degree so I'm probably not representative in this case. It would mean that we use standard notation for this kind of thing and it should be easy enough to ignore the brackets if you don't know what they are.

[ghislaineguerin]

@kgodey I like the idea of using the 'easier to parse' representation. I have updated the prototype based on that.

[mathemancer]

Overall, I think this looks pretty good. I have a couple of questions:

1. Are we planning to pre-populate the field for decimal precision, or just always assume 2 places unless otherwise specified?
2. The commas aren't consistent between systems (and sometimes within systems: think lakh, for example). Are we required to support moving those around, or always using the 3 digits then comma system?
3. Considering viewing the DB via a different client as part of the UX, should the commas and currency symbols be supported there?

[ghislaineguerin]

@kgodey what if we had something like this

[kgodey]

I think we should provide a list of locales instead of three different options to make things simpler. JavaScript already supports this, see: https://www.w3schools.com/jsref/tryit.asp?filename=tryjsref_tolocalestring_num_all

[pavish]

I like the idea of listing locales. I think we can have a dropdown with a set of locales, along with a preview below it. When the locale changes, the preview would get updated.

For the locales themselves, we will need to cherry-pick the locales we want to show. It would also be better if we researched how this is implemented in other products.

[ghislaineguerin]

@kgodey I have added a list of locales, I'm wondering if we want to merge the currency as part of it? Please take a look and let me know

[kgodey]

@ghislaineguerin I think it would be good to pre-select the locale based on the currency, but allow users to change it. e.g. an American user dealing with indian rupees might still want to use American style commas to view the amounts. I think the way you have it looks good.

Some more feedback:
(1) Can we have currency as part of the default options. Currency is a basic option of the field, it's not an advanced option.
(2) I'm not sure that we need the toggle for using a different locale format, it just takes up more space. We could just show the locale field in advanced options always.
(3) Is there a friendlier way to show locale other than en_US? It may not be intuitive for non-technical users.
(4) Not related to money, but we might want to reuse the same locale options for dates and times.

[kgodey]

@ghislaineguerin One more piece of feedback that I just thought of: the icon for money is hard to distinguish, the dollar sign inside the circle is very small. Would it be possible to use something that didn't involve something inside a circle?

[ghislaineguerin]

@kgodey this has been updated

[kgodey]

Thanks, looks good.

[kgodey]

@ghislaineguerin Also, I think we should use tabular figures instead of proportional figures for all number types (but especially money). This will make reading numbers easier.

[ghislaineguerin]

@kgodey this has been updated

[kgodey]

Thanks @ghislaineguerin! Please also add a note in the spec about this so that @pavish knows to implement it.

[pavish]

We can use the default monospace font for tabular figures, as we are only using system fonts. This also means that it will change per browser/OS.

This should be okay, but as a heads up, it will not look the same in all environments.

[kgodey]

For now, yes. We might reevaluate fonts when we get to UI styling.

[pavish]

@ghislaineguerin The design looks good to me. I have a few concerns:

• The height of the dropdowns are large. It may be larger than the window height, when all the settings are visible. I think we should make use of more horizontal space for dropdowns instead of vertical space. This applies to all data types.
  • It might be more reasonable to open advanced options in a separate modal.
• The textboxes within filter and group can use the same representation as the column. eg., for money, we can show a dollar symbol and use the same formatting options that we use for the table cell.
• For filter and sort, since now we have it on a dropdown, we can have an explicit apply button. It would make it a lot easier for development, and while showing validation error messages.

[kgodey]

@pavish Please explain the design/development issues behind your suggestion for the apply button. I'm not sure what it means when you say it's a lot easier for development and to show error messages.

[ghislaineguerin]

@kgodey @pavish The specs have been updated to include the interactive data type options. This has fixes for:

• Height of dropdowns optimized
• Added number locales as a separate option from currency. The only question is if we will include all locales, or select a subset of the most popular formats. I've been looking at examples and the lists are very long, consider that what's on the prototype is a representation only.

[kgodey]

Thanks @ghislaineguerin, looks good. I think it's worth including all locales because we can automatically pick one based on the user's detected locale if needed (we can't do this if we only support a subset).

[ghislaineguerin]

@kgodey based on discussion with @pavish it seems to make more sense that we select locale based on currency rather than having a separate option for that. I have updated the prototype accordingly.

[kgodey]

@ghislaineguerin Please document the reasoning for this rather than just saying it's based on a discussion. Otherwise, we will not be able to keep track of why we made decisions a certain way in the future when we read through our discussions.

The prototype looks good but I'm not convinced that we should remove the option to show numbers in a different locale. If an American (or Indian) is looking at money in a currency that uses periods for number separators and commas for decimals, they may be confused. That being said, it's probably not worth worrying about for the MVP, so let's keep the design as-is for now. I wanted to document the feedback for future reference.

[pavish]

@kgodey The idea is that the display would depend only on the chosen currency.

eg., It wouldn't make sense to allow/select a locale with dot separators, for US dollars. Since our money format requires an explicit currency associated with it, I am not sure if allowing configurable locale would be of any help.

[kgodey]

It wouldn't make sense to allow/select a locale with dot separators, for US dollars.

I'm not convinced of this, someone who is used to seeing all numbers with dot separators may also want to see US dollars with dot separators. However, for the MVP, let's go with the simpler option of formatting numbers by currency locale (in accordance with our product principles).