Number Data Types

[ghislaineguerin]

The specs related to issue #238 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/number-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 Friday 29th. Please plan your review so that at least one round of changes can be made and reviewed if needed.

[mathemancer]

Overall, I think this makes sense, and I like how it both lets the user get pretty granular with their choices if they want, or ignore the details if desired.

I have a few comments and questions:

• I think using "Database Type NUMERIC" is confusing, since the associated "Number Format" types aren't using that DB type.
  • For the advanced options, is the intent for the "Number Format" to choose the underlying DB type? If so, I think we should just use "Database Type" to describe that menu, and get rid of the other instance of "Database Type".
  • On that front, I think the word "Format" might give the user a wrong idea of what will happen if they change these settings, since they go beyond formatting to actually changing the underlying data, sometimes in lossy ways (see below).
• If the user changes the type of a column already containing data, this will change both the view the user sees, but also the underlying data (e.g., changing from Float to Integer will probably truncate or round some values).
  • Is the flow of rectifying such data changes (or at least making the user aware of them) intended to be covered by this spec?

[kgodey]

Since @ghislaineguerin is on vacation, I'll answer what I can here.

I think using "Database Type NUMERIC" is confusing, since the associated "Number Format" types aren't using that DB type.

The "Database Type" section is supposed to show the user what the underlying database type is – we will show the correct type here, it won't always be NUMERIC. We want to show both the "friendly type" (Number) and the database type so that advanced users have all the information they need without complicating it too much for non-technical users.

As you know from #269, the friendly type "Number" maps to a whole bunch of database types.

For the advanced options, is the intent for the "Number Format" to choose the underlying DB type? If so, I think we should just use "Database Type" to describe that menu, and get rid of the other instance of "Database Type".

Yes, the intent is for the Number Format to choose the underlying DB type. I'm not sure what problem you are trying to solve by suggesting the menu rename. Could you describe the problem instead of suggesting a solution?

Also, we may have other options here in the future that don't relate to the database type (e.g. formatting options like where to put commas).

Is the flow of rectifying such data changes (or at least making the user aware of them) intended to be covered by this spec?

I think it makes sense to at least make the user aware of them.

[mathemancer]

Yes, the intent is for the Number Format to choose the underlying DB type. I'm not sure what problem you are trying to solve by suggesting the menu rename. Could you describe the problem instead of suggesting a solution?

"Format" doesn't suggest changing the data, but rather the way the data is displayed. However, changing these settings will change the data, and sometimes in lossy ways. This is a serious problem.

[kgodey]

"Format" doesn't suggest changing the data, but rather the way the data is displayed. However, changing these settings will change the data, and sometimes in lossy ways. This is a serious problem.

I agree that we need to use a different term and indicate potentially lossy changes better.

[ghislaineguerin]

@kgodey @mathemancer
Agree that 'format' needs to be changed as it doesn't convey the data conversion aspects of the configuration. Would changing the heading to something like 'Set Number Type' or 'Number Type Setting' be more clear? I'm trying to avoid adding a warning as the action isn't destructive and does what it's supposed to do.

[kgodey]

Looks good! My feedback:

(1) I agree with Brent's feedback that "Number Format" doesn't sufficiently convey that the underlying data and data type will be changed. I think using the word "format" should only be used for UI-level display settings (e.g. where to put commas)

(2) I think the fact that "range options" is a part of "group options" is not clear from the styling.

(3) The database type is currently sandwiched between the "Set type" and "Number format" boxes, but both of those settings affect the database type so the location seems ocnfusing. I think it should be at the bottom of the menu or somewhere less obtrusive.

(4) The range options need to be specified more. What happens when you un-check the "automatic" check box?

(5) This is not really related to numbers, but it's confusing that the icon for primary key is after the column name but the icon for text/number is before the column name.

(6) We also need to design the options for integers and floats as specified in the original design issue.

[ghislaineguerin]

(1) I agree with Brent's feedback that "Number Format" doesn't sufficiently convey that the underlying data and data type will be changed. I think using the word "format" should only be used for UI-level display settings (e.g. where to put commas)

Agreed, I proposed we call those 'Number type setting' or 'Set number type'

(2) I think the fact that "range options" is a part of "group options" is not clear from the styling.

I agree, will update the design.

(3) The database type is currently sandwiched between the "Set type" and "Number format" boxes, but both of those settings affect the database type so the location seems confusing. I think it should be at the bottom of the menu or somewhere less obtrusive.

I agree, will update the design.

(4) The range options need to be specified more. What happens when you un-check the "automatic" check box?

Agreed. When unchecked the value should remain but the user can set it manually.

(5) This is not really related to numbers, but it's confusing that the icon for the primary key is after the column name but the icon for text/number is before the column name.

Agreed, I've updated this.

(6) We also need to design the options for integers and floats as specified in the original design issue.

It was included on a separate Figma file in the content of the spec. I will be more specific next time when there are multiple documents.

[pavish]

I think it looks good. Brent and Kriti have covered most of the points I had in mind.

1. The filter options like between, require additional text fields to be displayed. The design for it is not present. The pane does not have enough space to handle it in a single line.
2. The alignment of values in the cells are not consistent. If we are to go by spreadsheet standards, numbers are always right-aligned. But the spec does not reflect that in all columns. We need to properly define this for all types.
3. The grouping options are confusing. How would it look when we apply more than one grouping? Also, what are the values for the selection box within range options?
4. Unrelated to the spec itself, we have not yet worked on hiding/showing the sidebar and the filter pane. After development of filter pane, it is clear that it is obtrusive, and takes considerable space. We need to rethink on the pane itself, and make it less obtrusive. We also have to ensure that there is a status showing a sort/group/filter is applied, when the pane is not open.

[ghislaineguerin]

@pavish You are correct about the pane width limit. I will address this with the new filter design.

The alignment of values in the cells are not consistent. If we are to go by spreadsheet standards, numbers are always right-aligned. But the spec does not reflect that in all columns. We need to properly define this for all types

I think right-aligning all numbers makes sense. I will do some quick research on this to see if it applies to all cases.

The grouping options are confusing. How would it look when we apply more than one grouping? Also, what are the values for the selection box within range options?
I agree with you on this. I will iterate on the group controls.

[kgodey]

@ghislaineguerin It looks like you haven't updated the spec yet, so I'll wait to re-review.

A general comment for all data types: it seems like changing the type is now in the "Advanced Options" menu. I don't think it should be there – originally, the advanced options menu was to show technical users more details. Putting everything type related in the advanced options menu will probably be overwhelming to non-technical users.

[ghislaineguerin]

@kgodey @mathemancer @pavish
I have updated the prototype based on feedback. Please review.

[kgodey]

@ghislaineguerin It looks great! Two minor points of feedback, both related to all data types:

(1) I think the database type should go in the "Advanced Options" section since I don't think it's relevant for a lot of users.
(2) I'm not sure where I mentioned this, but I still don't like the "Set <column_name> type" text. If the column name is really long, it will make the options panel really wide. I'm also not sure we need the column name anymore, since the menu is right under the column.

[ghislaineguerin]

@kgodey Thanks, I've included both changes.

[kgodey]

@ghislaineguerin Looks good!

[ghislaineguerin]

@kgodey @pavish
The specs document has been updated to include the global prototype for data type options. This includes the fix for:

• Moving percentage as a display option
• Wording for labels updated to improve consistency when setting type, format, display options.

[kgodey]

@ghislaineguerin Thanks, looks good. Can you update the spec text to clarify that the data type menu in the prototype under the "Prototype" header is not the correct one. Otherwise it might confuse implementers.