[MPDX-7828] Fix several UX issues with task modals #851

canac · 2024-01-10T18:04:34Z

Description

Prevent blurring contacts and assignee autocomplete from removing the highlighted option
Fix selecting tags
Use desktop-optimized date and time pickers
Default appointments' completed date to their start date

There are other places that could use these new inputs, but that refactoring is part of a proposed tech debt item that will be addressed later.

Testing

Please specifically test keyboard usage for the task inputs and let me know if there's improvements to be made there. @caleballdrin I would love to get your feedback on that too.

https://jira.cru.org/browse/MPDX-7828

Checklist:

I have given my PR a title with the format "MPDX-(JIRA#) (summary sentence max 80 chars)"
I have applied the appropriate labels. (Add the label "On Staging" to get the branch automatically merged into staging.)
I have requested a review from another person on the project

aws-amplify-us-east-1 · 2024-01-10T18:20:16Z

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-851.d3dytjb8adxkk5.amplifyapp.com

dr-bizz

This is great! It adds a layer of simplicity to our fields. I had a few questions. The UI/UX looked great, but with a PR of this size, it's easy for me to miss something while reviewing

dr-bizz · 2024-01-16T20:42:01Z

src/components/Task/Modal/Form/Inputs/ActivityTypeAutocomplete/ActivityTypeAutocomplete.tsx

You're missing the activity === 'DONT_CHANGE' functionality at src/components/Task/MassActions/EditTasks/MassActionsEditTasksModal.tsx on line 188 - 191

I know the Don't change functioanilty is only used when editing items already set, but we will lose this functionailty. Maybe we can have a isUpdatingField prop which allows for the don't change functionailty.

Removing the "Don't change" option is intentional because Autocomplete already lets you click an X to clear the selection. If the user clears their selection, the autocomplete will be blank again and MassActionsEditTasksModal won't change the tasks' activity type. But let me know if there's a bug.

The reason I mentioned it is that I liked the don't change as it's clear what it does. It's fine to leave it out. Something which could be good further down the road, making the edit modals list out the changes the user is making. To ensure the user knows what is about to happen.
Changing title to "new title"
Updating action to "Call"

src/components/Task/Modal/Form/Inputs/ActivityTypeAutocomplete/ActivityTypeAutocomplete.tsx

src/components/Task/Modal/Form/Inputs/TagsAutocomplete/TagsAutocomplete.tsx

src/components/Task/Modal/Form/Inputs/TagsAutocomplete/TagsAutocomplete.graphql

src/theme.ts

dr-bizz · 2024-01-16T21:36:51Z

The error is with the file src/components/Layouts/Primary/TopBar/Items/AddMenu/Items/CreateMultipleContacts/CreateMultipleContacts.test.tsx Line 355

canac · 2024-01-16T22:06:28Z

The error is with the file src/components/Layouts/Primary/TopBar/Items/AddMenu/Items/CreateMultipleContacts/CreateMultipleContacts.test.tsx Line 355

Are you getting a test failure? I get that error locally occasionally. It probably needs a higher timeout. It usually works when testing just that file.

dr-bizz · 2024-01-17T18:24:39Z

CreateMultipleContacts.test.ts

This can be a different PR, could it be worth changing how it's testing the autocomplete address, so it doesn't need a 20-second timeout? such as mocking the predictions

canac · 2024-01-17T19:39:30Z

This can be a different PR, could it be worth changing how it's testing the autocomplete address, so it doesn't need a 20-second timeout? such as mocking the predictions

It will probably be in a different PR. I have an approved BC item for it. I believe that some of the tests are slow because jsdom emulating the DOM is slow, but I agree that speeding up the tests if possible is preferable to increasing the timeout.

dr-bizz · 2024-01-25T19:22:02Z

Awesome work!

canac · 2024-02-23T18:06:39Z

@dr-bizz Can I get a re-review on this? I added some more commits to address Scott's feedback in this comment.

I didn't love having to patch the autocomplete dependency, but it was the only way I could get it to work since autocomplete doesn't expose anything about controlling the highlight.

If Scott likes the date picker improvements, I'll modify the rest of the modals in the app, probably in a follow-up PR.

Also, if you test, do it locally because I'm still trying to get staging to update.

dr-bizz

It's looking great. I found some issues with it while testing it locally.

src/components/Task/MassActions/EditTasks/MassActionsEditTasksModal.tsx

src/components/Dashboard/ThisWeek/NewsletterMenu/MenuItems/LogNewsLetter/LogNewsletter.tsx

dr-bizz · 2024-02-26T13:46:21Z

src/components/Task/Modal/Form/DateTimeFieldPair.tsx

I think this will be fine, as users will know to click outside of the popup, but we might have to revert this if users complain about it.

I don't love this change either, but Scott wants it to not close after selecting the minutes because he chooses AM/PM after choosing the minutes. If he chose AM/PM, then hours, then minutes, we could keep closeOnSelect enabled.

dr-bizz · 2024-02-26T13:50:08Z

src/components/Task/Modal/Form/DateTimeFieldPair.tsx

I don't believe this fixes Scott concern about the time component. The time is still in 24 hours and it doesn't close when you enter the AM/PM. I believe this input might be better (Scrrenshot below.)

Unless I'm mistaken, that example is from the v6 page. We'd have to upgrade @mui/x-date-pickers from v5 to v6 to use that component. I'm not saying we can't or shouldn't upgrade, just pointing that out that it will be a little more work. I think that new picker is a better interface though.

Ah I didn't see that

src/components/Task/Modal/Form/DateTimeFieldPair.test.tsx

...ls/ContactDonationsTab/PartnershipInfo/EditPartnershipInfoModal/EditPartnershipInfoModal.tsx

canac · 2024-02-28T20:27:13Z

@dr-bizz Could I get a re-review? I ended up reverting the changes that added support for year and time abbreviations because I couldn't get them working correctly.

canac · 2024-02-29T21:00:46Z

@dr-bizz OK this is now actually ready for review. Scott gave me permission to remove the calendar and clock dropdowns in the task modals so that we can add support for date and time shortcuts like "1/1/24" and "1pm". I was unable to get the shortcuts working with the desktop dropdowns. If he likes how it works, I'll refactor the other date and time components to use <DateTimeFieldPair>.

dr-bizz · 2024-03-01T14:19:37Z

@dr-bizz OK this is now actually ready for review. Scott gave me permission to remove the calendar and clock dropdowns in the task modals so that we can add support for date and time shortcuts like "1/1/24" and "1pm". I was unable to get the shortcuts working with the desktop dropdowns. If he likes how it works, I'll refactor the other date and time components to use .

I think I'm a little confused. I feel like removing the Calendar date popups, removes a key part of the UX. Making it harder for users to enter the date.

dr-bizz

The code looks great I had a few comments, but that is it.

src/components/Dashboard/ThisWeek/NewsletterMenu/MenuItems/LogNewsLetter/LogNewsletter.tsx

src/components/Task/Modal/Form/DateTimeFieldPair/DesktopTimeField.test.tsx

dr-bizz

The fields and code look awesome! 🔥🔥🔥🔥🔥 I know how much effort you put into this. Great work.
I won't approve this until all input fields are updated, but overwise I would.

canac · 2024-03-06T19:44:38Z

@dr-bizz The rest of the app uses the custom date/time pickers now. There will be one more to fix once Caleb A's preferences PR merges, but that will be trivial to add.

canac · 2024-03-07T17:49:41Z

@dr-bizz I migrated Caleb's new preferences date pickers.

@caleballdrin In the most recent commit, I tweaked the preferences UI slightly when I migrated to my new custom date pickers. Can you look over that commit, run it locally, and make sure it still looks good? I mostly changed how we're doing the start and end date labels to be more Material-esque.

dr-bizz

You've used three different ways to verify if a value is correct or invalid and set it to null. Was that intentional?

src/components/Settings/preferences/accordions/MpdInfoAccordion/MpdInfoAccordion.tsx - L119
activeMpdStartAt: typeof activeMpdStartAt === 'string'
              ? DateTime.fromISO(activeMpdStartAt)
              : null,

src/components/Task/Modal/Form/TaskModalForm.tsx - L115
completedAt: task.completedAt
              ? DateTime.fromISO(task.completedAt)
              : null,

src/components/Task/Modal/Form/Complete/TaskModalCompleteForm.tsx - L95
completedAt: completedAt?.toISO()

src/components/Dashboard/ThisWeek/NewsletterMenu/MenuItems/LogNewsLetter/LogNewsletter.tsx

dr-bizz

Looks good on Local. Everything worked apart from the Newsletter completed on field

canac · 2024-03-07T19:19:59Z

You've used three different ways to verify if a value is correct or invalid and set it to null. Was that intentional?

@dr-bizz The src/components/Task/Modal/Form/Complete/TaskModalCompleteForm.tsx one is different because it is setting the GraphQL attributes whereas the other two are initializing the Formik form. But the other two aren't intentionally different.

caleballdrin · 2024-03-13T15:09:01Z

@dr-bizz I migrated Caleb's new preferences date pickers.

@caleballdrin In the most recent commit, I tweaked the preferences UI slightly when I migrated to my new custom date pickers. Can you look over that commit, run it locally, and make sure it still looks good? I mostly changed how we're doing the start and end date labels to be more Material-esque.

@canac The date pickers look great and work well!

…option list

* Upgrade to @mui/x-date-pickers v6 * Support 2-digit years and omitted minutes * Use custom date and time pickers throughout the app

canac requested review from caleballdrin and dr-bizz January 10, 2024 18:04

canac added the On Staging Will be merged to the staging branch by Github Actions label Jan 10, 2024

canac force-pushed the 7828-task-modal branch 2 times, most recently from 8c3d47e to ef339b8 Compare January 12, 2024 16:18

dr-bizz requested changes Jan 16, 2024

View reviewed changes

canac mentioned this pull request Jan 17, 2024

[MPDX-7746] Provide defaults for task modal fields #857

Merged

3 tasks

canac requested a review from dr-bizz January 23, 2024 21:12

dr-bizz approved these changes Jan 25, 2024

View reviewed changes

canac requested a review from dr-bizz February 23, 2024 18:06

canac force-pushed the 7828-task-modal branch from ed50bd6 to ca57344 Compare February 23, 2024 19:16

dr-bizz requested changes Feb 26, 2024

View reviewed changes

canac requested a review from dr-bizz February 28, 2024 20:27

dr-bizz reviewed Mar 1, 2024

View reviewed changes

src/components/Dashboard/ThisWeek/NewsletterMenu/MenuItems/LogNewsLetter/LogNewsletter.tsx Outdated Show resolved Hide resolved

src/components/Task/Modal/Form/DateTimeFieldPair/DesktopTimeField.test.tsx Outdated Show resolved Hide resolved

canac requested a review from dr-bizz March 1, 2024 17:32

dr-bizz reviewed Mar 5, 2024

View reviewed changes

canac force-pushed the 7828-task-modal branch from 5719675 to 554dedb Compare March 6, 2024 19:40

canac removed the On Staging Will be merged to the staging branch by Github Actions label Mar 6, 2024

canac requested a review from dr-bizz March 6, 2024 19:43

dr-bizz reviewed Mar 7, 2024

View reviewed changes

dr-bizz self-requested a review March 7, 2024 18:57

dr-bizz reviewed Mar 7, 2024

View reviewed changes

src/components/Dashboard/ThisWeek/NewsletterMenu/MenuItems/LogNewsLetter/LogNewsletter.tsx Outdated Show resolved Hide resolved

dr-bizz requested changes Mar 7, 2024

View reviewed changes

dr-bizz self-requested a review March 7, 2024 19:09

dr-bizz approved these changes Mar 7, 2024

View reviewed changes

canac force-pushed the 7828-task-modal branch from 5240f7b to b22a238 Compare March 7, 2024 19:26

canac added 3 commits March 13, 2024 10:05

Defaults appointments' completed date to their start date

ac3a9bb

Improve type safety

46e2ab4

Create reusable form inputs

bf5d68b

canac added 5 commits March 13, 2024 10:15

Use new inputs

e080ed8

Remove unneeded startAt second

cce7fbd

Patch autocomplete to remove the highlight when the mouse leaves the …

ca18fb9

…option list

Remove unused dependency

3ef260e

Add custom date and time pickers

ea2c914

* Upgrade to @mui/x-date-pickers v6 * Support 2-digit years and omitted minutes * Use custom date and time pickers throughout the app

canac force-pushed the 7828-task-modal branch from b22a238 to ea2c914 Compare March 13, 2024 16:05

canac merged commit 11017aa into main Mar 13, 2024

canac deleted the 7828-task-modal branch March 13, 2024 16:15

canac mentioned this pull request Mar 19, 2024

[HS-1122333] Fix date picker calendar weekdays #893

Merged

3 tasks

[MPDX-7828] Fix several UX issues with task modals #851

[MPDX-7828] Fix several UX issues with task modals #851

Uh oh!

Conversation

canac commented Jan 10, 2024

Description

Testing

Checklist:

Uh oh!

aws-amplify-us-east-1 bot commented Jan 10, 2024

Uh oh!

dr-bizz left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dr-bizz commented Jan 16, 2024

Uh oh!

canac commented Jan 16, 2024

Uh oh!

dr-bizz commented Jan 17, 2024

Uh oh!

canac commented Jan 17, 2024

Uh oh!

dr-bizz commented Jan 25, 2024

Uh oh!

canac commented Feb 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dr-bizz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

canac commented Feb 28, 2024

Uh oh!

canac commented Feb 29, 2024

Uh oh!

dr-bizz commented Mar 1, 2024

Uh oh!

dr-bizz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

dr-bizz left a comment

Choose a reason for hiding this comment

Uh oh!

canac commented Mar 6, 2024

Uh oh!

canac commented Mar 7, 2024

Uh oh!

dr-bizz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dr-bizz left a comment

dr-bizz left a comment •

edited

Loading

canac commented Feb 23, 2024 •

edited

Loading