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

Code refactor #6

Open
6 of 7 tasks
n-d-r-d-g opened this issue Oct 17, 2024 · 2 comments
Open
6 of 7 tasks

Code refactor #6

n-d-r-d-g opened this issue Oct 17, 2024 · 2 comments

Comments

@n-d-r-d-g
Copy link
Member

n-d-r-d-g commented Oct 17, 2024

The current code has the following issues:

  1. Prop drilling
  2. Template select options are hard coded
  3. Some components accept CSS as props instead of className
  4. Timing logic is hard coded

My suggestions are as follows:

  1. Use React Context to avoid prop drilling (can be implemented for meetup details as well as templates). The code will be cleaner and more maintainable.
  2. Loop in the TEMPLATE_CONFIGS object to display each template option.
  3. Rewrite/Remove these components as needed. CSS should be passed as className. Data should be passed as props.
  4. The timing logic has to be dynamic and magic numbers should be avoided. We can name variables/constants properly to ease readability. Each template config can have a timing calculator function that takes the meetup details as argument.

Other improvements:

  • Move DefaultTemplate from src/templates to src/components as IMHO src/templates should contain only template components.
  • Rename DefaultTemplate to DynamicTemplate.
  • Rename the meetup parameter at src/components/Root.tsx on line 54 to meetupDetails.
  • Move Root from src/components to src as src/components should contain reusable components.
  • Rename Root to App to match React project conventions.
  • Create an interface for TEMPLATE_CONFIGS objects to ensure type safety while adding/modifying TEMPLATE_CONFIGS.
  • Modify instructions in the README.
@derecklhw
Copy link
Contributor

@n-d-r-d-g sorry for prop drilling XD

@n-d-r-d-g
Copy link
Member Author

@derecklhw I did it too! We needed to make it work.
That's our excuse! 😜

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

No branches or pull requests

2 participants