Skip to content
This repository was archived by the owner on Feb 12, 2025. It is now read-only.

chore: change files structure #1286

Merged
merged 4 commits into from
Jan 30, 2025

Conversation

shootermv
Copy link
Contributor

@shootermv shootermv commented Jan 15, 2025

Summary of changes

the purpose of this change is to make the code more readable and convenient for developer...

Checklist

  • I have read and followed the contribution guidelines.
  • I have read through the Code of Conduct and agree to abide by the rules.
  • This PR is for one of the available issues and is not a PR for an issue already assigned to someone else.
  • My PR title has a short descriptive name so the maintainers can get an idea of what the PR is about.
  • I have provided a summary of my changes.

closes #1285

@shootermv shootermv force-pushed the better-files-structure branch 2 times, most recently from 03f9ae3 to f442462 Compare January 15, 2025 12:57
@shootermv shootermv force-pushed the better-files-structure branch from f442462 to 00ebad9 Compare January 15, 2025 13:11
Copy link
Contributor

@jdwilkin4 jdwilkin4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way ScrollToTop is currently structured doesn't make it a hook based on the definition set by the official react docs

https://react.dev/learn/reusing-logic-with-custom-hooks

right now it is just a custom component not a hook and should stay in the components folders

@shootermv
Copy link
Contributor Author

The way ScrollToTop is currently structured doesn't make it a hook based on the definition set by the official react docs

https://react.dev/learn/reusing-logic-with-custom-hooks

right now it is just a custom component not a hook and should stay in the components folders

actually this hook (or component) is not used anywhere in the code, as far as i can see
it looks to me more like custom hook because there is no jsx returned... but it may be a camponent as well

@jdwilkin4
Copy link
Contributor

Looking through the history is looks like the ScrollToTop was used in the early versions of the app.tsx file but later removed.
So it looks like that can be removed completely.

As for that others changes, I don't see any reason to remove the App.tsx file and move everything into the index.tsx file.
Those changes should be reverted.

@shootermv
Copy link
Contributor Author

shootermv commented Jan 23, 2025 via email

@jdwilkin4
Copy link
Contributor

The current QuizTemplate has WAY to much going on and has become bloated over the past few years.
That is a whole other separate issue.

As for this PR, we should do the following

  • add back in the App.tsx to the way it was.
  • revert the changes for the index.tsx file
  • remove the ScrollToTop since it is not being used
  • revert the changes for the rename of the QuizTemplate

once things have been properly refactored in separate issues to make things more maintainable, then the renaming will make sense

@shootermv
Copy link
Contributor Author

shootermv commented Jan 23, 2025 via email

@jdwilkin4
Copy link
Contributor

can i move the QuizTemplate out of components folder? (since it is
different from Button or Modal reusable componments)

sure

@shootermv
Copy link
Contributor Author

shootermv commented Jan 23, 2025

did the requested changes:

  • add back in the App.tsx to the way it was.
  • revert the changes for the index.tsx file
  • remove the ScrollToTop since it is not being used anywhere
  • rename the QuizTemplate component back to QuizTemplate (instead of Main)

@shootermv
Copy link
Contributor Author

is there some more things to be fixed?

@jdwilkin4 jdwilkin4 merged commit 32d9e25 into freeCodeCamp:main Jan 30, 2025
4 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug/Refactor] - better file structuring
2 participants