mikhael28 / paretOS

A high-level operating system to maximize human potential. Live at https://paret0.com
https://paret0.com
GNU General Public License v2.0
35 stars 45 forks source link

Refactor: Migrate react-bootstrap Button component to MaterialUI (multiple PRs accepted!) #177

Closed jayeclark closed 2 years ago

jayeclark commented 2 years ago

Description

Several components are still using the react-bootstrap Button component and need to be migrated to the @mui/material Button component.

If you would like to help address one (or more) of these, please respond with which you'd like to claim and I'll update the file. Newcomers are most welcome!

(When migrating, please make sure to remove any old Bootstrap props that are no longer relevant and set any new props that are needed by the Material UI component.)

React Bootstrap Button docs: https://react-bootstrap.github.io/components/buttons/ MaterialUI Button docs: https://mui.com/components/buttons/

Already completed

arena/CreateSprintTemplate, arena/Sprint, arena/SprintCreation

Files still needing to be updated to remove Bootstrap

(In most cases the Button component is used only once or twice in the file, it's definitely possible to knock out a few of these fairly quickly!)

Files needing to be updated to add MaterialUI Button (replace

Ram-dev27 commented 2 years ago

Hi @jayeclark , Can we create the custom component for the button (like LoaderButton)and we can use that custom button all over the application, can we go with that approach ?

mikhael28 commented 2 years ago

@Ram-dev27 the Material UI Button functionality is already pretty well abstracted, in the sense that you can specify different types of input, styling, size, etc. - what attributes do you feel are missing from Material UI's button implementation that you think would be a value add across the codebase?

annovo commented 2 years ago

Hi, if this is still available I can try out to replace buttons in the arena folder and maybe a few others if nobody claims them? I also didn't find arena/ArenaProofModal file

jayeclark commented 2 years ago

@annovo That would be amazing! I'll mark off the arena ones for you now. Re: arena/ArenaProofModal - sorry, looks like that's a typo on my end. It's actually components/ArenaProofModal

annovo commented 2 years ago

So I was doing refactoring and I have several questions:

  1. options-buttons in arena/Sprint that allow you to go to a specific day. They are gray and hovering makes them lighter gray. But when the button is clicked it has ambiguous behavior: if the pointer is still on it - it's still gray; when you move it out it became a gradient pinkish color (like the other button in the section) but it doesn't keep the color after clicking anywhere else. As I understand it, a focus state makes it gradient-like but doesn't keep the color. Is this intentional? If not, then what should it look like?
  2. I can't access the "admin" buttons that are in the same file below the "options" buttons. They change status to "active" etc. So I'm looking for a reference pic regarding their style. UPD. nevermind, I missed the part that I already have them in the project and can change them back lol. I see they are the same as options and have the same behavior
  3. I used makeStyles to create useStyles hook and used it across different components in the arena. (I think it may also be used in different places such as ArenaProofModal). For this, I made a separate file and put it in libs dir. Is it okay or should I move it somewhere else?
jayeclark commented 2 years ago
  1. Definitely not intentional… Those buttons were intended to operate differently from the standard button, to indicate a different type of functionality (selecting among options) from the standard ‘clicking a button submits or resets a form’ behavior. I’m definitely very open to suggestions - even a well-formulated argument that they should operate like regular buttons, or like the left-hand cards in the ‘experience’ module. My original thought was that they should be a darker gray, lighter gray when hovered and remain that same lighter gray when selected (visually indicating which day is being shown). Rather than creating a new button variant in theme, the Chip component of MUI might achieve the same functionality, it certainly has a very similar appearance and fulfills a similar function!
  2. I believe there’s also some code you can temporarily change in order to convince the front end that you are an admin. :)
  3. If it’s a style that’s being used repeatedly, it might be better to define a custom variant in theme.js, that would make it easier for new contributors/people less familiar with MUI to replicate the style with just the Button import and setting one prop.
annovo commented 2 years ago
  1. Okay, got it, thanks! The Chip component does look similar. The question is would you like it to be the same as the previous or like the default Chip? For example, outlined while not clicked and filled when clicked? Here is the reference (I will make them bigger regardless) Screen Shot 2022-04-13 at 11 37 23 AM
  2. Yep, figured it out, but got stuck with how to get the previous states, all good now, thanks!
  3. I put the MuiButton in theme.components and it affected the other (possibly undesired) buttons. Like custom LoaderButton in SprintCreation. It was supposed to be grey when inactive but became gradient. Do you have any suggestions on where would it be appropriate to put this gradient styling?
jayeclark commented 2 years ago
  1. I'd be happy to go with whatever you think looks best!
  2. I think it would be best to add it to the theme as a custom variant for MuiButton called "gradient" - similar to how the 'filled' variant for MuiPaper is implemented in the theme. That way, the styles shouldn't affect any other buttons that are a different variant, and all that needs to be done to make sure an instance of <Button> has the gradient is to pass the variant="gradient" prop to it.
annovo commented 2 years ago

Got it, makes sense, thank you :)

sumitsaurabh927 commented 2 years ago

Sure, I'd get started with this!

mikhael28 commented 2 years ago

@sumitsaurabh927 - I've assigned you! Any questions so far?

sumitsaurabh927 commented 2 years ago

Yes, How do I find which classes of index.css have been used. I've looked in app.js and found a bunch of classes that are in index.css but not used in app.js. Do I have to look somewhere else? If yes, where?

mikhael28 commented 2 years ago

I've gone ahead and worked through, and replaced instances of Button, and Image throughout the repository! That being said, it would be good to update the css class for .btn to bring that in-line with our vision for Buttons. @sumitsaurabh927 make sure to pull the latest changes from main, sorry if I stole your thunder a little bit here 😓 I just got the impulse to remove a bunch of these imports so that we can focus on getting the forms migrated!