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

Side content redux2 #2874

Draft
wants to merge 13 commits into
base: master
Choose a base branch
from
Draft

Side content redux2 #2874

wants to merge 13 commits into from

Conversation

RichDom2185
Copy link
Member

@RichDom2185 RichDom2185 commented Mar 25, 2024

The point of this proof of concept was to move to using Redux Toolkit. The main goal was to make the reducers, actions and sagas as workspace agnostic as possible.

Common to all workspaces is a set of state and actions. Under src/commons/redux/workspaces/WorkspaceRedux.ts, we have createWorkspaceSlice, which is used to create a new slice for each workspace. You are supposed to tag on any extra reducers you need using the reducers parameter. Your custom reducers are responsible for the state you define, but the rest gets handled automatically by the default set of reducers.

There is however more than 1 base type of workspace. In the process of working on this, I identified the PlaygroundWorkspace and AssessmentWorkspace types (both of which could do with better names). Both of these extend the idea as above: provide a base set of reducers and actions, but the user is free to add anything else they require.

The other main change that was made was to refactor out the Repl and Editor objects. Though each workspace has to make use of both, I thought it better to group the actions and state related to these two objects.

The separation of the state allows us to make use of hooks: every component need only useWorkspace, useRepl and useEditor. These hooks automatically bind action creators to the specific workspaceLocation.

I think a minor side thing here is that I changed the frontend to load its own tabs

@RichDom2185
Copy link
Member Author

@leeyi45 could you help outline the changes you made on this branch in the PR description? Thanks!

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.

2 participants