Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Split App component into multiple smaller components #68

Conversation

diogotcorreia
Copy link
Contributor

@diogotcorreia diogotcorreia commented Oct 1, 2022

Also changes them to be functional components instead of class components

A few behaviors that I've changed:

  • When deleting a timetable that's not the active one, it now stays in that timetable instead of going to the first one
  • Deleting a timetable now shows an alert on success

Things under the hood that have been changed:

  • ShiftRef was deleted (didn't seem to be doing anything)
  • Derived state is no longer calculated on state update, but on render (and memoized). This makes it much simpler to update state
  • Domain classes Timetable, Course and Shift are now fully immutable
  • CourseUpdate class was deleted (was just adding unneeded complexity)
  • I've tried to make sure there is only one source of truth across the app, so we don't have outdated data spread across components

@joaocmd joaocmd mentioned this pull request Nov 19, 2022
@diogotcorreia
Copy link
Contributor Author

diogotcorreia commented Nov 20, 2022

Almost done, just need to fix a few things I broke in the process!

  • Change NewTimetable to functional component
  • Add colors to excel export
  • Add colors to image export
  • Fix duplicate schedule button
  • Fix shift classes button
  • Fix console errors
  • Replace withStyles with useStyles on App.tsx
  • Fix // TODOs added

@diogotcorreia diogotcorreia marked this pull request as ready for review November 20, 2022 21:05
@diogotcorreia
Copy link
Contributor Author

I think this is done, and it has feature parity with the production version.
I've updated the top comment with some information.

I have a few more things that I want to change, but I'll open issues instead and do them on another PR, since this is huge already.
Consider squashing this instead of merging.

@joaocmd
Copy link
Collaborator

joaocmd commented Dec 11, 2022

@all-contributors please add @diogotcorreia for code

@allcontributors
Copy link
Contributor

@joaocmd

I've put up a pull request to add @diogotcorreia! 🎉

@joaocmd joaocmd merged commit 34edc7d into Criador-Horarios:master Jan 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants