From 6a5f586f738e71fac44fb1f354b5c645f43eebeb Mon Sep 17 00:00:00 2001 From: Oliver Eyton-Williams Date: Mon, 18 Oct 2021 13:58:06 +0200 Subject: [PATCH] fix: handle challenge resets through redux (#43843) Instead of relying on heuristics like "does the current content differ from the initial content?" this just resets if the reset button has been pressed. --- .../templates/Challenges/classic/editor.tsx | 40 +++++++++++-------- .../Challenges/redux/action-types.js | 1 + .../src/templates/Challenges/redux/index.js | 10 ++++- 3 files changed, 34 insertions(+), 17 deletions(-) diff --git a/client/src/templates/Challenges/classic/editor.tsx b/client/src/templates/Challenges/classic/editor.tsx index 6d987d5f2a..d8d8b9379c 100644 --- a/client/src/templates/Challenges/classic/editor.tsx +++ b/client/src/templates/Challenges/classic/editor.tsx @@ -38,7 +38,9 @@ import { updateFile, challengeTestsSelector, submitChallenge, - initTests + initTests, + isResettingSelector, + stopResetting } from '../redux'; import './editor.css'; @@ -61,11 +63,13 @@ interface EditorProps { initTests: (tests: Test[]) => void; initialTests: Test[]; isProjectStep: boolean; + isResetting: boolean; output: string[]; resizeProps: ResizePropsType; saveEditorContent: () => void; setEditorFocusability: (isFocusable: boolean) => void; submitChallenge: () => void; + stopResetting: () => void; tests: Test[]; theme: string; title: string; @@ -104,16 +108,19 @@ const mapStateToProps = createSelector( canFocusEditorSelector, consoleOutputSelector, isDonationModalOpenSelector, + isResettingSelector, userSelector, challengeTestsSelector, ( canFocus: boolean, output: string[], open, + isResetting: boolean, { theme = 'default' }: { theme: string }, tests: [{ text: string; testString: string }] ) => ({ canFocus: open ? false : canFocus, + isResetting, output, theme, tests @@ -128,7 +135,8 @@ const mapDispatchToProps = { setEditorFocusability, updateFile, submitChallenge, - initTests + initTests, + stopResetting }; const modeMap = { @@ -281,7 +289,7 @@ const Editor = (props: EditorProps): JSX.Element => { // Updates the model if the contents has changed. This is only necessary for // changes coming from outside the editor (such as code resets). - const updateEditorValues = () => { + const resetEditorValues = () => { const { challengeFiles, fileKey } = props; const { model } = data; @@ -290,9 +298,6 @@ const Editor = (props: EditorProps): JSX.Element => { )?.contents; if (model?.getValue() !== newContents) { model?.setValue(newContents ?? ''); - return true; - } else { - return false; } }; @@ -1001,21 +1006,24 @@ const Editor = (props: EditorProps): JSX.Element => { // editor. const { editor } = data; - const hasChangedContents = updateEditorValues(); - if (hasChangedContents && hasEditableRegion()) { - initializeDescriptionAndOutputWidgets(); - updateDescriptionZone(); - updateOutputZone(); - } - - if (hasChangedContents) { + if (props.isResetting) { + // NOTE: this looks a lot like a race condition, since stopResetting gets + // called in each editor and changes isResetting. However, all open editor + // are rendered in a batch (before stopResetting talks to redux), so they + // all get to this point. Also stopResetting is idempotent, so it doesn't + // matter that each editor calls it. + props.stopResetting(); + resetEditorValues(); focusIfTargetEditor(); } if (props.initialTests) initTests(props.initialTests); if (hasEditableRegion() && editor) { - if (hasChangedContents) { + if (props.isResetting) { + initializeDescriptionAndOutputWidgets(); + updateDescriptionZone(); + updateOutputZone(); showEditableRegion(editor); } // resetting test output @@ -1048,7 +1056,7 @@ const Editor = (props: EditorProps): JSX.Element => { } } // eslint-disable-next-line react-hooks/exhaustive-deps - }, [props.challengeFiles]); + }, [props.challengeFiles, props.isResetting]); useEffect(() => { const { output } = props; const editableRegion = getEditableRegionFromRedux(); diff --git a/client/src/templates/Challenges/redux/action-types.js b/client/src/templates/Challenges/redux/action-types.js index 368063fada..d938afde03 100644 --- a/client/src/templates/Challenges/redux/action-types.js +++ b/client/src/templates/Challenges/redux/action-types.js @@ -36,6 +36,7 @@ export const actionTypes = createTypes( 'checkChallenge', 'executeChallenge', 'resetChallenge', + 'stopResetting', 'submitChallenge', 'moveToTab', diff --git a/client/src/templates/Challenges/redux/index.js b/client/src/templates/Challenges/redux/index.js index cf2c1f3a09..8639edbc99 100644 --- a/client/src/templates/Challenges/redux/index.js +++ b/client/src/templates/Challenges/redux/index.js @@ -33,6 +33,7 @@ const initialState = { hasCompletedBlock: false, isCodeLocked: false, isBuildEnabled: true, + isResetting: false, logsOut: [], modal: { completion: false, @@ -117,6 +118,7 @@ export const challengeMounted = createAction(actionTypes.challengeMounted); export const checkChallenge = createAction(actionTypes.checkChallenge); export const executeChallenge = createAction(actionTypes.executeChallenge); export const resetChallenge = createAction(actionTypes.resetChallenge); +export const stopResetting = createAction(actionTypes.stopResetting); export const submitChallenge = createAction(actionTypes.submitChallenge); export const moveToTab = createAction(actionTypes.moveToTab); @@ -146,6 +148,7 @@ export const isCompletionModalOpenSelector = state => export const isHelpModalOpenSelector = state => state[ns].modal.help; export const isVideoModalOpenSelector = state => state[ns].modal.video; export const isResetModalOpenSelector = state => state[ns].modal.reset; +export const isResettingSelector = state => state[ns].isResetting; export const isBuildEnabledSelector = state => state[ns].isBuildEnabled; export const successMessageSelector = state => state[ns].successMessage; @@ -295,9 +298,14 @@ export const reducer = handleActions( text, testString })), - consoleOut: [] + consoleOut: [], + isResetting: true }; }, + [actionTypes.stopResetting]: state => ({ + ...state, + isResetting: false + }), [actionTypes.updateSolutionFormValues]: (state, { payload }) => ({ ...state, projectFormValues: payload