Issue 376: Updated MUI component to used styled component

bingeboy commented 10 months ago

Issue 376: Updated MUI component to used styled component

This pull requests adds support to the styled components found on the user's document page.

This PR:

Resolves #376

The files this PR effects:

Components

<DocumentTable />

Tests

Currrent there are zero tests for this component

Screenshots (if applicable):

bingeboy commented 10 months ago

@xscottxbrownx @andycwilliams @leekahung is this all that was needed or are there additional components that need to be replaced?

xscottxbrownx commented 10 months ago

I think you have completely misinterpreted Issue #376

The goal of that issue is to get rid of the styled components in those two components/files, and use the MUI based styling (or sx prop in MUI if needed.)

We are trying to get to and enforce a consistent way of styling/css...
First, we got rid of the separate styles.css file. Second, we are now trying to get rid of all the use of styled components everywhere and use the MUI built-in so the codebase is consistent.

xscottxbrownx commented 10 months ago

Also I just realized, that Issue #376 is tied/linked to Issue #219 ...

They both import the same styles, so honestly it seems these components would need to be refactored at same time or else they will break.

The source of the styled components styling: src/components/Table/TableStyles.js

Components importing/using the styling instead of MUI: src/components/Contacts/ContactListTable.jsx src/components/Contacts/ContactListTableRow.jsx src/components/Documents/DocumentTable.jsx src/components/Documents/DocumentTableRow.jsx

Not sure if it would be more efficient to make new components, change the theme.js to use these styles by default, or what honestly. I haven't researched the way I would go about making these changes yet.

leekahung commented 10 months ago

Think we could switch this up with MUI data grid https://mui.com/x/react-data-grid/

bingeboy commented 10 months ago

There are a few ways to support custom styles with MUI. Seemed like there were existing overrides in the theme.js file so I was removing the custom components in favor of keeping the existing MUI components based on feedback.

There was another PR #408 I was working on which requested the removal of classes being declared in the component file. Alternatively we could use hooks but that seemed like it would add complexity in this case. I haven't been brief on architecture decisions for MUI styling so I'm following the MUI docs.

I can also update #219 to use the same styles and then drop the additional existing components.
Adding the react-data-grid component can also be done. I'm not sure if there is a design for it or if I should add the component and let the design team provide feedback once it's been added?
Would #219 also use the react-data-grid ?

bingeboy commented 10 months ago

I believe I removed all the uses of the <StyledTableCell /> and <StyledTableRow /> . I can upgrade the table to a <DataGrid /> if that is within the scope of this ticket.

bingeboy commented 10 months ago

@leekahung so I have the <DataGrid /> mostly working on my local on another branch. I'd recommend making that another ticket though.

leekahung commented 10 months ago

@leekahung so I have the <DataGrid /> mostly working on my local on another branch. I'd recommend making that another ticket though.

Sounds good

bingeboy commented 10 months ago

Squashing thx

xscottxbrownx commented 9 months ago

@milofultz

Guess I had chances to look at this before merge and too late now,

but for future reference you don't need to pass the theme provider to each individual component. It's already passed in App.jsx to all children (meaning every component of the app.)

And I'm not a fan of making the variables for theme.pallete.primary.main and .slight. I'd prefer we do this to all the colors or none. Just inconsistent. As dev, do I now need to do theme.pallete.primary.light or themePalletePrimaryLight? If theme.pallete.primary.light was set to themePalletePrimaryLight then either would work, but you see the point in the inconsistency...

codeforpdx / PASS