search

codeforpdx / PASS

PASS project - with HMIS module integration
MIT License
28 stars 25 forks source link

Issue 503/pill buttons #540

Closed xscottxbrownx closed 10 months ago

xscottxbrownx commented 10 months ago

This PR:

Resolves #503

1. Makes buttons consistent with pill shapes by editing in theme.js 2. Fixes some typos in theme.js 3. Clears unused code in MessageStyles.js and renamed to PaginationStyles.js

xscottxbrownx commented 10 months ago

Home test will be fixed in PR #516

xscottxbrownx commented 10 months ago

@andycwilliams @leekahung

So since we are using this on every single MUI Button component, I just moved it into theme.js. So after this, anytime we put in a <Button> anywhere it will automatically have a border-radius: 25px, unless we specifically tell it differently via the sx prop.

But also new issue:

I believe only PaginationContainer is being used in the MessageStyles file (src > components > Messages > MessageStyles.jsx). Just need a few other eyes to verify that before deleting it all and renaming to PaginationStyles.jsx.

leekahung commented 10 months ago

@andycwilliams @leekahung

So since we are using this on every single MUI Button component, I just moved it into theme.js. So after this, anytime we put in a <Button> anywhere it will automatically have a border-radius: 25px, unless we specifically tell it differently via the sx prop.

Theming it would be good

andycwilliams commented 10 months ago

@andycwilliams @leekahung

So since we are using this on every single MUI Button component, I just moved it into theme.js. So after this, anytime we put in a <Button> anywhere it will automatically have a border-radius: 25px, unless we specifically tell it differently via the sx prop.

But also new issue:

I believe only PaginationContainer is being used in the MessageStyles file (src > components > Messages > MessageStyles.jsx). Just need a few other eyes to verify that before deleting it all and renaming to PaginationStyles.jsx.

Putting it in the theme is a good call.

I was wondering if we wanted to switch to the MUI pagination component instead. I have no strong feelings about it myself. There's nothing wrong with the React version.

andycwilliams commented 10 months ago

They aren't caused by this PR but I am now noticing a couple of typos in theme on lines 36 and 69. severieties and coolor, namely.

xscottxbrownx commented 10 months ago

They aren't caused by this PR but I am now noticing a couple of typos in theme on lines 36 and 69. severieties and coolor, namely.

fixed

leekahung commented 10 months ago

As for the snapshot test, think we could leave it alone here. Once PR #516 gets merged, we should be able to update it with the new tests there.