Andal.LND/frontend-app-authoring
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: image rendering in single and multi select problems (#1731)

Browse Source 
Fix images in single and multi select problems in libraries. Found following issues and fixed them:

* Images were not being rendered in any of the fields in these problems.
* Base url was not being set which is used by tinymce to load images with relative path.
* Answer fields were set to inline mode which does not initialize images or base paths
* If same image twice is used twice in a problem, the logic of replacing `static/image.jpg` with `/static/image.jpg` would replace the first occurrence twice resulting in `//static/image.jpg`, breaking both the links.
* On initialization of answer fields, the absolute static asset urls were being replaced by relative urls causing the editor being set as dirty without user changes.
This commit is contained in:
Navin Karkera
2025-03-13 16:11:44 +00:00
committed by GitHub
parent 77a55d9ad3
commit e66da2cb49
17 changed files with 103 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/AnswerOption.jsx
View File

@@ -9,6 +9,7 @@ import {
} from '@openedx/paragon';
import { FeedbackOutline, DeleteOutline } from '@openedx/paragon/icons';
import { FormattedMessage, injectIntl, intlShape } from '@edx/frontend-platform/i18n';
import { getConfig } from '@edx/frontend-platform';
import messages from './messages';
import { selectors } from '../../../../../data/redux';
import { answerOptionProps } from '../../../../../data/services/cms/types';
@@ -27,6 +28,7 @@ const AnswerOption = ({
problemType,
images,
isLibrary,
blockId,
learningContextId,
}) => {
const dispatch = useDispatch();
@@ -41,6 +43,10 @@ const AnswerOption = ({
const setSelectedFeedback = hooks.setSelectedFeedback({ answer, hasSingleAnswer, dispatch });
const setUnselectedFeedback = hooks.setUnselectedFeedback({ answer, hasSingleAnswer, dispatch });
const { isFeedbackVisible, toggleFeedback } = hooks.useFeedback(answer);
let staticRootUrl;
if (isLibrary) {
staticRootUrl = `${getConfig().STUDIO_BASE_URL }/library_assets/blocks/${ blockId }/`;
}
const getInputArea = () => {
if ([ProblemTypeKeys.SINGLESELECT, ProblemTypeKeys.MULTISELECT].includes(problemType)) {
@@ -54,6 +60,7 @@ const AnswerOption = ({
images,
isLibrary,
learningContextId,
staticRootUrl,
}}
/>
);
@@ -151,6 +158,7 @@ AnswerOption.propTypes = {
images: PropTypes.shape({}).isRequired,
learningContextId: PropTypes.string.isRequired,
isLibrary: PropTypes.bool.isRequired,
blockId: PropTypes.string.isRequired,
};
export const mapStateToProps = (state) => ({
@@ -158,6 +166,7 @@ export const mapStateToProps = (state) => ({
images: selectors.app.images(state),
isLibrary: selectors.app.isLibrary(state),
learningContextId: selectors.app.learningContextId(state),
blockId: selectors.app.blockId(state),
});
export const mapDispatchToProps = {};

1
src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/AnswerOption.test.jsx
View File

@@ -16,6 +16,7 @@ jest.mock('../../../../../data/redux', () => ({
images: jest.fn(state => ({ images: state })),
isLibrary: jest.fn(state => ({ isLibrary: state })),
learningContextId: jest.fn(state => ({ learningContextId: state })),
blockId: jest.fn(state => ({ blockId: state })),
},
},
thunkActions: {

5
src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/hooks.js
View File

@@ -34,12 +34,15 @@ export const setAnswerTitle = ({
hasSingleAnswer,
dispatch,
problemType,
}) => (updatedTitle) => {
}) => (updatedTitle, isDirty) => {
let title = updatedTitle;
if ([ProblemTypeKeys.TEXTINPUT, ProblemTypeKeys.NUMERIC, ProblemTypeKeys.DROPDOWN].includes(problemType)) {
title = updatedTitle.target.value;
}
dispatch(actions.problem.updateAnswer({ id: answer.id, hasSingleAnswer, title }));
if (isDirty !== undefined) {
dispatch(actions.problem.setDirty(isDirty));
}
};
export const setSelectedFeedback = ({ answer, hasSingleAnswer, dispatch }) => (value) => {

4
src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/hooks.test.js
View File

@@ -26,6 +26,7 @@ jest.mock('../../../../../data/redux', () => ({
problem: {
deleteAnswer: (args) => ({ deleteAnswer: args }),
updateAnswer: (args) => ({ updateAnswer: args }),
setDirty: (args) => ({ setDirty: args }),
},
},
}));
@@ -117,12 +118,13 @@ describe('Answer Options Hooks', () => {
hasSingleAnswer,
dispatch,
problemType,
})(updatedTitle);
})(updatedTitle, false);
expect(dispatch).toHaveBeenCalledWith(actions.problem.updateAnswer({
id: answer.id,
hasSingleAnswer,
title: updatedTitle,
}));
expect(dispatch).toHaveBeenCalledWith(actions.problem.setDirty(false));
});
});
describe('setSelectedFeedback', () => {

11
src/editors/containers/ProblemEditor/components/EditProblemView/ExplanationWidget/index.jsx
View File

@@ -2,9 +2,10 @@ import React from 'react';
import PropTypes from 'prop-types';
import { connect } from 'react-redux';
import { injectIntl, FormattedMessage, intlShape } from '@edx/frontend-platform/i18n';
import { getConfig } from '@edx/frontend-platform';
import { selectors } from '../../../../../data/redux';
import messages from './messages';
import TinyMceWidget from '../../../../../sharedComponents/TinyMceWidget';
import { prepareEditorRef, replaceStaticWithAsset } from '../../../../../sharedComponents/TinyMceWidget/hooks';
@@ -14,6 +15,7 @@ const ExplanationWidget = ({
learningContextId,
images,
isLibrary,
blockId,
// injected
intl,
}) => {
@@ -24,6 +26,10 @@ const ExplanationWidget = ({
learningContextId,
});
const solutionContent = newContent || initialContent;
let staticRootUrl;
if (isLibrary) {
staticRootUrl = `${getConfig().STUDIO_BASE_URL }/library_assets/blocks/${ blockId }/`;
}
if (!refReady) { return null; }
return (
<div className="tinyMceWidget mt-4 text-primary-500">
@@ -45,6 +51,7 @@ const ExplanationWidget = ({
images,
isLibrary,
learningContextId,
staticRootUrl,
}}
/>
</div>
@@ -58,6 +65,7 @@ ExplanationWidget.propTypes = {
learningContextId: PropTypes.string.isRequired,
images: PropTypes.shape({}).isRequired,
isLibrary: PropTypes.bool.isRequired,
blockId: PropTypes.string.isRequired,
// injected
intl: intlShape.isRequired,
};
@@ -66,6 +74,7 @@ export const mapStateToProps = (state) => ({
learningContextId: selectors.app.learningContextId(state),
images: selectors.app.images(state),
isLibrary: selectors.app.isLibrary(state),
blockId: selectors.app.blockId(state),
});
export const ExplanationWidgetInternal = ExplanationWidget; // For testing only

1
src/editors/containers/ProblemEditor/components/EditProblemView/ExplanationWidget/index.test.jsx
View File

@@ -16,6 +16,7 @@ jest.mock('../../../../../data/redux', () => ({
learningContextId: jest.fn(state => ({ learningContextId: state })),
images: jest.fn(state => ({ images: state })),
isLibrary: jest.fn(state => ({ isLibrary: state })),
blockId: jest.fn(state => ({ blockId: state })),
},
},
thunkActions: {

11
src/editors/containers/ProblemEditor/components/EditProblemView/QuestionWidget/index.jsx
View File

@@ -2,9 +2,10 @@ import React from 'react';
import PropTypes from 'prop-types';
import { connect } from 'react-redux';
import { injectIntl, FormattedMessage, intlShape } from '@edx/frontend-platform/i18n';
import { getConfig } from '@edx/frontend-platform';
import { selectors } from '../../../../../data/redux';
import messages from './messages';
import TinyMceWidget from '../../../../../sharedComponents/TinyMceWidget';
import { prepareEditorRef, replaceStaticWithAsset } from '../../../../../sharedComponents/TinyMceWidget/hooks';
@@ -14,6 +15,7 @@ const QuestionWidget = ({
learningContextId,
images,
isLibrary,
blockId,
// injected
intl,
}) => {
@@ -24,6 +26,10 @@ const QuestionWidget = ({
learningContextId,
});
const questionContent = newContent || initialContent;
let staticRootUrl;
if (isLibrary) {
staticRootUrl = `${getConfig().STUDIO_BASE_URL }/library_assets/blocks/${ blockId }/`;
}
if (!refReady) { return null; }
return (
<div className="tinyMceWidget">
@@ -42,6 +48,7 @@ const QuestionWidget = ({
images,
isLibrary,
learningContextId,
staticRootUrl,
}}
/>
</div>
@@ -54,6 +61,7 @@ QuestionWidget.propTypes = {
learningContextId: PropTypes.string.isRequired,
images: PropTypes.shape({}).isRequired,
isLibrary: PropTypes.bool.isRequired,
blockId: PropTypes.string.isRequired,
// injected
intl: intlShape.isRequired,
};
@@ -62,6 +70,7 @@ export const mapStateToProps = (state) => ({
learningContextId: selectors.app.learningContextId(state),
images: selectors.app.images(state),
isLibrary: selectors.app.isLibrary(state),
blockId: selectors.app.blockId(state),
});
export const QuestionWidgetInternal = QuestionWidget; // For testing only

1
src/editors/containers/ProblemEditor/components/EditProblemView/QuestionWidget/index.test.jsx
View File

@@ -18,6 +18,7 @@ jest.mock('../../../../../data/redux', () => ({
learningContextId: jest.fn(state => ({ learningContextId: state })),
images: jest.fn(state => ({ images: state })),
isLibrary: jest.fn(state => ({ isLibrary: state })),
blockId: jest.fn(state => ({ blockId: state })),
},
problem: {
question: jest.fn(state => ({ question: state })),

14
src/editors/data/redux/problem/reducers.test.ts
View File

@@ -453,5 +453,19 @@ describe('problem reducer', () => {
});
});
});
describe('setDirty', () => {
it('sets isDirty flag', () => {
expect(reducer(
{
...testingState,
isDirty: false,
},
actions.setDirty(false),
)).toEqual({
...testingState,
isDirty: false,
});
});
});
});
});

4
src/editors/data/redux/problem/reducers.ts
View File

@@ -228,6 +228,10 @@ const problem = createSlice({
problemType: null,
};
},
setDirty: (state, { payload }) => ({
...state,
isDirty: payload,
}),
},
});

12
src/editors/sharedComponents/ExpandableTextArea/__snapshots__/index.test.jsx.snap
View File

@@ -9,12 +9,14 @@ exports[`ExpandableTextArea snapshots renders as expected with default behavior
editorContentHtml="text"
editorRef={
{
"current": null,
"current": {
"value": "something",
},
}
}
editorType="expandable"
placeholder={null}
setEditorRef={[Function]}
setEditorRef={[MockFunction hooks.prepareEditorRef.setEditorRef]}
updateContent={[MockFunction]}
/>
</div>
@@ -30,12 +32,14 @@ exports[`ExpandableTextArea snapshots renders error message 1`] = `
editorContentHtml="text"
editorRef={
{
"current": null,
"current": {
"value": "something",
},
}
}
editorType="expandable"
placeholder={null}
setEditorRef={[Function]}
setEditorRef={[MockFunction hooks.prepareEditorRef.setEditorRef]}
updateContent={[MockFunction]}
/>
</div>

3
src/editors/sharedComponents/ExpandableTextArea/index.jsx
View File

@@ -11,8 +11,9 @@ const ExpandableTextArea = ({
errorMessage,
...props
}) => {
const { editorRef, setEditorRef } = prepareEditorRef();
const { editorRef, refReady, setEditorRef } = prepareEditorRef();
if (!refReady) { return null; }
return (
<>
<div className="expandable-mce error">

2
src/editors/sharedComponents/ExpandableTextArea/index.scss
View File

@@ -14,7 +14,7 @@
margin: 16px 40px;
}
}
*[contentEditable="false"] {
outline: 1px solid #D7D3D1;
}

9
src/editors/sharedComponents/ExpandableTextArea/index.test.jsx
View File

@@ -15,6 +15,15 @@ jest.mock('@tinymce/tinymce-react', () => {
jest.mock('../TinyMceWidget', () => 'TinyMceWidget');
// Mock the TinyMceWidget
jest.mock('../TinyMceWidget/hooks', () => ({
prepareEditorRef: jest.fn(() => ({
editorRef: { current: { value: 'something' } },
refReady: true,
setEditorRef: jest.fn().mockName('hooks.prepareEditorRef.setEditorRef'),
})),
}));
describe('ExpandableTextArea', () => {
const props = {
value: 'text',

20
src/editors/sharedComponents/TinyMceWidget/hooks.js
View File

@@ -250,7 +250,12 @@ export const setupCustomBehavior = ({
lmsEndpointUrl,
learningContextId,
});
if (newContent) { updateContent(newContent); }
// istanbul ignore if
if (newContent) {
// update content but mark as not dirty as user did not change anything
updateContent(newContent, false);
editor.setDirty(false);
}
});
}
@@ -479,14 +484,21 @@ export const setAssetToStaticUrl = ({ editorValue, lmsEndpointUrl }) => {
content = updatedContent;
});
/* istanbul ignore next */
const updatedStaticUrls = [];
assetSrcs.filter(src => src.startsWith('static/')).forEach(src => {
// Before storing assets we make sure that library static assets points again to
// `/static/dummy.jpg` instead of using the relative url `static/dummy.jpg`
const nameFromEditorSrc = parseAssetName(src);
const portableUrl = `/${ nameFromEditorSrc}`;
const portableUrl = `/${nameFromEditorSrc}`;
if (updatedStaticUrls.includes(portableUrl)) {
// If same image is used multiple times in the same src,
// replace all occurence once and do not process them again.
return;
}
// track updated urls to process only once.
updatedStaticUrls.push(portableUrl);
const currentSrc = src.substring(0, src.search(/("|&quot;)/));
const updatedContent = content.replace(currentSrc, portableUrl);
const updatedContent = content.replaceAll(currentSrc, portableUrl);
content = updatedContent;
});
return content;

6
src/editors/sharedComponents/TinyMceWidget/hooks.test.js
View File

@@ -221,6 +221,12 @@ describe('TinyMceEditor hooks', () => {
const content = module.setAssetToStaticUrl({ editorValue, lmsEndpointUrl });
expect(content).toEqual('<img src="/static/soME_ImagE_URl1"/> <a href="/static/soMEImagEURl">testing link</a>');
});
it('returns content with updated static img links', () => {
const editorValue = '<img src="static/goku.img"/> <a href="static/goku.img">testing link</a>';
const lmsEndpointUrl = getConfig().LMS_BASE_URL;
const content = module.setAssetToStaticUrl({ editorValue, lmsEndpointUrl });
expect(content).toEqual('<img src="/static/goku.img"/> <a href="/static/goku.img">testing link</a>');
});
});
describe('editorConfig', () => {

6
src/editors/sharedComponents/TinyMceWidget/pluginConfig.js
View File

@@ -12,8 +12,9 @@ const pluginConfig = ({ placeholder, editorType, enableImageUpload }) => {
const codeButton = editorType === 'text' ? buttons.code : '';
const labelButton = editorType === 'question' ? buttons.customLabelButton : '';
const quickToolbar = editorType === 'expandable' ? plugins.quickbars : '';
const inline = editorType === 'expandable';
const statusbar = editorType !== 'expandable';
const toolbar = editorType !== 'expandable';
const autoresizeBottomMargin = editorType === 'expandable' ? 10 : 50;
const defaultFormat = (editorType === 'question' || editorType === 'expandable') ? 'div' : 'p';
const hasStudioHeader = document.querySelector('.studio-header');
@@ -90,13 +91,14 @@ const pluginConfig = ({ placeholder, editorType, enableImageUpload }) => {
relative_urls: true,
convert_urls: false,
placeholder,
inline,
statusbar,
block_formats: 'Header 1=h1;Header 2=h2;Header 3=h3;Header 4=h4;Header 5=h5;Header 6=h6;Div=div;Paragraph=p;Preformatted=pre',
forced_root_block: defaultFormat,
powerpaste_allow_local_images: true,
powerpaste_word_import: 'prompt',
powerpaste_html_import: 'prompt',
powerpaste_googledoc_import: 'prompt',
autoresize_bottom_margin: autoresizeBottomMargin,
},
})
);