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

Context Rework #298

Merged
merged 6 commits into from
Oct 10, 2024
Merged

Context Rework #298

merged 6 commits into from
Oct 10, 2024

Conversation

KalebE36
Copy link
Contributor

@KalebE36 KalebE36 commented Oct 2, 2024

Closes #235

Hey,

I made the requested changes that were listed in the issue. I believe I made the code a bit cleaner and a bit more maintainable. You guys could probably make it even more modularized but this is okay for now. Let me know what you guys think.

My changes are ⬇️:

1. LocationContext

  • Modularized Location Management:
    • Extracted location fetching logic into a dedicated service (locationService.ts), removing it from the context to adhere to separation of concerns.
    • Simplified the LocationProvider to handle only state management, with the location fetching and permissions logic handled by external services.

2. SettingsContext

  • Refactored Settings Handling:
    • Moved theme loading and saving logic to a utility file (storageUtils.ts), separating storage concerns from context management.
    • Simplified SettingsProvider by initializing the state through utility functions and directly handling the toggle logic without redundant state reloading.

3. SocketContext

  • Modularized Socket Logic:
    • Moved socket initialization to a dedicated service (SocketService.ts) that handles the setup and connection of the Socket.IO client.
    • Moved token retrieval logic to a the SocketService, centralizing the authentication concerns.
    • Refactored location update emission also to SocketService, isolating the responsibility of sending location updates to the server.

4. UserContext

  • Modularized User Initialization:
    • Moved the user initialization logic, including generateName() and the default user object setup, to a separate service file (UserService.ts).
    • Simplified UserProvider by using the initializeUser function from the service to set the default state directly.

@fmcubium fmcubium requested a review from h1divp October 8, 2024 21:39
Copy link
Collaborator

@h1divp h1divp left a comment

Choose a reason for hiding this comment

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

Really good job here. I love the way things are compartmentalized now, and in fact I had no issues getting the server and client to connect while testing. Maybe this fixes the major bug we were having with linking the front and backend?

Anyways, I don't see any issues with the code as of now, but if bugs pop up later issues will be made. Thank you for your work.

@h1divp
Copy link
Collaborator

h1divp commented Oct 10, 2024

Check failed because of dependency issues for some reason. I'm suspecting that's because of the recent dependency cleanup... someone please ping a tech lead on discord if for some reason the app can't start because of the dependencies.

@h1divp h1divp merged commit e70ef82 into ufosc:main Oct 10, 2024
2 of 3 checks passed
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.

Rework all the contexts for clarity
2 participants