-
Notifications
You must be signed in to change notification settings - Fork 172
Store library externals in state #198
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
Conversation
- createDefaultWorkspace has only 1 argument (leave the library and chapter setting to CLEAR_CONTEXT) - IWorkspaceState contains externals
I.e in both components and containers
Pull Request Test Coverage Report for Build 309
💛 - Coveralls |
To include clearContext instead of changeChapter, and To utilise implementing an IStateProps
In Playground and ControlBar, rename all terms iwth Library to external/externals
And also remove unneeded mutable variables
src/actions/actionTypes.ts
Outdated
export const CHANGE_SIDE_CONTENT_HEIGHT = 'CHANGE_SIDE_CONTENT_HEIGHT' | ||
export const CHAPTER_SELECT = 'CHAPTER_SELECT' | ||
export const LIBRARY_SELECT = 'LIBRARY_SELECT' | ||
export const PLAYGROUND_EXTERNAL_SELECT = 'PLAYGROUND_EXTERNAL_SELECT ' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alphabetical pls
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also can remove this one line of comment https://github.com/source-academy/cadet-frontend/pull/198/files#diff-8904fb3dde411c91547eeb27d8ae6bb3R121
Also sorted some other actionTypes lines
Features
js-slang
.changeChapter
changeLibrary
tochangePlaygroundExternal
clearContext
is used to reset the context and set chapter and externals (previously library). This is because changing chapter or library will require the context to be cleared.