[Enhancement] - Replace existing Contact List with MUI Data Grid #457

leekahung · 2023-10-14T17:57:25Z

This PR:

Updates existing table used for our Contact List with MUI Data Grid. The module for MUI Data Grid has been included to package.json. The column with originally with the link to the contact profile has been split up into the original text and a button icon in order to allow for filtering via MUI Data Grid. Other than that, the grid should function similarly to our previous table.

The unused ContactListTableRow.jsx is removed and a new component ContactProfileIcon.jsx is created for the React Router Link to the contact profile page.

The PR also includes a minor correction for contactProfile.profileImage in src/components/Profile/ProfileImageField.jsx. Before the profileImage would be incorrectly rendered because of the incorrect variable name, profileImg, which doesn't exist as part of contactProfile.

The column with pinning has been removed (UX team believes if there's already a filter + sorting capability with MUI Data Grid, this might not be necessary; it'll be better to have this moved as a type of status instead). The name from Person has been split up into "First Name" and "Last Name" for a better sorting experience.

The files this PR effects:

Components

src/components/Contacts/ContactListTable.jsx
src/components/Contacts/ContactListTableRow.jsx (file removed)
src/components/Contacts/ContactProfileIcon.jsx (file added)
src/components/Profile/ProfileImageField.jsx

Test Files

test/components/Contacts/ContactsListTable.test.jsx
test/components/Profile/ProfileImageField.test.jsx
test/pages/Contacts.test.jsx

Other Files

src/components/Contacts/index.js
package.json
package-lock.json

Screenshots (if applicable):

Screen.Recording.2023-10-19.at.10.30.59.mov

Future Considerations:

Implement accessibility improvements with MUI Data Grid after merger

… contactProfile.profileImage in ProfileImageField.jsx

milofultz

If we could resolve the button nested in an a tag, that would be great. Thank you!

test/components/Contacts/ContactsListTable.test.jsx

src/components/Contacts/ContactProfileIcon.jsx

src/components/Contacts/ContactListTable.jsx

leekahung · 2023-10-15T19:14:27Z

If we could resolve the button nested in an a tag, that would be great. Thank you!

Thanks for the review @milofultz! I'll have the changes made in a bit.

andycwilliams · 2023-10-15T19:33:56Z

Is there an associated issue for this?

@milofultz This is valuable feedback, at least for my own understanding. More insight into accessibility is really helpful for the project.

leekahung · 2023-10-15T19:39:45Z

Is there an associated issue for this?

I was looking around the issues. There doesn't seem to be one for this or the ones that were there was closed.

…; Removed Button, but left ContactPageIcon for Link

timbot1789 · 2023-10-15T22:00:02Z

I think the data grid is a good enhancement over the current view. I especially like how we can easily add sorting to it (which may eliminate the need for pinning / favoriting).

How would you say it relates to this ticket, where I talk about converting the contacts list into a series of cards?
I based that off of one of the figma designs UX submitted.

leekahung · 2023-10-15T22:41:57Z

How would you say it relates to this ticket, where I talk about converting the contacts list into a series of cards? I based that off of one of the figma designs UX submitted.

I think we could certainly have this link this to that issue since it does update the UI.

If we want to, think we could also separate out different fields from contact (like first name, last name) in a follow-up PR to make better use of Data Grid's innate sorting capabilities instead of our existing sorting scheme.

andycwilliams · 2023-10-15T23:02:38Z

How would you say it relates to this ticket, where I talk about converting the contacts list into a series of cards? I based that off of one of the figma designs UX submitted.

I think we could certainly have this link this to that issue since it does update the UI.

If we want to, think we could also separate out different fields from contact (like first name, last name) in a follow-up PR to make better use of Data Grid's innate sorting capabilities instead of our existing sorting scheme.

When I last talked to UX about this, they said they were still working on the desktop version of these cards. So that combined with temporarily focusing on documentation recently, I haven't done a ton with #343 lately (and should really have moved that to the backlog).

I'm still willing to complete that, it would just mean waiting until this is merged before continuing.

Also, I would likely focus on #414 before moving onto #343.

timbot1789 · 2023-10-16T00:37:50Z

When I last talked to UX about this, they said they were still working on the desktop version of these cards. So that combined with temporarily focusing on documentation recently, I haven't done a ton with #343 lately (and should really have moved that to the backlog).

I'm still willing to complete that, it would just mean waiting until this is merged before continuing.

Also, I would likely focus on #414 before moving onto #343.

That sounds good to me. I personally like the card look more than the table. IMO it's more humanizing, and shows the data in a more familiar way. And I think it will work well in a desktop environment. The table view is good when you have several rows worth of data you need to look at all at once, but I don't know how often that will be the case here.

src/components/Contacts/ContactListTable.jsx

andycwilliams · 2023-10-16T05:26:13Z

package.json

@@ -34,6 +34,7 @@
    "@mui/material": "^5.12.2",
    "@mui/styled-engine-sc": "^5.12.0",
    "@mui/system": "^5.12.1",
+    "@mui/x-data-grid": "^6.16.2",


Isn't this data grid what we used months back when creating the original table and implementing MUI? I don't recall why we decided not to go with it at that time.

Yeah, it was the same one as before. I think at the time we didn't went with it was because there were still a few thing we wish to discuss, although I don't remember which things.

milofultz · 2023-10-16T14:36:27Z

Is there an associated issue for this?

I was looking around the issues. There doesn't seem to be one for this or the ones that were there was closed.

@andycwilliams @leekahung The issues I had when using VoiceOver on the data grid (which I'll admit isn't technically super proficient) were just basic navigation. In comparison to a data grid like this one from W3 which was super easy to navigate and interact with, the MUI one on their site was almost unusable.

I don't quite have to technical know how with grid yet to understand why this is the case, but I saw similar complaints in their backlog (mui/mui-x#4749, mui/mui-x#4658, mui/mui-x#4282).

Again, don't know how much of this is them, us, or me, but I saw a little bit of signal confirming my suspicions in their backlog so thought I'd bring it up.

edit: oop misread the original post, but leaving this here for clarity anyway, sorry for the erroneous tag now :)

milofultz

Just one small thing, but not a blocker

src/components/Contacts/ContactProfileIcon.jsx

test/components/Contacts/ContactsListTable.test.jsx

leekahung · 2023-10-19T17:48:07Z

Got everything updated from the suggestions. Let me know if there's still anything that needs to be reviewed for this PR.

leekahung · 2023-10-22T22:42:33Z

Hey @xscottxbrownx @andycwilliams. Think I've manage to touch on the things you guys mentioned. Let me know if there's still other things you guys need to review or it's good to go?

andycwilliams

Overall, it looks really nice. It's strange seeing the data grid again after all these months.

I think it would have been good to first create an issue before opening a PR as we normally do.

Curious, is the grid also intended to be implemented with the documents table?

andycwilliams · 2023-10-23T03:01:46Z

src/components/Contacts/ContactListTable.jsx

@@ -44,31 +50,74 @@ const ContactListTable = ({ contacts, deleteContact }) => {
  const contactsCopy = [...contacts];
  const sortedContacts = contactsCopy.sort(comparePerson);


With the new data grid, is a custom sorting function still necessary? Unless I'm mistaken, it should be possible to use initialState in DataGrid for the same results.

We could certainly drop the custom sort and leave it as an initial state.

Got the custom sort dropped for sorting from initialState

andycwilliams · 2023-10-23T03:24:07Z

src/components/Contacts/ContactListTable.jsx

+  const columnTitlesArray = [
+    {
+      field: 'First Name',
+      width: 120,


Could we possibly expand the table a bit when in desktop views? The quickest way would be by widening the columns.

I know there isn't a ton of data to display at the moment, but it'd help to use up some of all the empty space. Plus, in case anyone has a longer name, it'd prevent that from being cut off.

Would like to keep the minimum width of the overall grid to 300px for mobile, but I'll play around with the column widths.

Made text columns flexible. If we get more columns included in between them in the future, we could make them flexible as well.

Screen.Recording.2023-10-22.at.22.27.06.mov

andycwilliams · 2023-10-23T03:30:03Z

src/components/Contacts/ContactListTable.jsx

+      field: 'actions',
+      type: 'actions',
+      width: 70,
+      getActions: (contactData) => [


Although it's not essential, I think it'd be nicer to have a title for the delete column. If only because it's a bit odd to have one nameless column. The same kind of column in the documents is named, so this'd make it more consistent.

Adding headerName: 'Delete', here would accomplish this.

I'll add one. I'll expand the width of this too.

Got this included.

leekahung · 2023-10-23T04:18:33Z

Curious, is the grid also intended to be implemented with the documents table?

Yeah, I think the grid would work well for the document list as well. Think we could have an issue created for that.

xscottxbrownx · 2023-10-23T16:26:40Z

Issue with rendering size on skinny mobile devices (like Galaxy Fold and similar), but not too worried about for MVP.

leekahung · 2023-10-23T19:35:44Z

Issue with rendering size on skinny mobile devices (like Galaxy Fold and similar), but not too worried about for MVP.

Yeah, I believe those screen sizes are at around 280px width? At an earlier discussion, I believe we've talked about limiting the lower screen limit to about 300px. Think I could adjust it to make it fit the smaller size using vw for the width size on the box surrounding the grid.

Screen.Recording.2023-10-23.at.12.35.01.mov

andycwilliams

Looking better. There's a plan to add at least one more column (an icon button to allow for direct messaging), so that will help fill out the table.

I'm seeing some large gaps between the navbar and table (see screenshot) when using larger viewports. Looks to be caused by justifyContent: 'center' on line 58 in Contacts.jsx

Is this the desired effect? It might be nicer to have it consistent on all viewports and make the table always close to the navbar. Just want to be sure.

I'd really like to hear from case workers about how small the screens they actually use are. It may not even be an issue to set a 300px limit. But it may take time to get feedback.

leekahung · 2023-10-24T00:40:38Z

I'm seeing some large gaps between the navbar and table (see screenshot) when using larger viewports. Looks to be caused by justifyContent: 'center' on line 58 in Contacts.jsx

I could certainly pull the Data Grid back up closer the the Navbar without the justifyContent. I'll also make the grid use a set height to fill out the the spacing in the "Contacts" page. It would not affect the mobile view.

Screen.Recording.2023-10-23.at.17.33.35.mov

I'd really like to hear from case workers about how small the screens they actually use are. It may not even be an issue to set a 300px limit. But it may take time to get feedback.

With regards to the mobile width limit, what we're making here should be quite flexible down to 280px and lower, so I don't think we need to worry about that point now. (As you can see below, the grid content stretches out to 380px, but the grid itself can remain small (around ~210px in the example clip) thanks to the horizontal scrolling implemented with the grid.)

Screen.Recording.2023-10-23.at.17.37.52.mov

leekahung · 2023-10-26T01:09:25Z

I believe the changes here are now adequate?

leekahung added 4 commits October 14, 2023 09:05

Replaced existing contact list with MUI Data Grid; Fixed variable for…

7408a2c

… contactProfile.profileImage in ProfileImageField.jsx

Styled data grid

e4dd4e7

Additional style changes to data grid

411fb2c

Removing unneeded style changes for Contacts.jsx

1257771

leekahung added the enhancement Enhancement of existing features label Oct 14, 2023

leekahung self-assigned this Oct 14, 2023

leekahung requested review from xscottxbrownx, andycwilliams and timbot1789 October 14, 2023 17:59

leekahung added 3 commits October 14, 2023 10:59

Merge branch 'Development' into add-mui-data-grid

783eb60

Updating test files

15f632e

Updates JSDoc comments for ContactProfileIcon and ContactListTable

418c2a3

milofultz requested changes Oct 15, 2023

View reviewed changes

test/components/Contacts/ContactsListTable.test.jsx Outdated Show resolved Hide resolved

src/components/Contacts/ContactProfileIcon.jsx Outdated Show resolved Hide resolved

src/components/Contacts/ContactListTable.jsx Show resolved Hide resolved

leekahung added 2 commits October 15, 2023 13:53

Update unit test using queryByRole

bee5dd7

Refactored ContactProfileIncon with onClick in Link instead of button…

30edfdd

…; Removed Button, but left ContactPageIcon for Link

leekahung requested a review from milofultz October 15, 2023 20:59

leekahung added 2 commits October 15, 2023 14:07

Removed unused theme and moved style for Icon to sx in Icon

e6d8ab0

Included titleAccess to ContactPageIcon for accessibility

3d46243

leekahung linked an issue Oct 15, 2023 that may be closed by this pull request

Enhancement: Update Contacts View to display Contact Cards instead of Table #343

Closed

leekahung mentioned this pull request Oct 15, 2023

Enhancement: Update Contacts View to display Contact Cards instead of Table #343

Closed

xscottxbrownx reviewed Oct 16, 2023

View reviewed changes

src/components/Contacts/ContactListTable.jsx Outdated Show resolved Hide resolved

andycwilliams reviewed Oct 16, 2023

View reviewed changes

Merge branch 'Development' into add-mui-data-grid

a8f645d

milofultz approved these changes Oct 19, 2023

View reviewed changes

src/components/Contacts/ContactProfileIcon.jsx Show resolved Hide resolved

test/components/Contacts/ContactsListTable.test.jsx Outdated Show resolved Hide resolved

leekahung requested review from xscottxbrownx and andycwilliams October 19, 2023 17:33

Correcting unit test with grid update

ae54923

leekahung added 2 commits October 20, 2023 15:49

Merge branch 'Development' into add-mui-data-grid

35bd5a3

Merge branch 'Development' into add-mui-data-grid

c96988e

andycwilliams reviewed Oct 23, 2023

View reviewed changes

leekahung added 3 commits October 22, 2023 22:25

Made grid columns stretch out to width

e6212ec

Adjusting profile icon alignment in data grid

1e16e07

Replacing custom sort with initialState sorting

5434683

leekahung requested a review from andycwilliams October 23, 2023 05:47

Adjusting dimensions for lower screen widths

2631465

andycwilliams reviewed Oct 23, 2023

View reviewed changes

leekahung mentioned this pull request Oct 24, 2023

[Enhancement] - Issue 475/small screen adjustments #476

Merged

Adjust UI for Contact page to make list fill out more of the page

f7070d0

leekahung requested a review from andycwilliams October 24, 2023 00:42

leekahung added 4 commits October 24, 2023 11:51

Merge branch 'Development' into add-mui-data-grid

856c255

Merge branch 'Development' into add-mui-data-grid

8cb453c

Merge branch 'Development' into add-mui-data-grid

94176ad

Merge branch 'Development' into add-mui-data-grid

066770f

leekahung merged commit 1d1581c into Development Oct 26, 2023
3 checks passed

leekahung deleted the add-mui-data-grid branch October 26, 2023 01:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Enhancement] - Replace existing Contact List with MUI Data Grid #457

[Enhancement] - Replace existing Contact List with MUI Data Grid #457

leekahung commented Oct 14, 2023 •

edited

Loading

milofultz left a comment

leekahung commented Oct 15, 2023

andycwilliams commented Oct 15, 2023

leekahung commented Oct 15, 2023

timbot1789 commented Oct 15, 2023

leekahung commented Oct 15, 2023 •

edited

Loading

andycwilliams commented Oct 15, 2023 •

edited

Loading

timbot1789 commented Oct 16, 2023

andycwilliams Oct 16, 2023

leekahung Oct 16, 2023

milofultz commented Oct 16, 2023 •

edited

Loading

milofultz left a comment

leekahung commented Oct 19, 2023

leekahung commented Oct 22, 2023

andycwilliams left a comment

andycwilliams Oct 23, 2023

leekahung Oct 23, 2023

leekahung Oct 23, 2023

andycwilliams Oct 23, 2023

leekahung Oct 23, 2023

leekahung Oct 23, 2023

andycwilliams Oct 23, 2023 •

edited

Loading

leekahung Oct 23, 2023

leekahung Oct 23, 2023

leekahung commented Oct 23, 2023 •

edited

Loading

xscottxbrownx commented Oct 23, 2023

leekahung commented Oct 23, 2023

andycwilliams left a comment

leekahung commented Oct 24, 2023 •

edited

Loading

leekahung commented Oct 26, 2023

		@@ -44,31 +50,74 @@ const ContactListTable = ({ contacts, deleteContact }) => {
		const contactsCopy = [...contacts];
		const sortedContacts = contactsCopy.sort(comparePerson);

[Enhancement] - Replace existing Contact List with MUI Data Grid #457

[Enhancement] - Replace existing Contact List with MUI Data Grid #457

Conversation

leekahung commented Oct 14, 2023 • edited Loading

This PR:

The files this PR effects:

Components

Test Files

Other Files

Screenshots (if applicable):

Future Considerations:

milofultz left a comment

Choose a reason for hiding this comment

leekahung commented Oct 15, 2023

andycwilliams commented Oct 15, 2023

leekahung commented Oct 15, 2023

timbot1789 commented Oct 15, 2023

leekahung commented Oct 15, 2023 • edited Loading

andycwilliams commented Oct 15, 2023 • edited Loading

timbot1789 commented Oct 16, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

milofultz commented Oct 16, 2023 • edited Loading

milofultz left a comment

Choose a reason for hiding this comment

leekahung commented Oct 19, 2023

leekahung commented Oct 22, 2023

andycwilliams left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andycwilliams Oct 23, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leekahung commented Oct 23, 2023 • edited Loading

xscottxbrownx commented Oct 23, 2023

leekahung commented Oct 23, 2023

andycwilliams left a comment

Choose a reason for hiding this comment

leekahung commented Oct 24, 2023 • edited Loading

leekahung commented Oct 26, 2023

leekahung commented Oct 14, 2023 •

edited

Loading

leekahung commented Oct 15, 2023 •

edited

Loading

andycwilliams commented Oct 15, 2023 •

edited

Loading

milofultz commented Oct 16, 2023 •

edited

Loading

andycwilliams Oct 23, 2023 •

edited

Loading

leekahung commented Oct 23, 2023 •

edited

Loading

leekahung commented Oct 24, 2023 •

edited

Loading