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

fix: block resize with long content and broken image path #478

Closed
Show file tree
Hide file tree
Changes from all 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 @@ -123,7 +123,7 @@ exports[`EditorProblemView component renders simple view 1`] = `
className="editProblemView d-flex flex-row flex-nowrap justify-content-end"
>
<span
className="flex-grow-1 mb-5"
className="editProblemView-contentColumn flex-grow-1 mb-5"
>
<injectIntl(ShimmedIntlComponent) />
<injectIntl(ShimmedIntlComponent) />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ export const EditProblemView = ({
<RawEditor editorRef={editorRef} lang="xml" content={problemState.rawOLX} />
</Container>
) : (
<span className="flex-grow-1 mb-5">
<span className="editProblemView-contentColumn flex-grow-1 mb-5">
<QuestionWidget />
<ExplanationWidget />
<AnswerWidget problemType={problemType} />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,25 @@
}
}
}

span.mce-preview-object.mce-object-iframe {
position: relative;
overflow: hidden;
width: 100%;
padding-top: 56.25%;
display: inline-block;

iframe {
position: absolute;
top: 0;
left: 0;
bottom: 0;
right: 0;
width: 100%;
height: 100%;
}
}

.mce-preview-object.mce-object-audio audio {
max-width: 100%;
}
16 changes: 16 additions & 0 deletions src/editors/containers/ProblemEditor/data/utils.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/* eslint-disable import/prefer-default-export */
import { replaceRelativeImageUrlsByAbsolute } from '../../../data/services/cms/utils';

export const preprocessProblemData = (problemData, state) => {
if (problemData?.settings?.hints) {
problemData.settings.hints = problemData.settings.hints.map(
hint => ({ ...hint, value: replaceRelativeImageUrlsByAbsolute(hint.value, state.app.lmsEndpointUrl) }),
);
}

if (problemData?.answers) {
problemData.answers = problemData.answers.map(
answer => ({ ...answer, title: replaceRelativeImageUrlsByAbsolute(answer.title, state.app.lmsEndpointUrl) }),
);
}
};
18 changes: 15 additions & 3 deletions src/editors/data/constants/tinyMCEStyles.js
Original file line number Diff line number Diff line change
Expand Up @@ -197,9 +197,6 @@ const getStyles = () => (
color: #0075b4;
text-decoration: none;
}
.mce-content-body img {
max-width: 100%;
}
.mce-content-body pre {
margin: 1em 0;
color: #3c3c3c;
Expand Down Expand Up @@ -227,6 +224,21 @@ const getStyles = () => (
}
.mce-content-body[dir=rtl][data-mce-placeholder]:not(.mce-visualblocks)::before {
margin: 0;
}
.editProblemView-contentColumn {
max-width: calc(100% - 320px);
}
.answer-option .ml-1.flex-grow-1 {
max-width: 90%;
}
.answer-option .mce-content-body {
overflow: auto;
}
.settingsOption .container-fluid {
max-width: calc(100% - 50px);
}
.settingsOption .mce-content-body {
overflow: auto;
}`
);

Expand Down
13 changes: 8 additions & 5 deletions src/editors/data/redux/thunkActions/problem.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import ReactStateOLXParser from '../../../containers/ProblemEditor/data/ReactSta
import { blankProblemOLX } from '../../../containers/ProblemEditor/data/mockData/olxTestData';
import { camelizeKeys } from '../../../utils';
import { fetchEditorContent } from '../../../containers/ProblemEditor/components/EditProblemView/hooks';
import { preprocessProblemData } from '../../../containers/ProblemEditor/data/utils';

export const switchToAdvancedEditor = () => (dispatch, getState) => {
const state = getState();
Expand Down Expand Up @@ -47,15 +48,17 @@ export const getDataFromOlx = ({ rawOLX, rawSettings, defaultSettings }) => {
return { settings: parsedSettings };
};

export const loadProblem = ({ rawOLX, rawSettings, defaultSettings }) => (dispatch) => {
export const loadProblem = ({ rawOLX, rawSettings, defaultSettings }) => (dispatch, getState) => {
if (isBlankProblem({ rawOLX })) {
dispatch(actions.problem.setEnableTypeSelection(camelizeKeys(defaultSettings)));
} else {
dispatch(actions.problem.load(getDataFromOlx({ rawOLX, rawSettings, defaultSettings })));
const dataFromOlx = getDataFromOlx({ rawOLX, rawSettings, defaultSettings });
preprocessProblemData(dataFromOlx, getState());
Copy link
Member

Choose a reason for hiding this comment

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

This should be happening when the tinyMCE editor loads, so it is not necessary to preprocess the problem here. See https://github.com/openedx/frontend-lib-content-components/blob/main/src/editors/sharedComponents/TinyMceWidget/hooks.js#L74-L97

Copy link
Member

Choose a reason for hiding this comment

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

If this is not longer needed, please remove all changes related to preprocessProblemData

dispatch(actions.problem.load(dataFromOlx));
}
};

export const fetchAdvancedSettings = ({ rawOLX, rawSettings }) => (dispatch) => {
export const fetchAdvancedSettings = ({ rawOLX, rawSettings }) => (dispatch, getState) => {
const advancedProblemSettingKeys = ['max_attempts', 'showanswer', 'show_reset_button', 'rerandomize'];
dispatch(requests.fetchAdvancedSettings({
onSuccess: (response) => {
Expand All @@ -66,9 +69,9 @@ export const fetchAdvancedSettings = ({ rawOLX, rawSettings }) => (dispatch) =>
}
});
dispatch(actions.problem.updateField({ defaultSettings: camelizeKeys(defaultSettings) }));
loadProblem({ rawOLX, rawSettings, defaultSettings })(dispatch);
loadProblem({ rawOLX, rawSettings, defaultSettings })(dispatch, getState);
},
onFailure: () => { loadProblem({ rawOLX, rawSettings, defaultSettings: {} })(dispatch); },
onFailure: () => { loadProblem({ rawOLX, rawSettings, defaultSettings: {} })(dispatch, getState); },
}));
};

Expand Down
11 changes: 6 additions & 5 deletions src/editors/data/redux/thunkActions/problem.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { ProblemTypeKeys } from '../../constants/problem';
const mockOlx = 'SOmEVALue';
const mockBuildOlx = jest.fn(() => mockOlx);
jest.mock('../../../containers/ProblemEditor/data/ReactStateOLXParser', () => jest.fn().mockImplementation(() => ({ buildOLX: mockBuildOlx })));
jest.mock('../../../containers/ProblemEditor/data/utils');

jest.mock('..', () => ({
actions: {
Expand Down Expand Up @@ -59,20 +60,20 @@ describe('problem thunkActions', () => {
});
describe('fetchAdvanceSettings', () => {
it('dispatches fetchAdvanceSettings action', () => {
module.fetchAdvancedSettings({ rawOLX, rawSettings })(dispatch);
module.fetchAdvancedSettings({ rawOLX, rawSettings })(dispatch, getState);
[[dispatchedAction]] = dispatch.mock.calls;
expect(dispatchedAction.fetchAdvanceSettings).not.toEqual(undefined);
});
it('dispatches actions.problem.updateField and loadProblem on success', () => {
dispatch.mockClear();
module.fetchAdvancedSettings({ rawOLX, rawSettings })(dispatch);
module.fetchAdvancedSettings({ rawOLX, rawSettings })(dispatch, getState);
[[dispatchedAction]] = dispatch.mock.calls;
dispatchedAction.fetchAdvanceSettings.onSuccess({ data: { key: 'test', max_attempts: 1 } });
expect(dispatch).toHaveBeenCalledWith(actions.problem.load());
});
it('calls loadProblem on failure', () => {
dispatch.mockClear();
module.fetchAdvancedSettings({ rawOLX, rawSettings })(dispatch);
module.fetchAdvancedSettings({ rawOLX, rawSettings })(dispatch, getState);
[[dispatchedAction]] = dispatch.mock.calls;
dispatchedAction.fetchAdvanceSettings.onFailure();
expect(dispatch).toHaveBeenCalledWith(actions.problem.load());
Expand All @@ -81,12 +82,12 @@ describe('problem thunkActions', () => {
describe('loadProblem', () => {
test('initializeProblem advanced Problem', () => {
rawOLX = advancedProblemOlX.rawOLX;
module.loadProblem({ rawOLX, rawSettings, defaultSettings })(dispatch);
module.loadProblem({ rawOLX, rawSettings, defaultSettings })(dispatch, getState);
expect(dispatch).toHaveBeenCalledWith(actions.problem.load());
});
test('initializeProblem blank Problem', () => {
rawOLX = blankProblemOLX.rawOLX;
module.loadProblem({ rawOLX, rawSettings, defaultSettings })(dispatch);
module.loadProblem({ rawOLX, rawSettings, defaultSettings })(dispatch, getState);
expect(dispatch).toHaveBeenCalledWith(actions.problem.setEnableTypeSelection());
});
});
Expand Down
16 changes: 16 additions & 0 deletions src/editors/data/services/cms/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,19 @@ export const post = (...args) => getAuthenticatedHttpClient().post(...args);
export const deleteObject = (...args) => getAuthenticatedHttpClient().delete(...args);

export const client = getAuthenticatedHttpClient;

/**
* Replaces all relative image URLs in the given text with absolute URLs.
*
* This function searches for relative URLs in `src` attributes of image tags within the provided text,
* and replaces them with absolute URLs by prefixing them with the specified LMS endpoint URL.
*
* @param {string} text - The input text containing image tags with relative URLs.
* @param {string} lmsEndpointUrl - The base URL to prepend to each relative URL to form an absolute URL.
* @returns {string} The text with all relative image URLs replaced by absolute URLs.
*/
export const replaceRelativeImageUrlsByAbsolute = (text, lmsEndpointUrl) => {
const relativeSrcRegExp = /(?<=src=")(\/.*?)(?=")/g;
const replaceRelativeToAbsoluteUrl = (relativeUrl) => `${lmsEndpointUrl}${relativeUrl}`;
return (text || '').replaceAll(relativeSrcRegExp, replaceRelativeToAbsoluteUrl);
};
Comment on lines +36 to +40
Copy link
Member

Choose a reason for hiding this comment

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

See comment regarding preprocessProblemData and remove if necessary.

Loading