Skip to content
Open
Show file tree
Hide file tree
Changes from 2 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
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import Randomization from './settingsComponents/Randomization';
// This widget should be connected, grab all settings from store, update them as needed.
const SettingsWidget = ({
problemType,
editorRef,
// redux
answers,
groupFeedbackList,
Expand All @@ -45,6 +46,7 @@ const SettingsWidget = ({
isMarkdownEditorEnabledForContext,
} = useEditorContext();
const rawMarkdown = useSelector(selectors.problem.rawMarkdown);
const isMarkdownEditorEnabled = useSelector(selectors.problem.isMarkdownEditorEnabled);
const showMarkdownEditorButton = isMarkdownEditorEnabledForContext && rawMarkdown;
const { isAdvancedCardsVisible, showAdvancedCards } = showAdvancedSettingsCards();
const feedbackCard = () => {
Expand Down Expand Up @@ -159,12 +161,12 @@ const SettingsWidget = ({
</div>
)}
<div className="my-3">
<SwitchEditorCard problemType={problemType} editorType="advanced" />
<SwitchEditorCard problemType={problemType} editorType="advanced" editorRef={editorRef} />
</div>
{ showMarkdownEditorButton
{ (showMarkdownEditorButton && !isMarkdownEditorEnabled) // Only show button if not already in markdown editor
&& (
<div className="my-3">
<SwitchEditorCard problemType={problemType} editorType="markdown" />
<SwitchEditorCard problemType={problemType} editorType="markdown" editorRef={editorRef} />
</div>
)}
</Collapsible.Body>
Expand Down Expand Up @@ -193,6 +195,8 @@ SettingsWidget.propTypes = {
blockTitle: PropTypes.string.isRequired,
correctAnswerCount: PropTypes.number.isRequired,
problemType: PropTypes.string.isRequired,
// eslint-disable-next-line react/forbid-prop-types
editorRef: PropTypes.object.isRequired,
setBlockTitle: PropTypes.func.isRequired,
updateAnswer: PropTypes.func.isRequired,
updateField: PropTypes.func.isRequired,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React from 'react';

import { ProblemTypeKeys } from '@src/editors/data/constants/problem';
import { screen, initializeMocks } from '@src/testUtils';
import { editorRender } from '@src/editors/editorTestRender';
import { editorRender, type PartialEditorState } from '@src/editors/editorTestRender';
import { mockWaffleFlags } from '@src/data/apiHooks.mock';
import * as hooks from './hooks';
import { SettingsWidgetInternal as SettingsWidget } from '.';
Expand All @@ -23,11 +23,11 @@ describe('SettingsWidget', () => {
const showAdvancedSettingsCardsBaseProps = {
isAdvancedCardsVisible: false,
showAdvancedCards: jest.fn().mockName('showAdvancedSettingsCards.showAdvancedCards'),
setResetTrue: jest.fn().mockName('showAdvancedSettingsCards.setResetTrue'),
};

const props = {
problemType: ProblemTypeKeys.TEXTINPUT,
editorRef: { current: null },
settings: {},
defaultSettings: {
maxAttempts: 2,
Expand Down Expand Up @@ -92,6 +92,45 @@ describe('SettingsWidget', () => {
expect(container.querySelector('randomization')).toBeInTheDocument();
});
});
describe('SwitchEditorCard rendering (markdown vs advanced)', () => {
test('shows two SwitchEditorCard components when markdown is available and not currently enabled', () => {
const showAdvancedSettingsCardsProps = {
...showAdvancedSettingsCardsBaseProps,
isAdvancedCardsVisible: true,
};
jest.spyOn(hooks, 'showAdvancedSettingsCards').mockReturnValue(showAdvancedSettingsCardsProps);
const modifiedInitialState: PartialEditorState = {
problem: {
problemType: null, // non-advanced problem
isMarkdownEditorEnabled: false, // currently in advanced/raw (or standard) editor
rawOLX: '<problem></problem>',
rawMarkdown: '## Problem', // markdown content exists so button should appear
isDirty: false,
},
};
const { container } = editorRender(<SettingsWidget {...props} />, { initialState: modifiedInitialState });
expect(container.querySelectorAll('switcheditorcard')).toHaveLength(2);
});

test('shows only the advanced SwitchEditorCard when already in markdown mode', () => {
const showAdvancedSettingsCardsProps = {
...showAdvancedSettingsCardsBaseProps,
isAdvancedCardsVisible: true,
};
jest.spyOn(hooks, 'showAdvancedSettingsCards').mockReturnValue(showAdvancedSettingsCardsProps);
const modifiedInitialState: PartialEditorState = {
problem: {
problemType: null,
isMarkdownEditorEnabled: true, // already in markdown editor, so markdown button hidden
rawOLX: '<problem></problem>',
rawMarkdown: '## Problem',
isDirty: false,
},
};
const { container } = editorRender(<SettingsWidget {...props} />, { initialState: modifiedInitialState });
expect(container.querySelectorAll('switcheditorcard')).toHaveLength(1);
});
});

describe('isLibrary', () => {
const libraryProps = {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import React from 'react';
import { useDispatch, useSelector } from 'react-redux';
import { useDispatch } from 'react-redux';
import { FormattedMessage } from '@edx/frontend-platform/i18n';
import { Card } from '@openedx/paragon';
import PropTypes from 'prop-types';
import { useEditorContext } from '@src/editors/EditorContext';
import { selectors, thunkActions } from '@src/editors/data/redux';
import { thunkActions } from '@src/editors/data/redux';
import BaseModal from '@src/editors/sharedComponents/BaseModal';
import Button from '@src/editors/sharedComponents/Button';
import { ProblemTypeKeys } from '@src/editors/data/constants/problem';
Expand All @@ -14,14 +13,11 @@ import { handleConfirmEditorSwitch } from '../hooks';
const SwitchEditorCard = ({
editorType,
problemType,
editorRef,
}) => {
const [isConfirmOpen, setConfirmOpen] = React.useState(false);
const { isMarkdownEditorEnabledForContext } = useEditorContext();
const isMarkdownEditorEnabled = useSelector(selectors.problem.isMarkdownEditorEnabled);
const dispatch = useDispatch();

const isMarkdownEditorActive = isMarkdownEditorEnabled && isMarkdownEditorEnabledForContext;
if (isMarkdownEditorActive || problemType === ProblemTypeKeys.ADVANCED) { return null; }
if (problemType === ProblemTypeKeys.ADVANCED) { return null; }

return (
<Card className="border border-light-700 shadow-none">
Expand All @@ -33,7 +29,7 @@ const SwitchEditorCard = ({
<Button
/* istanbul ignore next */
onClick={() => handleConfirmEditorSwitch({
switchEditor: () => dispatch(thunkActions.problem.switchEditor(editorType)),
switchEditor: () => dispatch(thunkActions.problem.switchEditor(editorType, editorRef)),
setConfirmOpen,
})}
variant="primary"
Expand All @@ -60,6 +56,8 @@ const SwitchEditorCard = ({
SwitchEditorCard.propTypes = {
problemType: PropTypes.string.isRequired,
editorType: PropTypes.string.isRequired,
// eslint-disable-next-line react/forbid-prop-types
editorRef: PropTypes.object.isRequired,
Copy link
Contributor

Choose a reason for hiding this comment

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

This is fine, but I would have preferred we pass the editorRef around using a ProblemEditorContext instead of prop drilling.

};

export default SwitchEditorCard;
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ describe('SwitchEditorCard - markdown', () => {
const baseProps = {
problemType: 'stringresponse',
editorType: 'markdown',
editorRef: { current: null },
};

beforeEach(() => {
Expand Down Expand Up @@ -49,7 +50,7 @@ describe('SwitchEditorCard - markdown', () => {
expect(confirmButton).toBeInTheDocument();
expect(switchEditorSpy).not.toHaveBeenCalled();
await user.click(confirmButton);
expect(switchEditorSpy).toHaveBeenCalledWith('markdown');
expect(switchEditorSpy).toHaveBeenCalledWith('markdown', { current: null });
// Markdown editor would now be active.
});

Expand All @@ -58,20 +59,4 @@ describe('SwitchEditorCard - markdown', () => {
const reduxWrapper = (container.firstChild as HTMLElement | null);
expect(reduxWrapper?.innerHTML).toBe('');
});

test('returns null when editor is already Markdown', () => {
// Markdown Editor support is on for this course:
mockWaffleFlags({ useReactMarkdownEditor: true });
// The markdown editor *IS* currently active (default)

const { container } = editorRender(<SwitchEditorCard {...baseProps} />, {
initialState: {
problem: {
isMarkdownEditorEnabled: true,
},
},
});
const reduxWrapper = (container.firstChild as HTMLElement | null);
expect(reduxWrapper?.innerHTML).toBe('');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,10 @@ export const parseState = ({
return {
settings: {
...settings,
...(isMarkdownEditorEnabled && { markdown: contentString }),
// If the save action isn’t triggered from the Markdown editor, the Markdown content might be outdated. Since the
// Markdown editor shouldn't be displayed in future in this case, we’re sending `null` instead.
// TODO: Implement OLX-to-Markdown conversion to properly handle this scenario.
markdown: isMarkdownEditorEnabled ? contentString : null,
markdown_edited: isMarkdownEditorEnabled,
},
olx: isAdvanced || isMarkdownEditorEnabled ? rawOLX : reactBuiltOlx,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ describe('EditProblemView hooks parseState', () => {
assets: {},
})();
expect(res.olx).toBe(mockRawOLX);
expect(res.settings.markdown).toBe(null);
});
it('markdown problem', () => {
const res = hooks.parseState({
Expand Down Expand Up @@ -306,13 +307,16 @@ describe('EditProblemView hooks parseState', () => {
show_reset_button: false,
submission_wait_seconds: 0,
attempts_before_showanswer_button: 0,
markdown: null,
markdown_edited: false,
};
const openSaveWarningModal = jest.fn();

it('default visual save and returns parseState data', () => {
const problem = { ...problemState, problemType: ProblemTypeKeys.NUMERIC, answers: [{ id: 'A', title: 'problem', correct: true }] };
const content = hooks.getContent({
isAdvancedProblemType: false,
isMarkdownEditorEnabled: false,
problemState: problem,
editorRef,
assets,
Expand All @@ -339,6 +343,7 @@ describe('EditProblemView hooks parseState', () => {
};
const { settings } = hooks.getContent({
isAdvancedProblemType: false,
isMarkdownEditorEnabled: false,
problemState: problem,
editorRef,
assets,
Expand All @@ -353,12 +358,15 @@ describe('EditProblemView hooks parseState', () => {
attempts_before_showanswer_button: 0,
submission_wait_seconds: 0,
weight: 1,
markdown: null,
markdown_edited: false,
});
});

it('default advanced save and returns parseState data', () => {
const content = hooks.getContent({
isAdvancedProblemType: true,
isMarkdownEditorEnabled: false,
problemState,
editorRef,
assets,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ const EditProblemView = ({ returnFunction }) => {
)}

<span className="editProblemView-settingsColumn">
<SettingsWidget problemType={problemType} />
<SettingsWidget problemType={problemType} editorRef={editorRef} />
</span>
</div>
</EditorContainer>
Expand Down
91 changes: 73 additions & 18 deletions src/editors/data/redux/thunkActions/problem.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,31 +45,86 @@ describe('problem thunkActions', () => {
let dispatch;
let getState;
let dispatchedAction;
let mockEditorRef;

const mockProblemState = (isMarkdownEditorEnabled) => ({
problem: {
isMarkdownEditorEnabled,
rawOLX: 'PREVIOUS_OLX',
},
app: {
learningContextId: 'course-v1:org+course+run',
blockValue,
},
});

const createMockEditorRef = (content = 'MockMarkdownContent') => ({
current: {
state: {
doc: { toString: jest.fn(() => content) },
},
},
});

beforeEach(() => {
dispatch = jest.fn((action) => ({ dispatch: action }));
getState = jest.fn(() => ({
problem: {
},
app: {
learningContextId: 'course-v1:org+course+run',
blockValue,
},
}));
mockEditorRef = createMockEditorRef();
});

afterEach(() => {
jest.restoreAllMocks();
});
test('initializeProblem visual Problem :', () => {
initializeProblem(blockValue)(dispatch, getState);
expect(dispatch).toHaveBeenCalled();

describe('when markdown editor is enabled', () => {
beforeEach(() => {
getState = jest.fn(() => mockProblemState(true));
});

test('initializeProblem triggers dispatch', () => {
initializeProblem(blockValue)(dispatch, getState);
expect(dispatch).toHaveBeenCalled();
});

test('switchToAdvancedEditor converts markdown to OLX', () => {
switchToAdvancedEditor(mockEditorRef)(dispatch, getState);
expect(dispatch).toHaveBeenCalledWith(
actions.problem.updateField({
problemType: ProblemTypeKeys.ADVANCED,
rawOLX: '<problem>\n<p>MockMarkdownContent</p>\n</problem>',
isMarkdownEditorEnabled: false,
}),
);
});

test('switchToAdvancedEditor falls back to previous OLX if editorRef missing', () => {
switchToAdvancedEditor(null)(dispatch, getState);
expect(dispatch).toHaveBeenCalledWith(
actions.problem.updateField({
problemType: ProblemTypeKeys.ADVANCED,
rawOLX: 'PREVIOUS_OLX',
isMarkdownEditorEnabled: false,
}),
);
});
});
test('switchToAdvancedEditor visual Problem', () => {
switchToAdvancedEditor()(dispatch, getState);
expect(dispatch).toHaveBeenCalledWith(
actions.problem.updateField({ problemType: ProblemTypeKeys.ADVANCED, rawOLX: mockOlx }),
);

describe('when markdown editor is disabled', () => {
beforeEach(() => {
getState = jest.fn(() => mockProblemState(false));
});

test('switchToAdvancedEditor uses ReactStateOLXParser', () => {
switchToAdvancedEditor(mockEditorRef)(dispatch, getState);
expect(dispatch).toHaveBeenCalledWith(
actions.problem.updateField({
problemType: ProblemTypeKeys.ADVANCED,
rawOLX: mockOlx,
isMarkdownEditorEnabled: false,
}),
);
});
});

test('switchToMarkdownEditor dispatches correct actions', () => {
switchToMarkdownEditor()(dispatch);

Expand All @@ -94,12 +149,12 @@ describe('problem thunkActions', () => {
});

test('dispatches switchToAdvancedEditor when editorType is advanced', () => {
switchEditor('advanced')(dispatch, getState);
switchEditor('advanced', mockEditorRef)(dispatch, getState);
expect(switchToAdvancedEditorMock).toHaveBeenCalledWith(dispatch, getState);
});

test('dispatches switchToMarkdownEditor when editorType is markdown', () => {
switchEditor('markdown')(dispatch, getState);
switchEditor('markdown', mockEditorRef)(dispatch, getState);
expect(switchToMarkdownEditorMock).toHaveBeenCalledWith(dispatch);
});
});
Expand Down
Loading