Skip to content
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

Restyle Enter Contact Details form on case contact form #5726

Conversation

JoshDevHub
Copy link
Contributor

What github issue is this PR for, if any?

Resolves #5348

What changed, and why?

Updated text content to match updated design spec in figma file. Also updated contact duration input fields.

How is this tested? (please write tests!) 💖💪

Tested by existing specs.

Screenshots please :)

Desktop
Selection_415

Mobile
Selection_416

Questions

Our work currently differs from the figma in a couple of ways. Here's ours:
differences_figma

And here's what's in the figma
Selection_417

For the first difference, changing Text/Email isn't so straightforward because it relies on these string values that are saved to the database. Changing this input value introduces a conflict between the form and the underlying data. Any preference for how to proceed on this?

For the second difference, introducing text content that directly trails the numeric input value is tricky to do and is less accessible than just having normal labels. If this is really required though, we can probably put in the work to figure out something for it.

Feelings gif (optional)

What gif best describes your feeling working on this issue? https://giphy.com/
How to embed:

![alt text](https://media.giphy.com/media/1nP7ThJFes5pgXKUNf/giphy.gif)

jp524 and others added 4 commits May 13, 2024 16:04
Co-authored-by: Josh Smith <jmsmith1018@gmail.com>
Co-authored-by: Josh Smith <jmsmith1018@gmail.com>
Co-authored-by: Josh Smith <jmsmith1018@gmail.com>
….com:JoshDevHub/casa into 5348_enter_contact_details_case_contact_form
@compwron
Copy link
Collaborator

"For the second difference, introducing text content that directly trails the numeric input value is tricky to do and is less accessible than just having normal labels. If this is really required though, we can probably put in the work to figure out something for it."
do it the easy accessible way

Copy link
Collaborator

@compwron compwron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. Strange that no tests fail??

@compwron
Copy link
Collaborator

For the first difference, changing Text/Email isn't so straightforward because it relies on these string values that are saved to the database. Changing this input value introduces a conflict between the form and the underlying data. Any preference for how to proceed on this?

We can introduce a mapping display layer, maybe using the existing Presenter objects

This is a great example to remember for how to (not) design code to be easy to modify - the db and the UI should not be so closely tied together.

@compwron compwron merged commit bb074c4 into rubyforgood:main May 19, 2024
17 checks passed
@jp524
Copy link
Collaborator

jp524 commented May 20, 2024

lgtm. Strange that no tests fail??

The tests were relying on id tags, not on labels or text directly, so we didn't have to change them.

@JoshDevHub JoshDevHub deleted the 5348_enter_contact_details_case_contact_form branch May 20, 2024 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update "3. Enter Contact Details" section on Case contact form
3 participants