search

rightoneducation / righton-app

React Native mobile app & React web app
Apache License 2.0
21 stars 11 forks source link

[Host_v2] - PR Code Review Cheatsheet #1154

Open drewjhart opened 3 months ago

drewjhart commented 3 months ago

Hi Mani, Because there's a ton of PRs for host_v2, I've put together this brief review guide so you don't have to wade through them all unprepared. It's broken down by section and then you can find a list of the PRs at the bottom with some additional notes.

If you would like to see host_v2 in action, it has been built on our dev backend environment and is accessible via:

dev-central.rightoneducation.com. (just create a game using central and it will start it up in host_v2)

Major changes from host_v1:

1. HostDataManager One major pain point for host_v1 was that it relied on a series of graphics that all need to be kept updated through the various phases of a game. In host_v1 we kept strictly to the fact that the GameSession was our single source of truth and we then derived and calculated the various data off that GameSession as answer came in. This was all done in one gigantic GameSessionContainer, through subscriptions initialized in one unwieldly useEffect.

To improve this, I am starting with the assumption that host_v2 main job is to intake Answer data, and use it to maintain a stable TeamAnswer object. This object will be static through the course of a game, and populate as is relevant with new answers, confidences and hints. It will then be sent up to the GameSession object in one shot, so we can easily have a highly organized, single object of a game session for future analytics.

Therefore, I have abstracted away all the processing required for this object in a separate class called HostDataManager (there is also an equivalent for play). There are functions within this layer for building the initial empty HostTeamAnswers object, for establishing subscriptions to team answer create and update, and for populating and parsing the HostTeamAnswers object.

We've talked about this abstracted data layer previously, and this is the current implementation. It's done a lot for keeping our codebase clean, provided it's fairly understandable in and of itself.

You can find the hostTeamDataManager here: https://github.com/rightoneducation/righton-app/blob/drew-hart--hostv2animations/networking/src/APIClients/datamanagers/HostDataManagerAPIClient.ts

And you find the general shape of the HostTeamAnswers object when it is initialized in the above file on ln 317 for short answer questions and ln 345 for multiple choice questions

2. Context/Reducer: Another pain point was that because all of our data processing was being handling near the top of the component tree, we then had to prop drill like crazy to get down to our components. Often times, this meant having dozens of props in components, all that were just being passed down to children.

To improve this, we have added context/reducer to our code. We are storing both the GameSession and the LocalHostTeamAnswers in context and distributing them into the components that directly manipulate them. This implementation (of being selective in what to capture in context, and deploying it as near as possible to the children components) aligns with the React docs guidance per: https://react.dev/learn/scaling-up-with-reducer-and-context[https://react.dev/learn/scaling-up-with-reducer-and-context](https://react.dev/learn/scaling-up-with-reducer-and-context[https://react.dev/learn/scaling-up-with-reducer-and-context)

This has helped us greatly reduce the number of props we are drilling down. The trade-off is that it is a bit harder to keep track of where context is being used.

3. General improvements to the codebase that follow play: Host_v1 was written prior to play, so we are carrying through the upgrades we made to our codebase when we wrote play. This includes the following:

Following all this ensures that our codebase is more consistent across apps and in terms of user experience.

Detailed PR Breakdown Here's how the broad topics above carry down into the actual PRs (so you can more quickly parse these PRs):

These PRs represent preliminary work or PRs that you have already reviewed. I think it's safe to skip these:

Game Flow PRs that outline the various states that host_v2 will move through and the organization of UI components per state

End Game and Leaderboard screens (new to host_v2):

Short Answer Response required some additional updates to ensure that the Featured Mistakes component both received matching answers accurately, and provided Selected Answers to Phase 2 properly.

Moved from hardcoding a game session to using the routing from host_v1 (storing gamecode and gamesessionID in the url). Fixed the timer and ensured data was stable between screen refreshes. Fixes is associated misc bug and quality fixes

Currently WIP, adding screen wide transition animations between phases using the framer-motion library

Once this and some code cleanup is done, I will have a final PR that uses prettier and lints the code.

drewjhart commented 3 months ago

Additionally, I have added this PR: [Host_v2] - Context/Subscription Improvements #1158

Which simplifies our subscription and context logic per the overview on our call. Basically, we don't have backend/local gameSessions and hostTeamAnswers, we just do all over that processing on a single set of variables (that are held in Context) in the custom hook. This really cleans up the flow of the code and prevents a bunch of stale state that we were running into previously.