search

magnifydev / magnify

A Digital Program of Studies for LMHS.
https://magnify.web.app
Apache License 2.0
8 stars 0 forks source link

Rewrite? #21

Closed GoogolGenius closed 2 years ago

GoogolGenius commented 2 years ago

A lot of the JS and CSS is quite horrendous if you look at it closely. I myself have had to work with how the CSS was initially written and it's a limiting factor in my opinion. I don't think anything warrants a rewrite yet until people actually use this hopefully in the upcoming school year, definitely v2024.0.0 if it gets to that. We've moved the current infrastructure for JS over to TS so it hopefully scales better. At least a CSS rewrite would be nice for now, redoing the class names, removing redundant properties, etc. I think the JSX needs to be split into more components too.

Ascent817 commented 2 years ago

Yeah I totally agree. I think we should at least fix some of the css. I'm not very good with css though, so if someone else can rewrite that would be good. Also, do you think it's a good idea to break up the course into two components, one for editing and one for viewing? React doesn't like the hacky code I used to get the text out of the courses We really should be using a form and then capturing that data

Ascent817 commented 2 years ago

I can refactor the course editing and viewing if we decide that's something we want to do.

GoogolGenius commented 2 years ago

Also Magnify behaves like a React 17 app still because we're using the ReactDOM.render still

Ascent817 commented 2 years ago

Is there a simple fix for that?

GoogolGenius commented 2 years ago

Also, do you think it's a good idea to break up the course into two components, one for editing and one for viewing? React doesn't like the hacky code I used to get the text out of the courses We really should be using a form and then capturing that data

Definitely I think a modal pop up for editing or whatever would be better. Then we aren't letting people edit the page directly as that messes up with React state and the Virtual DOM.

I think we should at least fix some of the css. I'm not very good with css though, so if someone else can rewrite that would be good.

The sidebar definitely needs a rewrite, the way you manipulated the widths were very weird. Also because it behaved so weirdly I have 2 navbars lol. One for desktop, one for mobile.

GoogolGenius commented 2 years ago

Is there a simple fix for that?

I'm not sure, the way React creates the root element or whatever is different in react 18. You might want to look at how it's setup by create-react-app in my website code https://github.com/GoogleGenius/erich

GoogolGenius commented 2 years ago

I'm going to close this issue and open several smaller ones instead, so we have a better workable step-by-step goal.