Skip to content

Commit

Permalink
fix: simple editor without solution not loading (#489)
Browse files Browse the repository at this point in the history
* fix: update initialize to only call required functions

* feat: update asset urls without asset object

* feat: add pagination to select image modal

* fix: lint errors

* chore: update tests

* fix: asset pattern regex match

* feat: update pagination to be button to prevent page skipping

* fix: e.target.error for feedback fields

* fix: failing snapshots

* fix: new simple problem load error
  • Loading branch information
KristinAoki authored Jun 27, 2024
1 parent 0e91373 commit 2678234
Show file tree
Hide file tree
Showing 47 changed files with 635 additions and 404 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -39,19 +39,23 @@ export const setAnswerTitle = ({
};

export const setSelectedFeedback = ({ answer, hasSingleAnswer, dispatch }) => (e) => {
dispatch(actions.problem.updateAnswer({
id: answer.id,
hasSingleAnswer,
selectedFeedback: e.target.value,
}));
if (e.target) {
dispatch(actions.problem.updateAnswer({
id: answer.id,
hasSingleAnswer,
selectedFeedback: e.target.value,
}));
}
};

export const setUnselectedFeedback = ({ answer, hasSingleAnswer, dispatch }) => (e) => {
dispatch(actions.problem.updateAnswer({
id: answer.id,
hasSingleAnswer,
unselectedFeedback: e.target.value,
}));
if (e.target) {
dispatch(actions.problem.updateAnswer({
id: answer.id,
hasSingleAnswer,
unselectedFeedback: e.target.value,
}));
}
};

export const useFeedback = (answer) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ exports[`SolutionWidget render snapshot: renders correct default 1`] = `
/>
</div>
<[object Object]
editorContentHtml="This is my question"
editorContentHtml="This is my solution"
editorType="solution"
id="solution"
minHeight={150}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,20 @@ import { selectors } from '../../../../../data/redux';
import messages from './messages';

import TinyMceWidget from '../../../../../sharedComponents/TinyMceWidget';
import { prepareEditorRef } from '../../../../../sharedComponents/TinyMceWidget/hooks';
import { prepareEditorRef, replaceStaticWithAsset } from '../../../../../sharedComponents/TinyMceWidget/hooks';

export const ExplanationWidget = ({
// redux
settings,
learningContextId,
// injected
intl,
}) => {
const { editorRef, refReady, setEditorRef } = prepareEditorRef();
const solutionContent = replaceStaticWithAsset({
initialContent: settings?.solutionExplanation || '',
learningContextId,
});
if (!refReady) { return null; }
return (
<div className="tinyMceWidget mt-4 text-primary-500">
Expand All @@ -28,7 +33,7 @@ export const ExplanationWidget = ({
id="solution"
editorType="solution"
editorRef={editorRef}
editorContentHtml={settings?.solutionExplanation}
editorContentHtml={solutionContent}
setEditorRef={setEditorRef}
minHeight={150}
placeholder={intl.formatMessage(messages.placeholder)}
Expand All @@ -41,11 +46,13 @@ ExplanationWidget.propTypes = {
// redux
// eslint-disable-next-line
settings: PropTypes.any.isRequired,
learningContextId: PropTypes.string.isRequired,
// injected
intl: intlShape.isRequired,
};
export const mapStateToProps = (state) => ({
settings: selectors.problem.settings(state),
learningContextId: selectors.app.learningContextId(state),
});

export default injectIntl(connect(mapStateToProps)(ExplanationWidget));
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ jest.mock('../../../../../data/redux', () => ({
problem: {
settings: jest.fn(state => ({ question: state })),
},
app: {
learningContextId: jest.fn(state => ({ learningContextId: state })),
},
},
thunkActions: {
video: {
Expand All @@ -25,11 +28,13 @@ jest.mock('../../../../../sharedComponents/TinyMceWidget/hooks', () => ({
refReady: true,
setEditorRef: jest.fn().mockName('prepareEditorRef.setEditorRef'),
})),
replaceStaticWithAsset: jest.fn(() => 'This is my solution'),
}));

describe('SolutionWidget', () => {
const props = {
settings: { solutionExplanation: 'This is my question' },
settings: { solutionExplanation: 'This is my solution' },
learningContextId: 'course+org+run',
// injected
intl: { formatMessage },
};
Expand All @@ -40,8 +45,11 @@ describe('SolutionWidget', () => {
});
describe('mapStateToProps', () => {
const testState = { A: 'pple', B: 'anana', C: 'ucumber' };
test('question from problem.question', () => {
test('settings from problem.settings', () => {
expect(mapStateToProps(testState).settings).toEqual(selectors.problem.settings(testState));
});
test('learningContextId from app.learningContextId', () => {
expect(mapStateToProps(testState).learningContextId).toEqual(selectors.app.learningContextId(testState));
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,20 @@ import { selectors } from '../../../../../data/redux';
import messages from './messages';

import TinyMceWidget from '../../../../../sharedComponents/TinyMceWidget';
import { prepareEditorRef } from '../../../../../sharedComponents/TinyMceWidget/hooks';
import { prepareEditorRef, replaceStaticWithAsset } from '../../../../../sharedComponents/TinyMceWidget/hooks';

export const QuestionWidget = ({
// redux
question,
learningContextId,
// injected
intl,
}) => {
const { editorRef, refReady, setEditorRef } = prepareEditorRef();
const questionContent = replaceStaticWithAsset({
initialContent: question,
learningContextId,
});
if (!refReady) { return null; }
return (
<div className="tinyMceWidget">
Expand All @@ -25,7 +30,7 @@ export const QuestionWidget = ({
id="question"
editorType="question"
editorRef={editorRef}
editorContentHtml={question}
editorContentHtml={questionContent}
setEditorRef={setEditorRef}
minHeight={150}
placeholder={intl.formatMessage(messages.placeholder)}
Expand All @@ -37,11 +42,13 @@ export const QuestionWidget = ({
QuestionWidget.propTypes = {
// redux
question: PropTypes.string.isRequired,
learningContextId: PropTypes.string.isRequired,
// injected
intl: intlShape.isRequired,
};
export const mapStateToProps = (state) => ({
question: selectors.problem.question(state),
learningContextId: selectors.app.learningContextId(state),
});

export default injectIntl(connect(mapStateToProps)(QuestionWidget));
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,7 @@ jest.mock('../../../../../data/redux', () => ({
},
selectors: {
app: {
isLibrary: jest.fn(state => ({ isLibrary: state })),
lmsEndpointUrl: jest.fn(state => ({ lmsEndpointUrl: state })),
studioEndpointUrl: jest.fn(state => ({ studioEndpointUrl: state })),
learningContextId: jest.fn(state => ({ learningContextId: state })),
},
problem: {
question: jest.fn(state => ({ question: state })),
Expand All @@ -35,13 +33,14 @@ jest.mock('../../../../../sharedComponents/TinyMceWidget/hooks', () => ({
refReady: true,
setEditorRef: jest.fn().mockName('prepareEditorRef.setEditorRef'),
})),
// problemEditorConfig: jest.fn(args => ({ problemEditorConfig: args })),
replaceStaticWithAsset: jest.fn(() => 'This is my question'),
}));

describe('QuestionWidget', () => {
const props = {
question: 'This is my question',
updateQuestion: jest.fn(),
learningContextId: 'course+org+run',
// injected
intl: { formatMessage },
};
Expand All @@ -55,5 +54,8 @@ describe('QuestionWidget', () => {
test('question from problem.question', () => {
expect(mapStateToProps(testState).question).toEqual(selectors.problem.question(testState));
});
test('learningContextId from app.learningContextId', () => {
expect(mapStateToProps(testState).learningContextId).toEqual(selectors.app.learningContextId(testState));
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,13 @@ export const parseState = ({
problem,
isAdvanced,
ref,
assets,
lmsEndpointUrl,
}) => () => {
const rawOLX = ref?.current?.state.doc.toString();
const editorObject = fetchEditorContent({ format: '' });
const reactOLXParser = new ReactStateOLXParser({ problem, editorObject });
const reactSettingsParser = new ReactStateSettingsParser({ problem, rawOLX });
const reactBuiltOlx = setAssetToStaticUrl({ editorValue: reactOLXParser.buildOLX(), assets, lmsEndpointUrl });
const reactBuiltOlx = setAssetToStaticUrl({ editorValue: reactOLXParser.buildOLX(), lmsEndpointUrl });
return {
settings: isAdvanced ? reactSettingsParser.parseRawOlxSettings() : reactSettingsParser.getSettings(),
olx: isAdvanced ? rawOLX : reactBuiltOlx,
Expand Down Expand Up @@ -143,7 +142,6 @@ export const getContent = ({
openSaveWarningModal,
isAdvancedProblemType,
editorRef,
assets,
lmsEndpointUrl,
}) => {
const problem = problemState;
Expand All @@ -161,7 +159,6 @@ export const getContent = ({
isAdvanced: isAdvancedProblemType,
ref: editorRef,
problem,
assets,
lmsEndpointUrl,
})();
return data;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ export const EditProblemView = ({
// redux
problemType,
problemState,
assets,
lmsEndpointUrl,
returnUrl,
analytics,
Expand All @@ -48,7 +47,6 @@ export const EditProblemView = ({
openSaveWarningModal,
isAdvancedProblemType,
editorRef,
assets,
lmsEndpointUrl,
})}
returnFunction={returnFunction}
Expand All @@ -70,7 +68,6 @@ export const EditProblemView = ({
problem: problemState,
isAdvanced: isAdvancedProblemType,
ref: editorRef,
assets,
lmsEndpointUrl,
})(),
returnFunction,
Expand Down Expand Up @@ -118,7 +115,6 @@ export const EditProblemView = ({
};

EditProblemView.defaultProps = {
assets: null,
lmsEndpointUrl: null,
returnFunction: null,
};
Expand All @@ -128,7 +124,6 @@ EditProblemView.propTypes = {
returnFunction: PropTypes.func,
// eslint-disable-next-line
problemState: PropTypes.any.isRequired,
assets: PropTypes.shape({}),
analytics: PropTypes.shape({}).isRequired,
lmsEndpointUrl: PropTypes.string,
returnUrl: PropTypes.string.isRequired,
Expand All @@ -137,7 +132,6 @@ EditProblemView.propTypes = {
};

export const mapStateToProps = (state) => ({
assets: selectors.app.assets(state),
analytics: selectors.app.analytics(state),
lmsEndpointUrl: selectors.app.lmsEndpointUrl(state),
returnUrl: selectors.app.returnUrl(state),
Expand Down
13 changes: 3 additions & 10 deletions src/editors/containers/ProblemEditor/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,17 @@ export const ProblemEditor = ({
problemType,
blockFinished,
blockFailed,
studioViewFinished,
blockValue,
initializeProblemEditor,
assetsFinished,
advancedSettingsFinished,
}) => {
React.useEffect(() => {
if (blockFinished && studioViewFinished && assetsFinished && !blockFailed) {
if (blockFinished && !blockFailed) {
initializeProblemEditor(blockValue);
}
}, [blockFinished, studioViewFinished, assetsFinished, blockFailed]);
}, [blockFinished, blockFailed]);

if (!blockFinished || !studioViewFinished || !assetsFinished || !advancedSettingsFinished) {
if (!blockFinished || !advancedSettingsFinished) {
return (
<div className="text-center p-6">
<Spinner
Expand All @@ -55,18 +53,15 @@ export const ProblemEditor = ({
};

ProblemEditor.defaultProps = {
assetsFinished: null,
returnFunction: null,
};
ProblemEditor.propTypes = {
onClose: PropTypes.func.isRequired,
returnFunction: PropTypes.func,
// redux
assetsFinished: PropTypes.bool,
advancedSettingsFinished: PropTypes.bool.isRequired,
blockFinished: PropTypes.bool.isRequired,
blockFailed: PropTypes.bool.isRequired,
studioViewFinished: PropTypes.bool.isRequired,
problemType: PropTypes.string.isRequired,
initializeProblemEditor: PropTypes.func.isRequired,
blockValue: PropTypes.objectOf(PropTypes.shape({})).isRequired,
Expand All @@ -75,10 +70,8 @@ ProblemEditor.propTypes = {
export const mapStateToProps = (state) => ({
blockFinished: selectors.requests.isFinished(state, { requestKey: RequestKeys.fetchBlock }),
blockFailed: selectors.requests.isFailed(state, { requestKey: RequestKeys.fetchBlock }),
studioViewFinished: selectors.requests.isFinished(state, { requestKey: RequestKeys.fetchStudioView }),
problemType: selectors.problem.problemType(state),
blockValue: selectors.app.blockValue(state),
assetsFinished: selectors.requests.isFinished(state, { requestKey: RequestKeys.fetchAssets }),
advancedSettingsFinished: selectors.requests.isFinished(state, { requestKey: RequestKeys.fetchAdvancedSettings }),
});

Expand Down
Loading

0 comments on commit 2678234

Please sign in to comment.