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

feat: add vscode slice for message passing with extension #3080

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions src/commons/application/Application.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
import React from 'react';
import { useDispatch } from 'react-redux';
import { Outlet } from 'react-router-dom';
import Messages, { MessageType, sendToWebview } from 'src/features/vscode/messages';

import NavigationBar from '../navigationBar/NavigationBar';
import Constants from '../utils/Constants';
import { useLocalStorageState, useSession } from '../utils/Hooks';
import WorkspaceActions from '../workspace/WorkspaceActions';
import { defaultWorkspaceSettings, WorkspaceSettingsContext } from '../WorkspaceSettingsContext';
import SessionActions from './actions/SessionActions';
import VscodeActions from './actions/VscodeActions';

const Application: React.FC = () => {
const dispatch = useDispatch();
Expand Down Expand Up @@ -70,6 +73,47 @@
};
}, [isPWA, isMobile]);

// Effect to fetch the latest user info and course configurations from the backend on refresh,
// if the user was previously logged in
React.useEffect(() => {
// Polyfill confirm() to instead show as VSCode notification
window.confirm = () => {
console.log('You gotta confirm!');
return true;
};
Comment on lines +80 to +83
Copy link
Member

Choose a reason for hiding this comment

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

Should we use if (window.confirm) ... instead?


const message = Messages.WebviewStarted();
sendToWebview(message);

window.addEventListener('message', event => {
const message: MessageType = event.data;
// Only accept messages from the vscode webview
if (!event.origin.startsWith('vscode-webview://')) {
return;
Copy link
Member

Choose a reason for hiding this comment

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

I know this is a guard, but shall we log such an event?

Technically we might be able to configure our deployment with X-Frame-Options: DENY to prevent embedding but let's look into it another time

}
// console.log(`FRONTEND: Message from ${event.origin}: ${JSON.stringify(message)}`);
switch (message.type) {
case 'WebviewStarted':
console.log('Received WebviewStarted message, will set vsc');
dispatch(VscodeActions.setVscode());
break;
case 'Text':
const code = message.code;
console.log(`FRONTEND: TextMessage: ${code}`);
// TODO: Don't change ace editor directly
// const elements = document.getElementsByClassName('react-ace');
// if (elements.length === 0) {
// return;
// }
// // @ts-expect-error: ace is not available at compile time
// const editor = ace.edit(elements[0]);
// editor.setValue(code);
dispatch(WorkspaceActions.updateEditorValue('assessment', 0, code));
break;
}
});
}, []);

Check warning on line 115 in src/commons/application/Application.tsx

View workflow job for this annotation

GitHub Actions / lint (eslint)

React Hook React.useEffect has a missing dependency: 'dispatch'. Either include it or remove the dependency array

return (
<WorkspaceSettingsContext.Provider value={[workspaceSettings, setWorkspaceSettings]}>
<div className="Application">
Expand Down
9 changes: 8 additions & 1 deletion src/commons/application/ApplicationTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
import { RouterState } from './types/CommonsTypes';
import { ExternalLibraryName } from './types/ExternalTypes';
import { SessionState } from './types/SessionTypes';
import { VscodeState as VscodeState } from './types/VscodeTypes';

export type OverallState = {
readonly router: RouterState;
Expand All @@ -31,6 +32,7 @@ export type OverallState = {
readonly dashboard: DashboardState;
readonly fileSystem: FileSystemState;
readonly sideContent: SideContentManagerState;
readonly vscode: VscodeState;
};

export type Story = {
Expand Down Expand Up @@ -604,6 +606,10 @@ export const defaultSideContentManager: SideContentManagerState = {
stories: {}
};

export const defaultVscode: VscodeState = {
isVscode: false
};

export const defaultState: OverallState = {
router: defaultRouter,
achievement: defaultAchievement,
Expand All @@ -613,5 +619,6 @@ export const defaultState: OverallState = {
stories: defaultStories,
workspaces: defaultWorkspaceManager,
fileSystem: defaultFileSystem,
sideContent: defaultSideContentManager
sideContent: defaultSideContentManager,
vscode: defaultVscode
};
10 changes: 10 additions & 0 deletions src/commons/application/actions/VscodeActions.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { createActions } from 'src/commons/redux/utils';

const VscodeActions = createActions('vscode', {
setVscode: () => ({})
heyzec marked this conversation as resolved.
Show resolved Hide resolved
});

// For compatibility with existing code (actions helper)
export default {
...VscodeActions
};
4 changes: 3 additions & 1 deletion src/commons/application/reducers/RootReducer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { WorkspaceReducer as workspaces } from '../../workspace/WorkspaceReducer
import { OverallState } from '../ApplicationTypes';
import { RouterReducer as router } from './CommonsReducer';
import { SessionsReducer as session } from './SessionsReducer';
import { VscodeReducer as vscode } from './VscodeReducer';

const rootReducer: Reducer<OverallState, SourceActionType> = combineReducers({
router,
Expand All @@ -21,7 +22,8 @@ const rootReducer: Reducer<OverallState, SourceActionType> = combineReducers({
stories,
workspaces,
fileSystem,
sideContent
sideContent,
vscode
});

export default rootReducer;
20 changes: 20 additions & 0 deletions src/commons/application/reducers/VscodeReducer.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { createReducer, Reducer } from '@reduxjs/toolkit';

import { SourceActionType } from '../../utils/ActionsHelper';
import VscodeActions from '../actions/VscodeActions';
import { defaultVscode } from '../ApplicationTypes';
import { VscodeState } from '../types/VscodeTypes';

export const VscodeReducer: Reducer<VscodeState, SourceActionType> = (
state = defaultVscode,
action
) => {
state = newVscodeReducer(state, action);
return state;
};

const newVscodeReducer = createReducer(defaultVscode, builder => {
builder.addCase(VscodeActions.setVscode, state => {
return { ...state, ...{ isVscode: true } };
});
});
3 changes: 3 additions & 0 deletions src/commons/application/types/VscodeTypes.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export type VscodeState = {
isVscode: boolean;
};
56 changes: 30 additions & 26 deletions src/commons/assessmentWorkspace/AssessmentWorkspace.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import { useNavigate } from 'react-router';
import { showSimpleConfirmDialog } from 'src/commons/utils/DialogHelper';
import { onClickProgress } from 'src/features/assessments/AssessmentUtils';
import Messages, { sendToWebview } from 'src/features/vscode/messages';
import { mobileOnlyTabIds } from 'src/pages/playground/PlaygroundTabs';

import { initSession, log } from '../../features/eventLogging';
Expand Down Expand Up @@ -184,11 +185,11 @@
};
}, [dispatch]);

useEffect(() => {
// TODO: Hardcoded to make use of the first editor tab. Refactoring is needed for this workspace to enable Folder mode.
handleEditorValueChange(0, '');
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);
// useEffect(() => {
// // TODO: Hardcoded to make use of the first editor tab. Refactoring is needed for this workspace to enable Folder mode.
// handleEditorValueChange(0, '');
// // eslint-disable-next-line react-hooks/exhaustive-deps
// }, []);
Comment on lines +188 to +192
Copy link
Member

Choose a reason for hiding this comment

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

Will this break anything with assessments? Have you tested locally?

Copy link
Author

Choose a reason for hiding this comment

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

From my local testing, there's no unintended behavior. Assessments still loading as per normal.

handleEditorValueChange is ultimately called with the editor contents due to the useEffect here.

Copy link
Member

Choose a reason for hiding this comment

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

Let's just delete and clean up all this unused code then


useEffect(() => {
if (assessmentOverview && assessmentOverview.maxTeamSize > 1) {
Expand Down Expand Up @@ -220,28 +221,28 @@
if (!assessment) {
return;
}
// ------------- PLEASE NOTE, EVERYTHING BELOW THIS SEEMS TO BE UNUSED -------------
// checkWorkspaceReset does exactly the same thing.
let questionId = props.questionId;
if (props.questionId >= assessment.questions.length) {
questionId = assessment.questions.length - 1;
}

const question = assessment.questions[questionId];

let answer = '';
if (question.type === QuestionTypes.programming) {
if (question.answer) {
answer = (question as IProgrammingQuestion).answer as string;
} else {
answer = (question as IProgrammingQuestion).solutionTemplate;
}
}

// TODO: Hardcoded to make use of the first editor tab. Refactoring is needed for this workspace to enable Folder mode.
handleEditorValueChange(0, answer);
// eslint-disable-next-line react-hooks/exhaustive-deps
// // ------------- PLEASE NOTE, EVERYTHING BELOW THIS SEEMS TO BE UNUSED -------------
// // checkWorkspaceReset does exactly the same thing.
// let questionId = props.questionId;
// if (props.questionId >= assessment.questions.length) {
// questionId = assessment.questions.length - 1;
// }

// const question = assessment.questions[questionId];

// let answer = '';
// if (question.type === QuestionTypes.programming) {
// if (question.answer) {
// answer = (question as IProgrammingQuestion).answer as string;
// } else {
// answer = (question as IProgrammingQuestion).solutionTemplate;
// }
// }

// // TODO: Hardcoded to make use of the first editor tab. Refactoring is needed for this workspace to enable Folder mode.
// handleEditorValueChange(0, answer);
// // eslint-disable-next-line react-hooks/exhaustive-deps
}, []);

Check warning on line 245 in src/commons/assessmentWorkspace/AssessmentWorkspace.tsx

View workflow job for this annotation

GitHub Actions / lint (eslint)

React Hook useEffect has missing dependencies: 'assessment', 'handleAssessmentFetch', 'props.assessmentId', 'props.needsPassword', 'props.notAttempted', and 'props.questionId'. Either include them or remove the dependency array

/**
* Once there is an update (due to the assessment being fetched), check
Expand Down Expand Up @@ -415,9 +416,12 @@
);
handleClearContext(question.library, true);
handleUpdateHasUnsavedChanges(false);
sendToWebview(Messages.NewEditor(`assessment${assessment.id}`, props.questionId, ''));
if (options.editorValue) {
// TODO: Hardcoded to make use of the first editor tab. Refactoring is needed for this workspace to enable Folder mode.
handleEditorValueChange(0, options.editorValue);
} else {
handleEditorValueChange(0, '');
}
};

Expand Down
4 changes: 3 additions & 1 deletion src/commons/mocks/StoreMocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
defaultSession,
defaultSideContentManager,
defaultStories,
defaultVscode,
defaultWorkspaceManager,
OverallState
} from '../application/ApplicationTypes';
Expand All @@ -30,7 +31,8 @@ export function mockInitialStore(
session: defaultSession,
stories: defaultStories,
fileSystem: defaultFileSystem,
sideContent: defaultSideContentManager
sideContent: defaultSideContentManager,
vscode: defaultVscode
};

const lodashMergeCustomizer = (objValue: any, srcValue: any) => {
Expand Down
4 changes: 3 additions & 1 deletion src/commons/utils/ActionsHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import SourcecastActions from '../../features/sourceRecorder/sourcecast/Sourceca
import SourceRecorderActions from '../../features/sourceRecorder/SourceRecorderActions';
import SourcereelActions from '../../features/sourceRecorder/sourcereel/SourcereelActions';
import StoriesActions from '../../features/stories/StoriesActions';
import VscodeActions from '../application/actions/VscodeActions';
import { ActionType } from './TypeHelper';

export const actions = {
Expand All @@ -38,7 +39,8 @@ export const actions = {
...RemoteExecutionActions,
...FileSystemActions,
...StoriesActions,
...SideContentActions
...SideContentActions,
...VscodeActions
};

export type SourceActionType = ActionType<typeof actions>;
9 changes: 7 additions & 2 deletions src/commons/workspace/Workspace.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { Prompt } from '../ReactRouterPrompt';
import Repl, { ReplProps } from '../repl/Repl';
import SideBar, { SideBarTab } from '../sideBar/SideBar';
import SideContent, { SideContentProps } from '../sideContent/SideContent';
import { useDimensions } from '../utils/Hooks';
import { useDimensions, useTypedSelector } from '../utils/Hooks';

export type WorkspaceProps = DispatchProps & StateProps;

Expand Down Expand Up @@ -44,6 +44,7 @@ const Workspace: React.FC<WorkspaceProps> = props => {
const [contentContainerWidth] = useDimensions(contentContainerDiv);
const [expandedSideBarWidth, setExpandedSideBarWidth] = useState(200);
const [isSideBarExpanded, setIsSideBarExpanded] = useState(true);
const isVscode = useTypedSelector(state => state.vscode.isVscode);

const sideBarCollapsedWidth = 40;

Expand Down Expand Up @@ -222,7 +223,11 @@ const Workspace: React.FC<WorkspaceProps> = props => {
</Resizable>
<div className="row content-parent" ref={contentContainerDiv}>
<div className="editor-divider" ref={editorDividerDiv} />
<Resizable {...editorResizableProps()}>{createWorkspaceInput(props)}</Resizable>
{isVscode ? (
<div style={{ width: '0px' }}>{createWorkspaceInput(props)}</div>
Copy link
Member

Choose a reason for hiding this comment

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

It must still be visible is it? We can't use visibility: hidden? If we can't, then in addition to width: 0, should we also use overflow: hidden?

Copy link
Author

Choose a reason for hiding this comment

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

While visibility: hidden wouldn't work, I think we can completely remove it now that messages are passed directly to the redux store rather than to the editor.

) : (
<Resizable {...editorResizableProps()}>{createWorkspaceInput(props)}</Resizable>
)}
<div className="right-parent" ref={setFullscreenRefs}>
<Tooltip
className="fullscreen-button"
Expand Down
5 changes: 3 additions & 2 deletions src/commons/workspace/WorkspaceActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,9 @@ const newActions = createActions('workspace', {
updateEditorValue: (
workspaceLocation: WorkspaceLocation,
editorTabIndex: number,
newEditorValue: string
) => ({ workspaceLocation, editorTabIndex, newEditorValue }),
newEditorValue: string,
isFromVscode: boolean = false
) => ({ workspaceLocation, editorTabIndex, newEditorValue, isFromVscode }),
setEditorBreakpoint: (
workspaceLocation: WorkspaceLocation,
editorTabIndex: number,
Expand Down
3 changes: 2 additions & 1 deletion src/commons/workspace/__tests__/WorkspaceActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,8 @@ test('updateEditorValue generates correct action object', () => {
payload: {
workspaceLocation: assessmentWorkspace,
editorTabIndex,
newEditorValue
newEditorValue,
isFromVscode: false
}
});
});
Expand Down
4 changes: 4 additions & 0 deletions src/commons/workspace/reducers/editorReducer.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { ActionReducerMapBuilder } from '@reduxjs/toolkit';
import Messages, { sendToWebview } from 'src/features/vscode/messages';

import WorkspaceActions from '../WorkspaceActions';
import { getWorkspaceLocation } from '../WorkspaceReducer';
Expand Down Expand Up @@ -52,6 +53,9 @@ export const handleEditorActions = (builder: ActionReducerMapBuilder<WorkspaceMa
}

state[workspaceLocation].editorTabs[editorTabIndex].value = newEditorValue;
if (!action.payload.isFromVscode) {
sendToWebview(Messages.Text(newEditorValue));
}
})
.addCase(WorkspaceActions.setEditorBreakpoint, (state, action) => {
const workspaceLocation = getWorkspaceLocation(action);
Expand Down
Loading