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: allow to preview images #1403

Merged
merged 2 commits into from
Oct 24, 2024
Merged
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
9 changes: 9 additions & 0 deletions src/editors/containers/TextEditor/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
} from '@openedx/paragon';
import { injectIntl, intlShape } from '@edx/frontend-platform/i18n';

import { getConfig } from '@edx/frontend-platform';
import { actions, selectors } from '../../data/redux';
import { RequestKeys } from '../../data/constants/requests';

Expand All @@ -24,6 +25,7 @@ const TextEditor = ({
// redux
showRawEditor,
blockValue,
blockId,
blockFailed,
initializeEditor,
blockFinished,
Expand All @@ -40,6 +42,10 @@ const TextEditor = ({
learningContextId,
});
const editorContent = newContent || initialContent;
let staticRootUrl;
if (isLibrary) {
staticRootUrl = `${getConfig().STUDIO_BASE_URL }/library_assets/blocks/${ blockId }/`;
}

if (!refReady) { return null; }

Expand All @@ -65,6 +71,7 @@ const TextEditor = ({
images,
isLibrary,
learningContextId,
staticRootUrl,
}}
/>
);
Expand Down Expand Up @@ -107,6 +114,7 @@ TextEditor.propTypes = {
blockValue: PropTypes.shape({
data: PropTypes.shape({ data: PropTypes.string }),
}),
blockId: PropTypes.string,
blockFailed: PropTypes.bool.isRequired,
initializeEditor: PropTypes.func.isRequired,
showRawEditor: PropTypes.bool.isRequired,
Expand All @@ -121,6 +129,7 @@ TextEditor.propTypes = {
export const mapStateToProps = (state) => ({
blockValue: selectors.app.blockValue(state),
blockFailed: selectors.requests.isFailed(state, { requestKey: RequestKeys.fetchBlock }),
blockId: selectors.app.blockId(state),
showRawEditor: selectors.app.showRawEditor(state),
blockFinished: selectors.requests.isFinished(state, { requestKey: RequestKeys.fetchBlock }),
learningContextId: selectors.app.learningContextId(state),
Expand Down
1 change: 1 addition & 0 deletions src/editors/containers/TextEditor/index.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ jest.mock('../../data/redux', () => ({
showRawEditor: jest.fn(state => ({ showRawEditor: state })),
images: jest.fn(state => ({ images: state })),
isLibrary: jest.fn(state => ({ isLibrary: state })),
blockId: jest.fn(state => ({ blockId: state })),
learningContextId: jest.fn(state => ({ learningContextId: state })),
},
requests: {
Expand Down
1 change: 1 addition & 0 deletions src/editors/data/redux/app/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ const app = createSlice({
blockValue: payload,
blockTitle: payload.data.display_name,
}),
setBlockId: (state, { payload }) => ({ ...state, blockId: payload }),
setStudioView: (state, { payload }) => ({ ...state, studioView: payload }),
setBlockContent: (state, { payload }) => ({ ...state, blockContent: payload }),
setBlockTitle: (state, { payload }) => ({ ...state, blockTitle: payload }),
Expand Down
1 change: 1 addition & 0 deletions src/editors/data/redux/app/reducer.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ describe('app reducer', () => {
['setUnitUrl', 'unitUrl'],
['setStudioView', 'studioView'],
['setBlockContent', 'blockContent'],
['setBlockId', 'blockId'],
['setBlockTitle', 'blockTitle'],
['setSaveResponse', 'saveResponse'],
['setVideos', 'videos'],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ exports[`TinyMceWidget snapshots ImageUploadModal is not rendered 1`] = `
"selection": "hooks.selectedImage.selection",
"setEditorRef": undefined,
"setSelection": [MockFunction hooks.selectedImage.setSelection],
"staticRootUrl": undefined,
"studioEndpointUrl": "sOmEoThERvaLue.cOm",
"updateContent": [Function],
}
Expand Down Expand Up @@ -112,6 +113,7 @@ exports[`TinyMceWidget snapshots SourcecodeModal is not rendered 1`] = `
"selection": "hooks.selectedImage.selection",
"setEditorRef": undefined,
"setSelection": [MockFunction hooks.selectedImage.setSelection],
"staticRootUrl": undefined,
"studioEndpointUrl": "sOmEoThERvaLue.cOm",
"updateContent": [Function],
}
Expand Down Expand Up @@ -191,6 +193,7 @@ exports[`TinyMceWidget snapshots renders as expected with default behavior 1`] =
"selection": "hooks.selectedImage.selection",
"setEditorRef": undefined,
"setSelection": [MockFunction hooks.selectedImage.setSelection],
"staticRootUrl": undefined,
"studioEndpointUrl": "sOmEoThERvaLue.cOm",
"updateContent": [Function],
}
Expand Down
40 changes: 24 additions & 16 deletions src/editors/sharedComponents/TinyMceWidget/hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,20 +84,6 @@ export const replaceStaticWithAsset = ({
editorType,
lmsEndpointUrl,
}) => {
if (isLibraryKey(learningContextId)) {
// This function doesn't currently know how to deal with Library assets.
// Libraries don't mangle the path into an asset key–it might be sufficient
// to remove the initial "/" in a "/static/images/foo.png" link, and then
// set the base URL to the correct ComponentVersion base. If we let this
// function try to deal with Library assets, it would convert them in such a
// way that it wouldn't convert back later on, and we'd end up storing the
// incorrect OLX and breaking the display from that point forward.
//
// So until we handle it better, just disable static asset URL substitutions
// when dealing with Library content.
return false;
}

let content = initialContent;
let hasChanges = false;
const srcs = content.split(/(src="|src="|href="|href=&quot)/g).filter(
Expand All @@ -116,7 +102,15 @@ export const replaceStaticWithAsset = ({

// assets in expandable text areas do not support relative urls so all assets must have the lms
// endpoint prepended to the relative url
if (editorType === 'expandable') {
if (isLibraryKey(learningContextId)) {
// We are removing the initial "/" in a "/static/foo.png" link, and then
// set the base URL to an endpoint serving the draft version of an asset by
// its path.
/* istanbul ignore next */
if (isStatic) {
staticFullUrl = assetSrc.substring(1);
}
} else if (editorType === 'expandable') {
if (isCorrectAssetFormat) {
staticFullUrl = `${lmsEndpointUrl}${assetSrc}`;
} else {
Expand Down Expand Up @@ -261,9 +255,12 @@ export const editorConfig = ({
content,
minHeight,
learningContextId,
staticRootUrl,
}) => {
const lmsEndpointUrl = getConfig().LMS_BASE_URL;
const studioEndpointUrl = getConfig().STUDIO_BASE_URL;

const baseURL = staticRootUrl || lmsEndpointUrl;
const {
toolbar,
config,
Expand All @@ -290,7 +287,7 @@ export const editorConfig = ({
min_height: minHeight,
contextmenu: 'link table',
directionality: isLocaleRtl ? 'rtl' : 'ltr',
document_base_url: lmsEndpointUrl,
document_base_url: baseURL,
imagetools_cors_hosts: [removeProtocolFromUrl(lmsEndpointUrl), removeProtocolFromUrl(studioEndpointUrl)],
imagetools_toolbar: imageToolbar,
formats: { label: { inline: 'label' } },
Expand Down Expand Up @@ -436,6 +433,17 @@ export const setAssetToStaticUrl = ({ editorValue, lmsEndpointUrl }) => {
const updatedContent = content.replace(currentSrc, portableUrl);
content = updatedContent;
});

/* istanbul ignore next */
Ian2012 marked this conversation as resolved.
Show resolved Hide resolved
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 currentSrc = src.substring(0, src.search(/("|")/));
Ian2012 marked this conversation as resolved.
Show resolved Hide resolved
const updatedContent = content.replace(currentSrc, portableUrl);
content = updatedContent;
});
return content;
};

Expand Down
2 changes: 2 additions & 0 deletions src/editors/sharedComponents/TinyMceWidget/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ const TinyMceWidget = ({
images,
isLibrary,
onChange,
staticRootUrl,
...editorConfig
}) => {
const { isImgOpen, openImgModal, closeImgModal } = hooks.imgModalToggle();
Expand Down Expand Up @@ -85,6 +86,7 @@ const TinyMceWidget = ({
learningContextId,
images: imagesRef,
editorContentHtml,
staticRootUrl,
...imageSelection,
...editorConfig,
})
Expand Down