-
Notifications
You must be signed in to change notification settings - Fork 167
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
RichDom2185
wants to merge
13
commits into
master
Choose a base branch
from
side-content-redux2
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Side content redux2 #2874
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
@leeyi45 could you help outline the changes you made on this branch in the PR description? Thanks! |
4 tasks
7 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 havecreateWorkspaceSlice
, which is used to create a new slice for each workspace. You are supposed to tag on any extra reducers you need using thereducers
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
andAssessmentWorkspace
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
andEditor
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
anduseEditor
. These hooks automatically bind action creators to the specificworkspaceLocation
.I think a minor side thing here is that I changed the frontend to load its own tabs