UI: Form fields should describe the accepted input (data type, format)
clstaudt opened this issue · 23 comments
Forms should more clearly describe which input is accepted and validate it.
The model classes should contain all the necessary information, including a field description. How can the UI code reuse them?
class Contract(SQLModel, table=True):
"""A contract defines the business conditions of a project"""
id: Optional[int] = Field(default=None, primary_key=True)
title: str = Field(description="Short description of the contract.")
client: Client = Relationship(
back_populates="contracts", sa_relationship_kwargs={"lazy": "subquery"}
)
signature_date: datetime.date = Field(
description="Date on which the contract was signed",
)
start_date: datetime.date = Field(
description="Date from which the contract is valid",
)
end_date: Optional[datetime.date] = Field(
description="Date until which the contract is valid",
)
# Contract n:1 Client
client_id: Optional[int] = Field(
default=None,
foreign_key="client.id",
)
rate: condecimal(decimal_places=2) = Field(
description="Rate of remuneration",
)
is_completed: bool = Field(
default=False, description="flag marking if contract has been completed"
)
currency: str # TODO: currency representation
VAT_rate: Decimal = Field(
description="VAT rate applied to the contractual rate.",
default=0.19, # TODO: configure by country?
)
unit: TimeUnit = Field(
description="Unit of time tracked. The rate applies to this unit.",
sa_column=sqlalchemy.Column(sqlalchemy.Enum(TimeUnit)),
default=TimeUnit.hour,
)
units_per_workday: int = Field(
description="How many units of time (e.g. hours) constitute a whole work day?",
default=8,
)
volume: Optional[int] = Field(
description="Number of units agreed on",
)
term_of_payment: Optional[int] = Field(
description="How many days after receipt of invoice this invoice is due.",
default=31,
)
billing_cycle: Cycle = Field(
sa_column=sqlalchemy.Column(sqlalchemy.Enum(Cycle)),
description="How often is an invoice sent?",
)
``@clstaudt updated, pending review
@flamesjames care to test and review?
@clstaudt yes, I would like this one as well! What would be the end result you are looking for? Documentation of all of the test cases and their results? Anything else required?
@flamesjames Here you can basically take the role of a new user of the app, enter user data and check if the forms give you proper hints on what to fill in. These should also be consistent with the data fields defined in tuttle/model.py.
Comment here and/or create a (draft) Pull Request from the dev branch to propose changes.
@clstaudt sounds good! Thank you.
@clstaudt would you like documentation provided for the tests or not necessary?
@flamesjames what do you mean by documentation?
@clstaudt sorry - for some projects, I had to list my test cases in a word doc with the results of each test. If not, I can comment here once the fields are tested thoroughly..
@flamesjames Please no Word docs. Just test and comment here.
Hello @clstaudt - I have completed my testing on the fields; comments below:
Auth Screen
-
Name: It might be helpful to add verbiage for whether a full name should be entered or just a first name. I feel the use of a person’s name varies from app to app, so a user might think twice when seeing only “your name” as I did. If both first and last name are significant for Tuttle, it also might be suggested to break these apart into two separate text fields for data capture (only if the distinction is needed somewhere else in the app). If not necessary, one text field for name is fine.
-
Street Name/Number: I feel that the verbiage “Street Name” and “Street Number” could confuse some folks - “Address Line 1” and “Address Line 2” OR “Street Address” and “Apt/Bldg/Unit #” may be better as these seem more universal.
-
Street Number: Should this field be considered required? (if this has the same meaning as “Apt/Bldg/Unit #”.
-
Validations on all of the required fields occurred correctly if a user tried to move forward without filling a required field out.
Profile Screen
- The same validations noted on the Auth Screen were working on the profile screen.
Preferences Screen
- The dropdown pickers on the Cloud and Locale screens were descriptive.
Create New Contact Screen
- Street and Street No appear here as well (see note above about that verbiage).
- Other than that, fields were validated correctly.
Create New Client Screen
- The validation messages always showing right above “Client’s Name” could be slightly confusing. It would be better if validation messages could be near the field that failed validation.
- Upon selecting an Invoicing Contact from the dropdown, are the fields that are below that supposed to auto-populate with that contact’s info? That is not happening.
- There were address validations on the “Invoicing Contact” fields, however, I was able to submit without entering a Country for one of them.
Create New Contract Screen
- It may be a good idea to show an example format of the “Rate of remuneration” to let the user know how the data should be entered. I was also able to enter a with more than 2 decimal places.
- I was also able to enter a large number with many decimal places for VAT rate.
Create New Project Screen
- The fields/hints were good here. However, I selected a Contract from the dropdown but it was still throwing the “Please specify the contract” validation error message.
Misc. Suggestions
- Not having a “Save” button on the “Cloud provider” screen in the Preferences section kinda threw me off, although you are saving the user a button click.
- The textfield labels get pretty small after entering values (and they are small to begin with). I feel slightly increasing the font size would be useful.
- After adding a Contact on the Contacts screen, the validation displayed in red “You have not added any contacts yet” does not go away. Same thing happened on My Clients screen.
- It might be better UX to show required fields with a red asterisk or some way to clearly tell the user which info is required or not. Reason I say this is because when creating a new contract, after typing in a Title and choosing a client and clicking “Create Contract”, it just says “Failed to save the contract. Verify the info and try again” but doesn’t tell you which fields are causing that error message.
👍🏾
Thank you for the thorough review @flamesjames! Let me turn this into tickets.
Name: It might be helpful to add verbiage for whether a full name should be entered or just a first name. I feel the use of a person’s name varies from app to app, so a user might think twice when seeing only “your name” as I did. If both first and last name are significant for Tuttle, it also might be suggested to break these apart into two separate text fields for data capture (only if the distinction is needed somewhere else in the app). If not necessary, one text field for name is fine.
-> #201
Not having a “Save” button on the “Cloud provider” screen in the Preferences section kinda threw me off, although you are saving the user a button click.
@vlad-ed-git Would you like to triage those comments, i.e. make tickets (in ToDo if really urgent because it throws testers off, in backlog otherwise) or decide "wontfix"?
The validation messages always showing right above “Client’s Name” could be slightly confusing. It would be better if validation messages could be near the field that failed validation.
It might be better UX to show required fields with a red asterisk or some way to clearly tell the user which info is required or not. Reason I say this is because when creating a new contract, after typing in a Title and choosing a client and clicking “Create Contract”, it just says “Failed to save the contract. Verify the info and try again” but doesn’t tell you which fields are causing that error message.
Upon selecting an Invoicing Contact from the dropdown, are the fields that are below that supposed to auto-populate with that contact’s info? That is not happening.
It may be a good idea to show an example format of the “Rate of remuneration” to let the user know how the data should be entered. I was also able to enter a with more than 2 decimal places.
I was also able to enter a large number with many decimal places for VAT rate.
Street Name/Number: I feel that the verbiage “Street Name” and “Street Number” could confuse some folks - “Address Line 1” and “Address Line 2” OR “Street Address” and “Apt/Bldg/Unit #” may be better as these seem more universal.
@flamesjames I think this is our first internationalization issue: Expectations differ depending on your location. Can there be a one size fits all solution? Do we need to customize the form based on country?
Street Name/Number: I feel that the verbiage “Street Name” and “Street Number” could confuse some folks - “Address Line 1” and “Address Line 2” OR “Street Address” and “Apt/Bldg/Unit #” may be better as these seem more universal.
@flamesjames I think this is our first internationalization issue: Expectations differ depending on your location. Can there be a one size fits all solution? Do we need to customize the form based on country?
@clstaudt - that's a great point, I can do some more research on this topic to see what our options are and get back to you.
@flamesjames It's 2023, can I make an overly optimistic wish? A Python library that parses any address from anywhere in the world from text and stores it in a flexible data structure so that it just works.
Similar to my comment on the name field under #201 . Is there any point where we need for example, the street number itself (in the features of the app) and not a full address? If not (and I can't think of a reason we would) , this should just a single address field. And it shouldn't be separated (as we do now). Doing this makes this an internationalization issue unnecessarily.
Here is a link on best practices
link
Would you like to triage those comments, i.e. make tickets (in ToDo if really urgent because it throws testers off, in backlog otherwise) or decide "wontfix"?
Yes. I will comment here , then create necessary issues for the fixes.
The validation messages always showing right above “Client’s Name” could be slightly confusing. It would be better if validation messages could be near the field that failed validation.
Will fix this. #209
It might be better UX to show required fields with a red asterisk or some way to clearly tell the user which info is required or not. Reason I say this is because when creating a new contract, after typing in a Title and choosing a client and clicking “Create Contract”, it just says “Failed to save the contract. Verify the info and try again” but doesn’t tell you which fields are causing that error message.
I agree with specifying required fields. But will mark the optional fields, instead of the required ones.
Upon selecting an Invoicing Contact from the dropdown, are the fields that are below that supposed to auto-populate with that contact’s info? That is not happening.
Fixed.
@clstaudt you made some changes in these form dropdowns that introduced this issue. This was fixed on latest merge. The reason a drop-down item is prefixed with an id is because we need a way to get the object associated with the selected item (and hence we need to make the displayed item unique). I saw that you removed the prefix setting, but then didn't setup an alternative (I can't think of one myself), so I reverted that change. To illustrate, consider a drop-down that shows contacts. A contact name (or title) which is what we display in the drop-down for user to select, is not a unique field. We can't rely on it, as is, to figure out which contact exactly was selected. This is why I write #4. Christian Paul, with the 4 being the id, so this way I know which Christian Paul (of the possibly* hundreds) this is.
It may be a good idea to show an example format of the “Rate of remuneration” to let the user know how the data should be entered. I was also able to enter a with more than 2 decimal places.
@clstaudt this is an example of checks that should be added either as a method of a model e.g. is_valid() OR as part of the data source create/update method.
Will fix this together (*which would fix the one below). #210
I was also able to enter a large number with many decimal places for VAT rate.
Same as above
@vlad-ed-git sorry, wasn't aware that the "#id - " is required. Okay with reverting it for now. But unhappy that it is required: Database IDs are not meaningful to the user and should not be displayed. Other apps aren't doing that. Let's see if an alternative comes up.
@clstaudt this is an example of checks that should be added either as a method of a model e.g. is_valid() OR as part of the data source create/update method.
@vlad-ed-git Here we are probably not using the sqlmodel library's full potential yet. sqlmodel = sqlalchemy (an ORM) + pydantic (a library for data validation). Some required reading in order to avoid reinventing the wheel.
@clstaudt We can go ahead and close this as all the fixes have been delegated to respective issues.