diff --git a/.eslintrc.js b/.eslintrc.js index ad8583bc7..b758dee97 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -10,6 +10,7 @@ const config = createConfig('eslint', { 'react-hooks/rules-of-hooks': 2, 'react-hooks/exhaustive-deps': 'off', 'no-promise-executor-return': 'off', + 'no-param-reassign': ['error', { props: false }], radix: 'off', }, }); diff --git a/src/editors/containers/ProblemEditor/components/EditProblemView/ExplanationWidget/__snapshots__/index.test.jsx.snap b/src/editors/containers/ProblemEditor/components/EditProblemView/ExplanationWidget/__snapshots__/index.test.jsx.snap index 76a7347a3..468b4b388 100644 --- a/src/editors/containers/ProblemEditor/components/EditProblemView/ExplanationWidget/__snapshots__/index.test.jsx.snap +++ b/src/editors/containers/ProblemEditor/components/EditProblemView/ExplanationWidget/__snapshots__/index.test.jsx.snap @@ -23,12 +23,12 @@ exports[`SolutionWidget render snapshot: renders correct default 1`] = ` /> <[object Object] + editorContentHtml="This is my question" editorType="solution" id="solution" minHeight={150} placeholder="Enter your explanation" setEditorRef={[MockFunction prepareEditorRef.setEditorRef]} - textValue="This is my question" /> `; diff --git a/src/editors/containers/ProblemEditor/components/EditProblemView/ExplanationWidget/index.jsx b/src/editors/containers/ProblemEditor/components/EditProblemView/ExplanationWidget/index.jsx index 2cd04cb95..0dccc5a95 100644 --- a/src/editors/containers/ProblemEditor/components/EditProblemView/ExplanationWidget/index.jsx +++ b/src/editors/containers/ProblemEditor/components/EditProblemView/ExplanationWidget/index.jsx @@ -28,7 +28,7 @@ export const ExplanationWidget = ({ id="solution" editorType="solution" editorRef={editorRef} - textValue={settings?.solutionExplanation} + editorContentHtml={settings?.solutionExplanation} setEditorRef={setEditorRef} minHeight={150} placeholder={intl.formatMessage(messages.placeholder)} diff --git a/src/editors/containers/ProblemEditor/components/EditProblemView/QuestionWidget/__snapshots__/index.test.jsx.snap b/src/editors/containers/ProblemEditor/components/EditProblemView/QuestionWidget/__snapshots__/index.test.jsx.snap index 6a0b69249..891bf8846 100644 --- a/src/editors/containers/ProblemEditor/components/EditProblemView/QuestionWidget/__snapshots__/index.test.jsx.snap +++ b/src/editors/containers/ProblemEditor/components/EditProblemView/QuestionWidget/__snapshots__/index.test.jsx.snap @@ -14,12 +14,12 @@ exports[`QuestionWidget render snapshot: renders correct default 1`] = ` /> <[object Object] + editorContentHtml="This is my question" editorType="question" id="question" minHeight={150} placeholder="Enter your question" setEditorRef={[MockFunction prepareEditorRef.setEditorRef]} - textValue="This is my question" /> `; diff --git a/src/editors/containers/ProblemEditor/components/EditProblemView/QuestionWidget/index.jsx b/src/editors/containers/ProblemEditor/components/EditProblemView/QuestionWidget/index.jsx index 0c95ad17a..df21809db 100644 --- a/src/editors/containers/ProblemEditor/components/EditProblemView/QuestionWidget/index.jsx +++ b/src/editors/containers/ProblemEditor/components/EditProblemView/QuestionWidget/index.jsx @@ -25,7 +25,7 @@ export const QuestionWidget = ({ id="question" editorType="question" editorRef={editorRef} - textValue={question} + editorContentHtml={question} setEditorRef={setEditorRef} minHeight={150} placeholder={intl.formatMessage(messages.placeholder)} diff --git a/src/editors/containers/ProblemEditor/components/EditProblemView/hooks.js b/src/editors/containers/ProblemEditor/components/EditProblemView/hooks.js index e831b3c69..5b338f127 100644 --- a/src/editors/containers/ProblemEditor/components/EditProblemView/hooks.js +++ b/src/editors/containers/ProblemEditor/components/EditProblemView/hooks.js @@ -124,7 +124,7 @@ export const checkForSettingDiscrepancy = ({ problem, ref, openSaveWarningModal const problemSettings = reactSettingsParser.getSettings(); const rawOlxSettings = reactSettingsParser.parseRawOlxSettings(); let isMismatched = false; - // console.log(rawOlxSettings); + Object.entries(rawOlxSettings).forEach(([key, value]) => { if (value !== problemSettings[key]) { isMismatched = true; diff --git a/src/editors/containers/TextEditor/__snapshots__/index.test.jsx.snap b/src/editors/containers/TextEditor/__snapshots__/index.test.jsx.snap index 81166806a..6f0f030bb 100644 --- a/src/editors/containers/TextEditor/__snapshots__/index.test.jsx.snap +++ b/src/editors/containers/TextEditor/__snapshots__/index.test.jsx.snap @@ -36,6 +36,7 @@ exports[`TextEditor snapshots block failed to load, Toast is shown 1`] = ` /> <[object Object] + editorContentHtml="eDiTablE Text" editorRef={ Object { "current": Object { @@ -48,7 +49,6 @@ exports[`TextEditor snapshots block failed to load, Toast is shown 1`] = ` initializeEditor={[MockFunction args.intializeEditor]} minHeight={500} setEditorRef={[MockFunction hooks.prepareEditorRef.setEditorRef]} - textValue="eDiTablE Text" /> @@ -194,6 +194,7 @@ exports[`TextEditor snapshots renders as expected with default behavior 1`] = ` /> <[object Object] + editorContentHtml="eDiTablE Text" editorRef={ Object { "current": Object { @@ -206,7 +207,6 @@ exports[`TextEditor snapshots renders as expected with default behavior 1`] = ` initializeEditor={[MockFunction args.intializeEditor]} minHeight={500} setEditorRef={[MockFunction hooks.prepareEditorRef.setEditorRef]} - textValue="eDiTablE Text" /> diff --git a/src/editors/containers/TextEditor/index.jsx b/src/editors/containers/TextEditor/index.jsx index d620c262d..052e098b4 100644 --- a/src/editors/containers/TextEditor/index.jsx +++ b/src/editors/containers/TextEditor/index.jsx @@ -48,7 +48,7 @@ export const TextEditor = ({ ( } .mce-content-body img { max-width: 100%; - height: auto; } .mce-content-body pre { margin: 1em 0; diff --git a/src/editors/sharedComponents/ExpandableTextArea/__snapshots__/index.test.jsx.snap b/src/editors/sharedComponents/ExpandableTextArea/__snapshots__/index.test.jsx.snap index 7e00c918b..2bae039f0 100644 --- a/src/editors/sharedComponents/ExpandableTextArea/__snapshots__/index.test.jsx.snap +++ b/src/editors/sharedComponents/ExpandableTextArea/__snapshots__/index.test.jsx.snap @@ -6,6 +6,7 @@ exports[`ExpandableTextArea snapshots renders as expected with default behavior className="expandable-mce error" > @@ -27,6 +27,7 @@ exports[`ExpandableTextArea snapshots renders error message 1`] = ` className="expandable-mce error" > diff --git a/src/editors/sharedComponents/ExpandableTextArea/index.jsx b/src/editors/sharedComponents/ExpandableTextArea/index.jsx index 5bf5f56e5..cf9025c5f 100644 --- a/src/editors/sharedComponents/ExpandableTextArea/index.jsx +++ b/src/editors/sharedComponents/ExpandableTextArea/index.jsx @@ -17,7 +17,7 @@ export const ExpandableTextArea = ({ <>
({ - onInputChange: (handler) => ({ 'hooks.onInputChange': handler }), -})); +const WrappedDimensionControls = () => { + const dimensions = hooks.dimensions('altText'); + + useEffect(() => { + dimensions.onImgLoad({ })({ target: { naturalWidth: 1517, naturalHeight: 803 } }); + }, []); + + return ; +}; + +const UnlockedDimensionControls = () => { + const dimensions = hooks.dimensions('altText'); + + useEffect(() => { + dimensions.onImgLoad({ })({ target: { naturalWidth: 1517, naturalHeight: 803 } }); + dimensions.unlock(); + }, []); + + return ; +}; describe('DimensionControls', () => { - const props = { - lockDims: { width: 12, height: 15 }, - locked: { 'props.locked': 'lockedValue' }, - isLocked: true, - value: { width: 20, height: 40 }, - // inject - intl: { formatMessage }, - }; - beforeEach(() => { - props.setWidth = jest.fn().mockName('props.setWidth'); - props.setHeight = jest.fn().mockName('props.setHeight'); - props.lock = jest.fn().mockName('props.lock'); - props.unlock = jest.fn().mockName('props.unlock'); - props.updateDimensions = jest.fn().mockName('props.updateDimensions'); - }); describe('render', () => { + const props = { + lockAspectRatio: { width: 4, height: 5 }, + locked: { 'props.locked': 'lockedValue' }, + isLocked: true, + value: { width: 20, height: 40 }, + // inject + intl: { formatMessage }, + }; + beforeEach(() => { + jest.spyOn(hooks, 'onInputChange').mockImplementation((handler) => ({ 'hooks.onInputChange': handler })); + props.setWidth = jest.fn().mockName('props.setWidth'); + props.setHeight = jest.fn().mockName('props.setHeight'); + props.lock = jest.fn().mockName('props.lock'); + props.unlock = jest.fn().mockName('props.unlock'); + props.updateDimensions = jest.fn().mockName('props.updateDimensions'); + }); + afterEach(() => { + jest.spyOn(hooks, 'onInputChange').mockRestore(); + }); test('snapshot', () => { expect(shallow()).toMatchSnapshot(); }); @@ -38,4 +65,76 @@ describe('DimensionControls', () => { expect(el).toMatchSnapshot(); }); }); + describe('component tests for dimensions', () => { + beforeEach(() => { + paragon.Form.Group = jest.fn().mockImplementation(({ children }) => ( +
{children}
+ )); + paragon.Form.Label = jest.fn().mockImplementation(({ children }) => ( +
{children}
+ )); + // eslint-disable-next-line no-import-assign + paragon.Icon = jest.fn().mockImplementation(({ children }) => ( +
{children}
+ )); + // eslint-disable-next-line no-import-assign + paragon.IconButton = jest.fn().mockImplementation(({ children }) => ( +
{children}
+ )); + paragon.Form.Control = jest.fn().mockImplementation(({ value, onChange, onBlur }) => ( + + )); + // eslint-disable-next-line no-import-assign + icons.Locked = jest.fn().mockImplementation(() => {}); + // eslint-disable-next-line no-import-assign + icons.Unlocked = jest.fn().mockImplementation(() => {}); + }); + afterEach(() => { + paragon.Form.Group.mockRestore(); + paragon.Form.Label.mockRestore(); + paragon.Form.Control.mockRestore(); + paragon.Icon.mockRestore(); + paragon.IconButton.mockRestore(); + icons.Locked.mockRestore(); + icons.Unlocked.mockRestore(); + }); + + it('renders with initial dimensions', () => { + const { container } = render(); + const widthInput = container.querySelector('.formControl'); + expect(widthInput.value).toBe('1517'); + }); + + it('resizes dimensions proportionally', async () => { + const { container } = render(); + const widthInput = container.querySelector('.formControl'); + expect(widthInput.value).toBe('1517'); + fireEvent.change(widthInput, { target: { value: 758 } }); + await waitFor(() => { + expect(container.querySelectorAll('.formControl')[0].value).toBe('758'); + }); + fireEvent.blur(widthInput); + await waitFor(() => { + expect(container.querySelectorAll('.formControl')[0].value).toBe('758'); + expect(container.querySelectorAll('.formControl')[1].value).toBe('401'); + }); + screen.debug(); + }); + + it('resizes only changed dimension when unlocked', async () => { + const { container } = render(); + const widthInput = container.querySelector('.formControl'); + expect(widthInput.value).toBe('1517'); + fireEvent.change(widthInput, { target: { value: 758 } }); + await waitFor(() => { + expect(container.querySelectorAll('.formControl')[0].value).toBe('758'); + }); + fireEvent.blur(widthInput); + await waitFor(() => { + expect(container.querySelectorAll('.formControl')[0].value).toBe('758'); + expect(container.querySelectorAll('.formControl')[1].value).toBe('803'); + }); + screen.debug(); + }); + }); }); diff --git a/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/__snapshots__/index.test.jsx.snap b/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/__snapshots__/index.test.jsx.snap index c0459220a..8d728271b 100644 --- a/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/__snapshots__/index.test.jsx.snap +++ b/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/__snapshots__/index.test.jsx.snap @@ -12,7 +12,7 @@ exports[`ImageSettingsModal render snapshot 1`] = ` "altText": Object { "error": Object { "dismiss": [MockFunction], - "show": "sHoW", + "show": true, }, "isDecorative": false, "value": "alternative Taxes", @@ -45,7 +45,7 @@ exports[`ImageSettingsModal render snapshot 1`] = ` React.useState(val), // eslint-disable-next-line react-hooks/rules-of-hooks - lockDims: (val) => React.useState(val), - // eslint-disable-next-line react-hooks/rules-of-hooks - lockInitialized: (val) => React.useState(val), + lockAspectRatio: (val) => React.useState(val), }; export const dimKeys = StrictDict({ @@ -32,12 +30,21 @@ export const dimKeys = StrictDict({ /** * findGcd(numerator, denominator) - * Find the greatest common denominator of a ratio or fraction. + * Find the greatest common denominator of a ratio or fraction, which may be 1. * @param {number} numerator - ratio numerator * @param {number} denominator - ratio denominator * @return {number} - ratio greatest common denominator */ -export const findGcd = (a, b) => (b ? findGcd(b, a % b) : a); +export const findGcd = (a, b) => { + const gcd = b ? findGcd(b, a % b) : a; + + if (gcd === 1 || [a, b].some(v => !Number.isInteger(v / gcd))) { + return 1; + } + + return gcd; +}; + const checkEqual = (d1, d2) => (d1.height === d2.height && d1.width === d2.width); /** @@ -52,34 +59,38 @@ export const getValidDimensions = ({ dimensions, local, isLocked, - lockDims, + lockAspectRatio, }) => { + // if lock is not active, just return new dimensions. + // If lock is active, but dimensions have not changed, also just return new dimensions. if (!isLocked || checkEqual(local, dimensions)) { return local; } - const out = {}; - let iter; - const isMin = dimensions.height === lockDims.height; + const out = {}; + + // changed key is value of local height if that has changed, otherwise width. const keys = (local.height !== dimensions.height) ? { changed: dimKeys.height, other: dimKeys.width } : { changed: dimKeys.width, other: dimKeys.height }; - const direction = local[keys.changed] > dimensions[keys.changed] ? 1 : -1; - - // don't move down if already at minimum size - if (direction < 0 && isMin) { return dimensions; } - // find closest valid iteration of the changed field - iter = Math.max(Math.round(local[keys.changed] / lockDims[keys.changed]), 1); - // if closest valid iteration is current iteration, move one iteration in the change direction - if (iter === (dimensions[keys.changed] / lockDims[keys.changed])) { iter += direction; } - - out[keys.changed] = Math.round(iter * lockDims[keys.changed]); - out[keys.other] = Math.round(out[keys.changed] * (lockDims[keys.other] / lockDims[keys.changed])); + out[keys.changed] = local[keys.changed]; + out[keys.other] = Math.round((local[keys.changed] * lockAspectRatio[keys.other]) / lockAspectRatio[keys.changed]); return out; }; +/** + * reduceDimensions(width, height) + * reduces both values by dividing by their greates common denominator (which can simply be 1). + * @return {Array} [width, height] + */ +export const reduceDimensions = (width, height) => { + const gcd = module.findGcd(width, height); + + return [width / gcd, height / gcd]; +}; + /** * dimensionLockHooks({ dimensions }) * Returns a set of hooks pertaining to the dimension locks. @@ -88,28 +99,26 @@ export const getValidDimensions = ({ * @return {obj} - dimension lock hooks * {func} initializeLock - enable the lock mechanism * {bool} isLocked - are dimensions locked? - * {obj} lockDims - image dimensions ({ height, width }) + * {obj} lockAspectRatio - image dimensions ({ height, width }) * {func} lock - lock the dimensions * {func} unlock - unlock the dimensions */ export const dimensionLockHooks = () => { - const [lockDims, setLockDims] = module.state.lockDims(null); + const [lockAspectRatio, setLockAspectRatio] = module.state.lockAspectRatio(null); const [isLocked, setIsLocked] = module.state.isLocked(true); const initializeLock = ({ width, height }) => { - // find minimum viable increment - let gcd = module.findGcd(width, height); - if ([width, height].some(v => !Number.isInteger(v / gcd))) { - gcd = 1; - } - setLockDims({ width: width / gcd, height: height / gcd }); + // width and height are treated as a fraction and reduced. + const [w, h] = reduceDimensions(width, height); + + setLockAspectRatio({ width: w, height: h }); }; return { initializeLock, isLocked, lock: () => setIsLocked(true), - lockDims, + lockAspectRatio, unlock: () => setIsLocked(false), }; }; @@ -144,6 +153,7 @@ export const dimensionLockHooks = () => { export const dimensionHooks = (altTextHook) => { const [dimensions, setDimensions] = module.state.dimensions(null); const [local, setLocal] = module.state.local(null); + const setAll = ({ height, width, altText }) => { if (altText === '' || altText) { if (altText === '') { @@ -154,11 +164,30 @@ export const dimensionHooks = (altTextHook) => { setDimensions({ height, width }); setLocal({ height, width }); }; + + const setHeight = (height) => { + if (height.match(/[0-9]+[%]{1}/)) { + const heightPercent = height.match(/[0-9]+[%]{1}/)[0]; + setLocal({ ...local, height: heightPercent }); + } else if (height.match(/[0-9]/)) { + setLocal({ ...local, height: parseInt(height, 10) }); + } + }; + + const setWidth = (width) => { + if (width.match(/[0-9]+[%]{1}/)) { + const widthPercent = width.match(/[0-9]+[%]{1}/)[0]; + setLocal({ ...local, width: widthPercent }); + } else if (width.match(/[0-9]/)) { + setLocal({ ...local, width: parseInt(width, 10) }); + } + }; + const { initializeLock, isLocked, lock, - lockDims, + lockAspectRatio, unlock, } = module.dimensionLockHooks({ dimensions }); @@ -166,33 +195,19 @@ export const dimensionHooks = (altTextHook) => { onImgLoad: (selection) => ({ target: img }) => { const imageDims = { height: img.naturalHeight, width: img.naturalWidth }; setAll(selection.height ? selection : imageDims); - initializeLock(imageDims); + initializeLock(selection.height ? selection : imageDims); }, isLocked, lock, unlock, value: local, - setHeight: (height) => { - if (height.match(/[0-9]+[%]{1}/)) { - const heightPercent = height.match(/[0-9]+[%]{1}/)[0]; - setLocal({ ...local, height: heightPercent }); - } else if (height.match(/[0-9]/)) { - setLocal({ ...local, height: parseInt(height, 10) }); - } - }, - setWidth: (width) => { - if (width.match(/[0-9]+[%]{1}/)) { - const widthPercent = width.match(/[0-9]+[%]{1}/)[0]; - setLocal({ ...local, width: widthPercent }); - } else if (width.match(/[0-9]/)) { - setLocal({ ...local, width: parseInt(width, 10) }); - } - }, + setHeight, + setWidth, updateDimensions: () => setAll(module.getValidDimensions({ dimensions, local, isLocked, - lockDims, + lockAspectRatio, })), }; }; diff --git a/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/hooks.test.js b/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/hooks.test.js index 2827381a3..932577cc4 100644 --- a/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/hooks.test.js +++ b/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/hooks.test.js @@ -40,8 +40,7 @@ describe('state values', () => { test('provides isDecorative state value', () => testStateMethod(state.keys.isDecorative)); test('provides isLocked state value', () => testStateMethod(state.keys.isLocked)); test('provides local state value', () => testStateMethod(state.keys.local)); - test('provides lockDims state value', () => testStateMethod(state.keys.lockDims)); - test('provides lockInitialized state value', () => testStateMethod(state.keys.lockInitialized)); + test('provides lockAspectRatio state value', () => testStateMethod(state.keys.lockAspectRatio)); }); describe('ImageSettingsModal hooks', () => { @@ -55,7 +54,7 @@ describe('ImageSettingsModal hooks', () => { dimensions: simpleDims, local: reducedDims, isLocked: false, - lockDims: simpleDims, + lockAspectRatio: simpleDims, })).toEqual(reducedDims); }); it('returns local dimensions if the same as stored', () => { @@ -63,64 +62,48 @@ describe('ImageSettingsModal hooks', () => { dimensions: simpleDims, local: simpleDims, isLocked: true, - lockDims: reducedDims, + lockAspectRatio: reducedDims, })).toEqual(simpleDims); }); - describe('decreasing change when at minimum valid increment', () => { - it('returns current dimensions', () => { - const dimensions = { ...reducedDims }; - const lockDims = { ...dimensions }; - let local = { ...dimensions, width: dimensions.width - 1 }; - expect( - hooks.getValidDimensions({ - dimensions, - isLocked: true, - local, - lockDims, - }), - ).toEqual(dimensions); - local = { ...dimensions, height: dimensions.height - 1 }; - expect( - hooks.getValidDimensions({ - dimensions, - isLocked: true, - local, - lockDims, - }), - ).toEqual(dimensions); - }); - }); - describe('valid change', () => { - it( - 'returns the nearest valid pair of dimensions in the change direction', + describe('valid change when aspect ratio is locked', () => { + describe( + 'keeps changed dimension and keeps the other dimension proportional but rounded', () => { const [w, h] = [7, 13]; - const values = [ - // bumps up if direction is up but nearest is current - [[w + 1, h], [w * 2, h * 2]], - [[w + 1, h], [w * 2, h * 2]], - // bumps up if just below next - [[w, 2 * h - 1], [w * 2, h * 2]], - [[w, 2 * h - 1], [w * 2, h * 2]], - // rounds down to next if that is closest - [[w, 2 * h + 1], [w * 2, h * 2]], - [[w, 2 * h + 1], [w * 2, h * 2]], - // ensure is not locked to second iteration, by getting close to 3rd - [[w, 3 * h - 1], [w * 3, h * 3]], - [[w, 3 * h - 1], [w * 3, h * 3]], - ]; - values.forEach(([local, expected]) => { + + const testDimensions = (newDimensions, expected) => { const dimensions = { width: w, height: h }; expect(hooks.getValidDimensions({ dimensions, - local: { width: local[0], height: local[1] }, - lockDims: { ...dimensions }, + local: { width: newDimensions[0], height: newDimensions[1] }, + lockAspectRatio: { ...dimensions }, isLocked: true, })).toEqual({ width: expected[0], height: expected[1] }); + }; + + it('if width is increased, increases and rounds height to stay proportional', () => { + testDimensions([8, h], [8, 15]); + }); + it('if height is increased, increases and rounds width to stay proportional', () => { + testDimensions([w, 25], [13, 25]); + }); + it('if width is decreased, decreases and rounds height to stay proportional', () => { + testDimensions([6, h], [6, 11]); + }); + it('if height is decreased, decreases and rounds width to stay proportional', () => { + testDimensions([7, 10], [5, 10]); }); }, ); }); + it('calculates new dimensions proportionally and correctly when lock is active', () => { + expect(hooks.getValidDimensions({ + dimensions: { width: 1517, height: 803 }, + local: { width: 758, height: 803 }, + isLocked: true, + lockAspectRatio: { width: 1517, height: 803 }, + })).toEqual({ width: 758, height: 401 }); + }); }); describe('dimensionLockHooks', () => { beforeEach(() => { @@ -130,21 +113,21 @@ describe('ImageSettingsModal hooks', () => { afterEach(() => { state.restore(); }); - test('lockDims defaults to null', () => { - expect(hook.lockDims).toEqual(null); + test('lockAspectRatio defaults to null', () => { + expect(hook.lockAspectRatio).toEqual(null); }); test('isLocked defaults to true', () => { expect(hook.isLocked).toEqual(true); }); describe('initializeLock', () => { - it('calls setLockDims with the passed dimensions divided by their gcd', () => { + it('calls setLockAspectRatio with the passed dimensions divided by their gcd', () => { hook.initializeLock(multiDims); - expect(state.setState.lockDims).toHaveBeenCalledWith(reducedDims); + expect(state.setState.lockAspectRatio).toHaveBeenCalledWith(reducedDims); }); it('returns the values themselves if they have no gcd', () => { - jest.spyOn(hooks, hookKeys.findGcd).mockReturnValueOnce(2); + jest.spyOn(hooks, hookKeys.findGcd).mockReturnValueOnce(1); hook.initializeLock(simpleDims); - expect(state.setState.lockDims).toHaveBeenCalledWith(simpleDims); + expect(state.setState.lockAspectRatio).toHaveBeenCalledWith(simpleDims); }); }); test('lock sets isLocked to true', () => { @@ -225,14 +208,14 @@ describe('ImageSettingsModal hooks', () => { const getValidDimensions = (args) => ({ ...testDims(args), junk: 'data' }); state.mockVal(state.keys.isLocked, true); state.mockVal(state.keys.dimensions, simpleDims); - state.mockVal(state.keys.lockDims, reducedDims); + state.mockVal(state.keys.lockAspectRatio, reducedDims); state.mockVal(state.keys.local, multiDims); jest.spyOn(hooks, hookKeys.getValidDimensions).mockImplementationOnce(getValidDimensions); hook = hooks.dimensionHooks(); hook.updateDimensions(); const expected = testDims({ dimensions: simpleDims, - lockDims: reducedDims, + lockAspectRatio: reducedDims, local: multiDims, isLocked: true, }); @@ -245,8 +228,8 @@ describe('ImageSettingsModal hooks', () => { describe('altTextHooks', () => { const value = 'myVAL'; const isDecorative = true; - const showAltTextDismissibleError = 'dismiSSiBLE'; - const showAltTextSubmissionError = 'subMISsion'; + const showAltTextDismissibleError = true; + const showAltTextSubmissionError = true; beforeEach(() => { state.mock(); hook = hooks.altTextHooks(); @@ -389,4 +372,16 @@ describe('ImageSettingsModal hooks', () => { expect(props.saveToEditor).not.toHaveBeenCalled(); }); }); + describe('findGcd', () => { + it('should return correct gcd', () => { + expect(hooks.findGcd(9, 12)).toBe(3); + expect(hooks.findGcd(3, 4)).toBe(1); + }); + }); + describe('reduceDimensions', () => { + it('should return correct gcd', () => { + expect(hooks.reduceDimensions(9, 12)).toEqual([3, 4]); + expect(hooks.reduceDimensions(7, 8)).toEqual([7, 8]); + }); + }); }); diff --git a/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/index.test.jsx b/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/index.test.jsx index 95662a7b8..333d2db6b 100644 --- a/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/index.test.jsx +++ b/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/index.test.jsx @@ -10,7 +10,7 @@ jest.mock('./DimensionControls', () => 'DimensionControls'); jest.mock('./hooks', () => ({ altText: () => ({ error: { - show: 'sHoW', + show: true, dismiss: jest.fn(), }, isDecorative: false, diff --git a/src/editors/sharedComponents/ImageUploadModal/SelectImageModal/hooks.js b/src/editors/sharedComponents/ImageUploadModal/SelectImageModal/hooks.js index ab0482aae..efc396762 100644 --- a/src/editors/sharedComponents/ImageUploadModal/SelectImageModal/hooks.js +++ b/src/editors/sharedComponents/ImageUploadModal/SelectImageModal/hooks.js @@ -34,7 +34,7 @@ export const searchAndSortHooks = () => { }; export const filteredList = ({ searchString, imageList }) => ( - imageList.filter(({ displayName }) => displayName.toLowerCase().includes(searchString.toLowerCase())) + imageList.filter(({ displayName }) => displayName?.toLowerCase().includes(searchString?.toLowerCase())) ); export const displayList = ({ sortBy, searchString, images }) => ( diff --git a/src/editors/sharedComponents/ImageUploadModal/SelectImageModal/index.jsx b/src/editors/sharedComponents/ImageUploadModal/SelectImageModal/index.jsx index c56637a66..54c75e793 100644 --- a/src/editors/sharedComponents/ImageUploadModal/SelectImageModal/index.jsx +++ b/src/editors/sharedComponents/ImageUploadModal/SelectImageModal/index.jsx @@ -25,7 +25,7 @@ export const SelectImageModal = ({ galleryProps, searchSortProps, selectBtnProps, - } = hooks.imgHooks({ setSelection, clearSelection, images }); + } = hooks.imgHooks({ setSelection, clearSelection, images: images.current }); const modalMessages = { confirmMsg: messages.nextButtonLabel, diff --git a/src/editors/sharedComponents/ImageUploadModal/SelectImageModal/index.test.jsx b/src/editors/sharedComponents/ImageUploadModal/SelectImageModal/index.test.jsx index d834cfdfe..9de790cbc 100644 --- a/src/editors/sharedComponents/ImageUploadModal/SelectImageModal/index.test.jsx +++ b/src/editors/sharedComponents/ImageUploadModal/SelectImageModal/index.test.jsx @@ -6,6 +6,30 @@ import SelectionModal from '../../SelectionModal'; import hooks from './hooks'; import { SelectImageModal } from '.'; +const mockImage = { + displayName: 'DALL·E 2023-03-10.png', + contentType: 'image/png', + dateAdded: 1682009100000, + url: '/asset-v1:TestX+Test01+Test0101+type@asset+block@DALL_E_2023-03-10.png', + externalUrl: 'http://localhost:18000/asset-v1:TestX+Test01+Test0101+type@asset+block@DALL_E_2023-03-10.png', + portableUrl: '/static/DALL_E_2023-03-10.png', + thumbnail: '/asset-v1:TestX+Test01+Test0101+type@thumbnail+block@DALL_E_2023-03-10.jpg', + locked: false, + staticFullUrl: '/assets/courseware/v1/af2bf9ac70804e54c534107160a8e51e/asset-v1:TestX+Test01+Test0101+type@asset+block@DALL_E_2023-03-10.png', + id: 'asset-v1:TestX+Test01+Test0101+type@asset+block@DALL_E_2023-03-10.png', + width: 100, + height: 150, +}; + +const mockImagesRef = { current: [mockImage] }; + +jest.mock('../../BaseModal', () => 'BaseModal'); +jest.mock('../../FileInput', () => 'FileInput'); +jest.mock('../../SelectionModal/Gallery', () => 'Gallery'); +jest.mock('../../SelectionModal/SearchSort', () => 'SearchSort'); +jest.mock('../../ErrorAlerts/FetchErrorAlert', () => 'FetchErrorAlert'); +jest.mock('../../ErrorAlerts/UploadErrorAlert', () => 'UploadErrorAlert'); +jest.mock('../..//ErrorAlerts/ErrorAlert', () => 'ErrorAlert'); jest.mock('../../SelectionModal', () => 'SelectionModal'); jest.mock('./hooks', () => ({ @@ -56,6 +80,7 @@ describe('SelectImageModal', () => { close: jest.fn().mockName('props.close'), setSelection: jest.fn().mockName('props.setSelection'), clearSelection: jest.fn().mockName('props.clearSelection'), + images: mockImagesRef, intl: { formatMessage }, }; let el; diff --git a/src/editors/sharedComponents/ImageUploadModal/__snapshots__/index.test.jsx.snap b/src/editors/sharedComponents/ImageUploadModal/__snapshots__/index.test.jsx.snap index a3c78ef73..2850782a7 100644 --- a/src/editors/sharedComponents/ImageUploadModal/__snapshots__/index.test.jsx.snap +++ b/src/editors/sharedComponents/ImageUploadModal/__snapshots__/index.test.jsx.snap @@ -4,6 +4,55 @@ exports[`ImageUploadModal component snapshot: no selection (Select Image Modal) +`; + +exports[`ImageUploadModal component snapshot: selection has no externalUrl (Select Image Modal) 1`] = ` + @@ -11,10 +60,31 @@ exports[`ImageUploadModal component snapshot: no selection (Select Image Modal) exports[`ImageUploadModal component snapshot: with selection content (ImageSettingsUpload) 1`] = ` ( Object.keys(props).map((key) => `${key}="${props[key]}"`).join(' ') @@ -17,8 +18,8 @@ export const imgProps = ({ lmsEndpointUrl, editorType, }) => { - let url = selection.externalUrl; - if (url.startsWith(lmsEndpointUrl) && editorType !== 'expandable') { + let url = selection?.externalUrl; + if (url?.startsWith(lmsEndpointUrl) && editorType !== 'expandable') { const sourceEndIndex = lmsEndpointUrl.length; url = url.substring(sourceEndIndex); } @@ -30,28 +31,61 @@ export const imgProps = ({ }; }; +export const saveToEditor = ({ + settings, selection, lmsEndpointUrl, editorType, editorRef, +}) => { + const newImgTag = module.hooks.imgTag({ + settings, + selection, + lmsEndpointUrl, + editorType, + }); + + editorRef.current.execCommand( + tinyMCEKeys.commands.insertContent, + false, + newImgTag, + ); +}; + +export const updateImagesRef = ({ + images, selection, height, width, newImage, +}) => { + const { result: mappedImages, foundMatch: imageAlreadyExists } = updateImageDimensions({ + images: images.current, url: selection.externalUrl, height, width, + }); + + images.current = imageAlreadyExists ? mappedImages : [...images.current, newImage]; +}; + +export const updateReactState = ({ + settings, selection, setSelection, images, +}) => { + const { height, width } = settings.dimensions; + const newImage = { + externalUrl: selection.externalUrl, + altText: settings.altText, + width, + height, + }; + + updateImagesRef({ + images, selection, height, width, newImage, + }); + + setSelection(newImage); +}; + export const hooks = { createSaveCallback: ({ close, - editorRef, - editorType, - setSelection, - selection, - lmsEndpointUrl, + ...args }) => ( settings, ) => { - editorRef.current.execCommand( - tinyMCEKeys.commands.insertContent, - false, - module.hooks.imgTag({ - settings, - selection, - lmsEndpointUrl, - editorType, - }), - ); - setSelection(null); + saveToEditor({ settings, ...args }); + updateReactState({ settings, ...args }); + close(); }, onClose: ({ clearSelection, close }) => () => { @@ -72,6 +106,11 @@ export const hooks = { }); return ``; }, + updateReactState, + updateImagesRef, + saveToEditor, + imgProps, + propsString, }; export const ImageUploadModal = ({ @@ -86,15 +125,17 @@ export const ImageUploadModal = ({ editorType, lmsEndpointUrl, }) => { - if (selection) { + if (selection && selection.externalUrl) { return ( 'ImageSettingsModal'); jest.mock('./SelectImageModal', () => 'SelectImageModal'); @@ -22,7 +23,28 @@ const settings = { }, }; +const mockImage = { + displayName: 'DALL·E 2023-03-10.png', + contentType: 'image/png', + dateAdded: 1682009100000, + url: '/asset-v1:TestX+Test01+Test0101+type@asset+block@DALL_E_2023-03-10.png', + externalUrl: 'http://localhost:18000/asset-v1:TestX+Test01+Test0101+type@asset+block@DALL_E_2023-03-10.png', + portableUrl: '/static/DALL_E_2023-03-10.png', + thumbnail: '/asset-v1:TestX+Test01+Test0101+type@thumbnail+block@DALL_E_2023-03-10.jpg', + locked: false, + staticFullUrl: '/assets/courseware/v1/af2bf9ac70804e54c534107160a8e51e/asset-v1:TestX+Test01+Test0101+type@asset+block@DALL_E_2023-03-10.png', + id: 'asset-v1:TestX+Test01+Test0101+type@asset+block@DALL_E_2023-03-10.png', + width: 100, + height: 150, +}; + +let mockImagesRef = { current: [mockImage] }; + describe('ImageUploadModal', () => { + beforeEach(() => { + mockImagesRef = { current: [mockImage] }; + }); + describe('hooks', () => { describe('imgTag', () => { const selection = { externalUrl: 'sOmEuRl.cOm' }; @@ -52,36 +74,62 @@ describe('ImageUploadModal', () => { }); }); describe('createSaveCallback', () => { + const updateImageDimensionsSpy = jest.spyOn(tinyMceHooks, 'updateImageDimensions'); const close = jest.fn(); const execCommandMock = jest.fn(); const editorRef = { current: { some: 'dATa', execCommand: execCommandMock } }; const setSelection = jest.fn(); - const selection = jest.fn(); + const selection = { externalUrl: 'sOmEuRl.cOm' }; const lmsEndpointUrl = 'sOmE'; + const images = mockImagesRef; let output; + const newImage = { + altText: settings.altText, + externalUrl: selection.externalUrl, + width: settings.dimensions.width, + height: settings.dimensions.height, + }; + beforeEach(() => { output = module.hooks.createSaveCallback({ - close, editorRef, setSelection, selection, lmsEndpointUrl, + close, settings, images, editorRef, setSelection, selection, lmsEndpointUrl, }); }); afterEach(() => { jest.clearAllMocks(); }); - test('It creates a callback, that when called, inserts to the editor, sets the selection to be null, and calls close', () => { - jest.spyOn(module.hooks, hookKeys.imgTag) - .mockImplementationOnce((props) => ({ selection, settings: props.settings, lmsEndpointUrl })); - expect(execCommandMock).not.toBeCalled(); - expect(setSelection).not.toBeCalled(); - expect(close).not.toBeCalled(); - output(settings); - expect(execCommandMock).toBeCalledWith( - tinyMCEKeys.commands.insertContent, - false, - { selection, settings, lmsEndpointUrl }, - ); - expect(setSelection).toBeCalledWith(null); - expect(close).toBeCalled(); - }); + test( + `It creates a callback, that when called, inserts to the editor, sets the selection to the current element, + adds new image to the images ref, and calls close`, + () => { + jest.spyOn(module.hooks, hookKeys.imgTag) + .mockImplementationOnce((props) => ({ selection, settings: props.settings, lmsEndpointUrl })); + + expect(execCommandMock).not.toBeCalled(); + expect(setSelection).not.toBeCalled(); + expect(close).not.toBeCalled(); + expect(images.current).toEqual([mockImage]); + + output(settings); + + expect(execCommandMock).toBeCalledWith( + tinyMCEKeys.commands.insertContent, + false, + { selection, settings, lmsEndpointUrl }, + ); + expect(setSelection).toBeCalledWith(newImage); + expect(updateImageDimensionsSpy.mock.calls.length).toBe(1); + expect(updateImageDimensionsSpy).toBeCalledWith({ + images: [mockImage], + url: selection.externalUrl, + width: settings.dimensions.width, + height: settings.dimensions.height, + }); + expect(updateImageDimensionsSpy.mock.results[0].value.foundMatch).toBe(false); + expect(images.current).toEqual([mockImage, newImage]); + expect(close).toBeCalled(); + }, + ); }); describe('onClose', () => { it('takes and calls clearSelection and close callbacks', () => { @@ -104,9 +152,12 @@ describe('ImageUploadModal', () => { isOpen: false, close: jest.fn().mockName('props.close'), clearSelection: jest.fn().mockName('props.clearSelection'), - selection: { some: 'images' }, + selection: { some: 'images', externalUrl: 'sOmEuRl.cOm' }, setSelection: jest.fn().mockName('props.setSelection'), lmsEndpointUrl: 'sOmE', + images: { + current: [mockImage], + }, }; module.hooks = { createSaveCallback: jest.fn().mockName('hooks.createSaveCallback'), @@ -119,6 +170,9 @@ describe('ImageUploadModal', () => { test('snapshot: with selection content (ImageSettingsUpload)', () => { expect(shallow()).toMatchSnapshot(); }); + test('snapshot: selection has no externalUrl (Select Image Modal)', () => { + expect(shallow()).toMatchSnapshot(); + }); test('snapshot: no selection (Select Image Modal)', () => { expect(shallow()).toMatchSnapshot(); }); diff --git a/src/editors/sharedComponents/TinyMceWidget/__snapshots__/index.test.jsx.snap b/src/editors/sharedComponents/TinyMceWidget/__snapshots__/index.test.jsx.snap index 0a080b7f4..3be7c557b 100644 --- a/src/editors/sharedComponents/TinyMceWidget/__snapshots__/index.test.jsx.snap +++ b/src/editors/sharedComponents/TinyMceWidget/__snapshots__/index.test.jsx.snap @@ -18,21 +18,25 @@ exports[`TinyMceWidget snapshots ImageUploadModal is not rendered 1`] = ` editorConfig={ Object { "clearSelection": [MockFunction hooks.selectedImage.clearSelection], + "editorContentHtml": undefined, "editorRef": Object { "current": Object { "value": "something", }, }, "editorType": "text", - "images": Array [ - Object { - "staTICUrl": "/assets/sOmEaSsET", - }, - ], + "images": Object { + "current": Array [ + Object { + "externalUrl": "/assets/sOmEaSsET", + }, + ], + }, "isLibrary": true, "lmsEndpointUrl": "sOmEvaLue.cOm", "openImgModal": [MockFunction modal.openModal], "openSourceCodeModal": [MockFunction modal.openModal], + "selection": "hooks.selectedImage.selection", "setSelection": [MockFunction hooks.selectedImage.setSelection], "studioEndpointUrl": "sOmEoThERvaLue.cOm", } @@ -56,11 +60,13 @@ exports[`TinyMceWidget snapshots SourcecodeModal is not rendered 1`] = ` } editorType="problem" images={ - Array [ - Object { - "staTICUrl": "/assets/sOmEaSsET", - }, - ] + Object { + "current": Array [ + Object { + "externalUrl": "/assets/sOmEaSsET", + }, + ], + } } isOpen={false} lmsEndpointUrl="sOmEvaLue.cOm" @@ -72,21 +78,25 @@ exports[`TinyMceWidget snapshots SourcecodeModal is not rendered 1`] = ` editorConfig={ Object { "clearSelection": [MockFunction hooks.selectedImage.clearSelection], + "editorContentHtml": undefined, "editorRef": Object { "current": Object { "value": "something", }, }, "editorType": "problem", - "images": Array [ - Object { - "staTICUrl": "/assets/sOmEaSsET", - }, - ], + "images": Object { + "current": Array [ + Object { + "externalUrl": "/assets/sOmEaSsET", + }, + ], + }, "isLibrary": false, "lmsEndpointUrl": "sOmEvaLue.cOm", "openImgModal": [MockFunction modal.openModal], "openSourceCodeModal": [MockFunction modal.openModal], + "selection": "hooks.selectedImage.selection", "setSelection": [MockFunction hooks.selectedImage.setSelection], "studioEndpointUrl": "sOmEoThERvaLue.cOm", } @@ -110,11 +120,13 @@ exports[`TinyMceWidget snapshots renders as expected with default behavior 1`] = } editorType="text" images={ - Array [ - Object { - "staTICUrl": "/assets/sOmEaSsET", - }, - ] + Object { + "current": Array [ + Object { + "externalUrl": "/assets/sOmEaSsET", + }, + ], + } } isOpen={false} lmsEndpointUrl="sOmEvaLue.cOm" @@ -137,21 +149,25 @@ exports[`TinyMceWidget snapshots renders as expected with default behavior 1`] = editorConfig={ Object { "clearSelection": [MockFunction hooks.selectedImage.clearSelection], + "editorContentHtml": undefined, "editorRef": Object { "current": Object { "value": "something", }, }, "editorType": "text", - "images": Array [ - Object { - "staTICUrl": "/assets/sOmEaSsET", - }, - ], + "images": Object { + "current": Array [ + Object { + "externalUrl": "/assets/sOmEaSsET", + }, + ], + }, "isLibrary": false, "lmsEndpointUrl": "sOmEvaLue.cOm", "openImgModal": [MockFunction modal.openModal], "openSourceCodeModal": [MockFunction modal.openModal], + "selection": "hooks.selectedImage.selection", "setSelection": [MockFunction hooks.selectedImage.setSelection], "studioEndpointUrl": "sOmEoThERvaLue.cOm", } diff --git a/src/editors/sharedComponents/TinyMceWidget/hooks.js b/src/editors/sharedComponents/TinyMceWidget/hooks.js index 21d53661f..70763d352 100644 --- a/src/editors/sharedComponents/TinyMceWidget/hooks.js +++ b/src/editors/sharedComponents/TinyMceWidget/hooks.js @@ -21,6 +21,25 @@ export const state = StrictDict({ refReady: (val) => useState(val), }); +export const addImagesAndDimensionsToRef = ({ imagesRef, assets, editorContentHtml }) => { + const imagesWithDimensions = module.filterAssets({ assets }).map((image) => { + const imageFragment = module.getImageFromHtmlString(editorContentHtml, image.url); + return { ...image, width: imageFragment?.width, height: imageFragment?.height }; + }); + + imagesRef.current = imagesWithDimensions; +}; + +export const useImages = ({ assets, editorContentHtml }) => { + const imagesRef = useRef([]); + + useEffect(() => { + module.addImagesAndDimensionsToRef({ imagesRef, assets, editorContentHtml }); + }, []); + + return { imagesRef }; +}; + export const parseContentForLabels = ({ editor, updateContent }) => { let content = editor.getContent(); if (content && content?.length > 0) { @@ -90,13 +109,31 @@ export const replaceStaticwithAsset = ({ }); }; +export const getImageResizeHandler = ({ editor, imagesRef, setImage }) => () => { + const { + src, alt, width, height, + } = editor.selection.getNode(); + + imagesRef.current = module.updateImageDimensions({ + images: imagesRef.current, url: src, width, height, + }).result; + + setImage({ + externalUrl: src, + altText: alt, + width, + height, + }); +}; + export const setupCustomBehavior = ({ updateContent, openImgModal, openSourceCodeModal, - setImage, editorType, imageUrls, + images, + setImage, lmsEndpointUrl, }) => (editor) => { // image upload button @@ -109,7 +146,9 @@ export const setupCustomBehavior = ({ editor.ui.registry.addButton(tinyMCE.buttons.editImageSettings, { icon: 'image', tooltip: 'Edit Image Settings', - onAction: module.openModalWithSelectedImage({ editor, setImage, openImgModal }), + onAction: module.openModalWithSelectedImage({ + editor, images, setImage, openImgModal, + }), }); // overriding the code plugin's icon with 'HTML' text editor.ui.registry.addButton(tinyMCE.buttons.code, { @@ -166,6 +205,8 @@ export const setupCustomBehavior = ({ editor.formatter.remove('label'); } }); + // after resizing an image in the editor, synchronize React state and ref + editor.on('ObjectResized', getImageResizeHandler({ editor, imagesRef: images, setImage })); }; // imagetools_cors_hosts needs a protocol-sanatized url @@ -174,7 +215,7 @@ export const removeProtocolFromUrl = (url) => url.replace(/^https?:\/\//, ''); export const editorConfig = ({ editorType, setEditorRef, - textValue, + editorContentHtml, images, lmsEndpointUrl, studioEndpointUrl, @@ -185,6 +226,7 @@ export const editorConfig = ({ openSourceCodeModal, setSelection, updateContent, + content, minHeight, }) => { const { @@ -195,6 +237,7 @@ export const editorConfig = ({ quickbarsInsertToolbar, quickbarsSelectionToolbar, } = pluginConfig({ isLibrary, placeholder, editorType }); + return { onInit: (evt, editor) => { setEditorRef(editor); @@ -202,7 +245,7 @@ export const editorConfig = ({ initializeEditor(); } }, - initialValue: textValue || '', + initialValue: editorContentHtml || '', init: { ...config, skin: false, @@ -221,6 +264,8 @@ export const editorConfig = ({ openSourceCodeModal, lmsEndpointUrl, setImage: setSelection, + content, + images, imageUrls: module.fetchImageUrls(images), }), quickbars_insert_toolbar: quickbarsInsertToolbar, @@ -269,14 +314,68 @@ export const sourceCodeModalToggle = (editorRef) => { }; }; -export const openModalWithSelectedImage = ({ editor, setImage, openImgModal }) => () => { - const imgHTML = editor.selection.getNode(); +/** + * const imageMatchRegex + * + * Image urls and ids used in the TinyMceEditor vary wildly, with different base urls, + * different lengths and constituent parts, and replacement of some "/" with "@". + * Common are the keys "asset-v1", "type", and "block", each holding a value after some separator. + * This regex captures only the values for these keys using capture groups, which can be used for matching. + */ +export const imageMatchRegex = /asset-v1.(.*).type.(.*).block.(.*)/; + +/** + * function matchImageStringsByIdentifiers + * + * matches two strings by comparing their regex capture groups using the `imageMatchRegex` + */ +export const matchImageStringsByIdentifiers = (a, b) => { + if (!a || !b || !(typeof a === 'string') || !(typeof b === 'string')) { return null; } + const matchA = JSON.stringify(a.match(imageMatchRegex)?.slice?.(1)); + const matchB = JSON.stringify(b.match(imageMatchRegex)?.slice?.(1)); + return matchA && matchA === matchB; +}; + +export const stringToFragment = (htmlString) => document.createRange().createContextualFragment(htmlString); + +export const getImageFromHtmlString = (htmlString, imageSrc) => { + const images = stringToFragment(htmlString)?.querySelectorAll('img') || []; + + return Array.from(images).find((img) => matchImageStringsByIdentifiers(img.src || '', imageSrc)); +}; + +export const detectImageMatchingError = ({ matchingImages, tinyMceHTML }) => { + if (!matchingImages.length) { return true; } + if (matchingImages.length > 1) { return true; } + + if (!matchImageStringsByIdentifiers(matchingImages[0].id, tinyMceHTML.src)) { return true; } + if (!matchingImages[0].width || !matchingImages[0].height) { return true; } + if (matchingImages[0].width !== tinyMceHTML.width) { return true; } + if (matchingImages[0].height !== tinyMceHTML.height) { return true; } + + return false; +}; + +export const openModalWithSelectedImage = ({ + editor, images, setImage, openImgModal, +}) => () => { + const tinyMceHTML = editor.selection.getNode(); + const { src: mceSrc } = tinyMceHTML; + + const matchingImages = images.current.filter(image => matchImageStringsByIdentifiers(image.id, mceSrc)); + + const imageMatchingErrorDetected = detectImageMatchingError({ tinyMceHTML, matchingImages }); + + const width = imageMatchingErrorDetected ? null : matchingImages[0]?.width; + const height = imageMatchingErrorDetected ? null : matchingImages[0]?.height; + setImage({ - externalUrl: imgHTML.src, - altText: imgHTML.alt, - width: imgHTML.width, - height: imgHTML.height, + externalUrl: tinyMceHTML.src, + altText: tinyMceHTML.alt, + width, + height, }); + openImgModal(); }; @@ -331,7 +430,7 @@ export const setAssetToStaticUrl = ({ editorValue, assets, lmsEndpointUrl }) => export const fetchImageUrls = (images) => { const imageUrls = []; - images.forEach(image => { + images.current.forEach(image => { imageUrls.push({ staticFullUrl: image.staticFullUrl, displayName: image.displayName }); }); return imageUrls; @@ -345,3 +444,34 @@ export const selectedImage = (val) => { setSelection, }; }; + +/** + * function updateImageDimensions + * + * Updates one images' dimensions in an array by identifying one image via a url string match + * that includes asset-v1, type, and block. Returns a new array. + * + * @param {Object[]} images - [{ id, ...other }] + * @param {string} url + * @param {number} width + * @param {number} height + * + * @returns {Object} { result, foundMatch } + */ +export const updateImageDimensions = ({ + images, url, width, height, +}) => { + let foundMatch = false; + + const result = images.map((image) => { + const imageIdentifier = image.id || image.url || image.src || image.externalUrl; + const isMatch = matchImageStringsByIdentifiers(imageIdentifier, url); + if (isMatch) { + foundMatch = true; + return { ...image, width, height }; + } + return image; + }); + + return { result, foundMatch }; +}; diff --git a/src/editors/sharedComponents/TinyMceWidget/hooks.test.js b/src/editors/sharedComponents/TinyMceWidget/hooks.test.js index 5078cc680..2eb52c412 100644 --- a/src/editors/sharedComponents/TinyMceWidget/hooks.test.js +++ b/src/editors/sharedComponents/TinyMceWidget/hooks.test.js @@ -19,16 +19,57 @@ const moduleKeys = keyStore(module); let hook; let output; +const editorImageWidth = 2022; +const editorImageHeight = 1619; + const mockNode = { - src: 'sOmEuRl.cOm', + src: 'http://localhost:18000/asset-v1:TestX+Test01+Test0101+type@asset+block/DALL_E_2023-03-10.png', alt: 'aLt tExt', - width: 2022, - height: 1619, + width: editorImageWidth, + height: editorImageHeight, }; +const initialContentHeight = 150; +const initialContentWidth = 100; +const mockNodeWithInitialContentDimensions = { ...mockNode, width: initialContentWidth, height: initialContentHeight }; +const mockEditorWithSelection = { selection: { getNode: () => mockNode } }; + +const mockImage = { + displayName: 'DALL·E 2023-03-10.png', + contentType: 'image/png', + dateAdded: 1682009100000, + url: '/asset-v1:TestX+Test01+Test0101+type@asset+block@DALL_E_2023-03-10.png', + externalUrl: 'http://localhost:18000/asset-v1:TestX+Test01+Test0101+type@asset+block@DALL_E_2023-03-10.png', + portableUrl: '/static/DALL_E_2023-03-10.png', + thumbnail: '/asset-v1:TestX+Test01+Test0101+type@thumbnail+block@DALL_E_2023-03-10.jpg', + locked: false, + staticFullUrl: '/assets/courseware/v1/af2bf9ac70804e54c534107160a8e51e/asset-v1:TestX+Test01+Test0101+type@asset+block@DALL_E_2023-03-10.png', + id: 'asset-v1:TestX+Test01+Test0101+type@asset+block@DALL_E_2023-03-10.png', + width: initialContentWidth, + height: initialContentHeight, +}; + +const mockAssets = { + [mockImage.id]: mockImage, +}; + +const mockEditorContentHtml = ` +

+ + +

+`; + +const mockImagesRef = { current: [mockImage] }; + describe('TinyMceEditor hooks', () => { beforeEach(() => { jest.clearAllMocks(); + mockImagesRef.current = [mockImage]; }); describe('state hooks', () => { state.testGetter(state.keys.isImageModalOpen); @@ -40,6 +81,34 @@ describe('TinyMceEditor hooks', () => { beforeEach(() => { state.mock(); }); afterEach(() => { state.restore(); }); + describe('detectImageMatchingError', () => { + it('should detect an error if the matchingImages array is empty', () => { + const matchingImages = []; + const tinyMceHTML = mockNode; + expect(module.detectImageMatchingError({ matchingImages, tinyMceHTML })).toBe(true); + }); + it('should detect an error if the matchingImages array has more than one element', () => { + const matchingImages = [mockImage, mockImage]; + const tinyMceHTML = mockNode; + expect(module.detectImageMatchingError({ matchingImages, tinyMceHTML })).toBe(true); + }); + it('should detect an error if the image id does not match the tinyMceHTML src', () => { + const matchingImages = [{ ...mockImage, id: 'some-other-id' }]; + const tinyMceHTML = mockNode; + expect(module.detectImageMatchingError({ matchingImages, tinyMceHTML })).toBe(true); + }); + it('should detect an error if the image id matches the tinyMceHTML src, but width and height do not match', () => { + const matchingImages = [{ ...mockImage, width: 100, height: 100 }]; + const tinyMceHTML = mockNode; + expect(module.detectImageMatchingError({ matchingImages, tinyMceHTML })).toBe(true); + }); + it('should not detect any errors if id matches src, and width and height match', () => { + const matchingImages = [{ ...mockImage, width: mockNode.width, height: mockNode.height }]; + const tinyMceHTML = mockNode; + expect(module.detectImageMatchingError({ matchingImages, tinyMceHTML })).toBe(false); + }); + }); + describe('setupCustomBehavior', () => { test('It calls addButton and addToggleButton in the editor, but openModal is not called', () => { const addButton = jest.fn(); @@ -62,6 +131,7 @@ describe('TinyMceEditor hooks', () => { const setupCodeFormatting = expect.any(Function); jest.spyOn(module, moduleKeys.openModalWithSelectedImage) .mockImplementationOnce(mockOpenModalWithImage); + output = module.setupCustomBehavior({ editorType, updateContent, @@ -152,16 +222,17 @@ describe('TinyMceEditor hooks', () => { describe('editorConfig', () => { const props = { - textValue: null, + editorContentHtml: null, editorType: 'text', lmsEndpointUrl: 'sOmEuRl.cOm', studioEndpointUrl: 'sOmEoThEruRl.cOm', - images: [{ staTICUrl: '/assets/sOmEuiMAge' }], + images: mockImagesRef, isLibrary: false, }; const evt = 'fakeEvent'; const editor = 'myEditor'; const setupCustomBehavior = args => ({ setupCustomBehavior: args }); + beforeEach(() => { props.setEditorRef = jest.fn(); props.openImgModal = jest.fn(); @@ -172,6 +243,7 @@ describe('TinyMceEditor hooks', () => { .mockImplementationOnce(setupCustomBehavior); output = module.editorConfig(props); }); + describe('text editor plugins and toolbar', () => { test('It configures plugins and toolbars correctly', () => { const pluginProps = { @@ -259,9 +331,9 @@ describe('TinyMceEditor hooks', () => { expect(output.initialValue).toBe(''); }); test('It sets the blockvalue to be the blockvalue if nonempty', () => { - const textValue = 'SomE hTML content'; - output = module.editorConfig({ ...props, textValue }); - expect(output.initialValue).toBe(textValue); + const editorContentHtml = 'SomE hTML content'; + output = module.editorConfig({ ...props, editorContentHtml }); + expect(output.initialValue).toBe(editorContentHtml); }); it('calls setupCustomBehavior on setup', () => { @@ -273,6 +345,7 @@ describe('TinyMceEditor hooks', () => { openSourceCodeModal: props.openSourceCodeModal, setImage: props.setSelection, imageUrls: module.fetchImageUrls(props.images), + images: mockImagesRef, lmsEndpointUrl: props.lmsEndpointUrl, }), ); @@ -330,20 +403,57 @@ describe('TinyMceEditor hooks', () => { }); describe('openModalWithSelectedImage', () => { - test('image is set to be value stored in editor, modal is opened', () => { - const setImage = jest.fn(); - const openImgModal = jest.fn(); - const editor = { selection: { getNode: () => mockNode } }; - module.openModalWithSelectedImage({ editor, openImgModal, setImage })(); + const setImage = jest.fn(); + const openImgModal = jest.fn(); + let editor; + + beforeEach(() => { + editor = { selection: { getNode: () => mockNodeWithInitialContentDimensions } }; + module.openModalWithSelectedImage({ + editor, images: mockImagesRef, openImgModal, setImage, + })(); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + test('updates React state for selected image to be value stored in editor, adding dimensions from images ref', () => { expect(setImage).toHaveBeenCalledWith({ externalUrl: mockNode.src, altText: mockNode.alt, - width: mockNode.width, - height: mockNode.height, + width: mockImage.width, + height: mockImage.height, }); + }); + + test('opens image setting modal', () => { expect(openImgModal).toHaveBeenCalled(); }); + + describe('when images cannot be successfully matched', () => { + beforeEach(() => { + editor = { selection: { getNode: () => mockNode } }; + module.openModalWithSelectedImage({ + editor, images: mockImagesRef, openImgModal, setImage, + })(); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + test('updates React state for selected image to be value stored in editor, setting dimensions to null', () => { + expect(setImage).toHaveBeenCalledWith({ + externalUrl: mockNode.src, + altText: mockNode.alt, + width: null, + height: null, + }); + }); + }); }); + describe('selectedImage hooks', () => { const val = { a: 'VaLUe' }; beforeEach(() => { @@ -361,5 +471,118 @@ describe('TinyMceEditor hooks', () => { expect(hook.setSelection).toHaveBeenCalledWith(null); }); }); + describe('imageMatchRegex', () => { + it('should match a valid image url using "@" separators', () => { + expect( + 'http://localhost:18000/asset-v1:TestX+Test01+Test0101+type@asset+block@image-name.png', + ).toMatch(module.imageMatchRegex); + }); + it('should match a url including the keywords "asset-v1", "type", "block" in that order', () => { + expect( + 'https://some.completely/made.up///url-with.?!keywords/asset-v1:Some-asset-key?type=some.type.key!block@image-name.png', + ).toMatch(module.imageMatchRegex); + }); + it('should not match a url excluding the keyword "asset-v1"', () => { + expect( + 'https://some.completely/made.up///url-with.?!keywords/Some-asset-key?type=some.type.key!block@image-name.png', + ).not.toMatch(module.imageMatchRegex); + }); + it('should match an identifier including the keywords "asset-v1", "type", "block" using "/" separators', () => { + expect( + 'asset-v1:TestX+Test01+Test0101+type/asset+block/image-name.png', + ).toMatch(module.imageMatchRegex); + }); + it('should capture values for the keys "asset-v1", "type", "block"', () => { + const match = 'asset-v1:TestX+Test01+Test0101+type/asset+block/image-name.png'.match(module.imageMatchRegex); + expect(match[1]).toBe('TestX+Test01+Test0101'); + expect(match[2]).toBe('asset'); + expect(match[3]).toBe('image-name.png'); + }); + }); + + describe('matchImageStringsByIdentifiers', () => { + it('should be true for an image url and identifier that have the same values for asset-v1, type, and block', () => { + const url = 'http://localhost:18000/asset-v1:TestX+Test01+Test0101+type@asset+block@image-name.png'; + const id = 'asset-v1:TestX+Test01+Test0101+type/asset+block/image-name.png'; + expect(module.matchImageStringsByIdentifiers(url, id)).toBe(true); + }); + it('should be false for an image url and identifier that have different values for block', () => { + const url = 'http://localhost:18000/asset-v1:TestX+Test01+Test0101+type@asset+block@image-name.png'; + const id = 'asset-v1:TestX+Test01+Test0101+type/asset+block/different-image-name.png'; + expect(module.matchImageStringsByIdentifiers(url, id)).toBe(false); + }); + it('should return null if it doesnt receive two strings as input', () => { + expect(module.matchImageStringsByIdentifiers(['a'], { b: 'c ' })).toBe(null); + }); + it('should return undefined if the strings dont match the regex at all', () => { + expect(module.matchImageStringsByIdentifiers('wrong-url', 'blub')).toBe(undefined); + }); + }); + + describe('addImagesAndDimensionsToRef', () => { + it('should add images to ref', () => { + const imagesRef = { current: null }; + const assets = { ...mockAssets, height: undefined, width: undefined }; + module.addImagesAndDimensionsToRef( + { + imagesRef, + assets, + editorContentHtml: mockEditorContentHtml, + }, + ); + expect(imagesRef.current).toEqual([mockImage]); + expect(imagesRef.current[0].width).toBe(initialContentWidth); + expect(imagesRef.current[0].height).toBe(initialContentHeight); + }); + }); + + describe('getImageResizeHandler', () => { + const setImage = jest.fn(); + + it('sets image ref and state to new width', () => { + expect(mockImagesRef.current[0].width).toBe(initialContentWidth); + module.getImageResizeHandler({ editor: mockEditorWithSelection, imagesRef: mockImagesRef, setImage })(); + + expect(setImage).toHaveBeenCalledTimes(1); + expect(setImage).toHaveBeenCalledWith(expect.objectContaining({ width: editorImageWidth })); + expect(mockImagesRef.current[0].width).not.toBe(initialContentWidth); + expect(mockImagesRef.current[0].width).toBe(editorImageWidth); + }); + }); + + describe('updateImageDimensions', () => { + const unchangedImg = { + id: 'asset-v1:TestX+Test01+Test0101+type@asset+block@unchanged-image.png', + width: 3, + height: 5, + }; + const images = [ + mockImage, + unchangedImg, + ]; + + it('updates dimensions of correct image in images array', () => { + const { result, foundMatch } = module.updateImageDimensions({ + images, url: mockNode.src, width: 123, height: 321, + }); + const imageToHaveBeenUpdated = result.find(img => img.id === mockImage.id); + const imageToHaveBeenUnchanged = result.find(img => img.id === unchangedImg.id); + + expect(imageToHaveBeenUpdated.width).toBe(123); + expect(imageToHaveBeenUpdated.height).toBe(321); + expect(imageToHaveBeenUnchanged.width).toBe(3); + expect(imageToHaveBeenUnchanged.height).toBe(5); + + expect(foundMatch).toBe(true); + }); + + it('does not update images if id is not found', () => { + const { result, foundMatch } = module.updateImageDimensions({ + images, url: 'not_found', width: 123, height: 321, + }); + expect(result.find(img => img.width === 123 || img.height === 321)).toBeFalsy(); + expect(foundMatch).toBe(false); + }); + }); }); }); diff --git a/src/editors/sharedComponents/TinyMceWidget/index.jsx b/src/editors/sharedComponents/TinyMceWidget/index.jsx index 7a2ccd11b..35db0cca7 100644 --- a/src/editors/sharedComponents/TinyMceWidget/index.jsx +++ b/src/editors/sharedComponents/TinyMceWidget/index.jsx @@ -31,6 +31,7 @@ export const TinyMceWidget = ({ editorRef, disabled, id, + editorContentHtml, // editorContent in html form // redux assets, isLibrary, @@ -40,8 +41,10 @@ export const TinyMceWidget = ({ }) => { const { isImgOpen, openImgModal, closeImgModal } = hooks.imgModalToggle(); const { isSourceCodeOpen, openSourceCodeModal, closeSourceCodeModal } = hooks.sourceCodeModalToggle(editorRef); - const images = hooks.filterAssets({ assets }); + const { imagesRef } = hooks.useImages({ assets, editorContentHtml }); + const imageSelection = hooks.selectedImage(null); + return ( <> {isLibrary ? null : ( @@ -49,7 +52,7 @@ export const TinyMceWidget = ({ isOpen={isImgOpen} close={closeImgModal} editorRef={editorRef} - images={images} + images={imagesRef} editorType={editorType} lmsEndpointUrl={lmsEndpointUrl} {...imageSelection} @@ -74,9 +77,9 @@ export const TinyMceWidget = ({ isLibrary, lmsEndpointUrl, studioEndpointUrl, - images, - setSelection: imageSelection.setSelection, - clearSelection: imageSelection.clearSelection, + images: imagesRef, + editorContentHtml, + ...imageSelection, ...props, }) } @@ -93,6 +96,7 @@ TinyMceWidget.defaultProps = { assets: null, id: null, disabled: false, + editorContentHtml: undefined, }; TinyMceWidget.propTypes = { editorType: PropTypes.string, @@ -103,6 +107,7 @@ TinyMceWidget.propTypes = { studioEndpointUrl: PropTypes.string, id: PropTypes.string, disabled: PropTypes.bool, + editorContentHtml: PropTypes.string, }; export const mapStateToProps = (state) => ({ diff --git a/src/editors/sharedComponents/TinyMceWidget/index.test.jsx b/src/editors/sharedComponents/TinyMceWidget/index.test.jsx index 5634b2864..d328c401e 100644 --- a/src/editors/sharedComponents/TinyMceWidget/index.test.jsx +++ b/src/editors/sharedComponents/TinyMceWidget/index.test.jsx @@ -6,6 +6,8 @@ import ImageUploadModal from '../ImageUploadModal'; import { imgModalToggle, sourceCodeModalToggle } from './hooks'; import { TinyMceWidget, mapStateToProps } from '.'; +const staticUrl = '/assets/sOmEaSsET'; + // Per https://github.com/tinymce/tinymce-react/issues/91 React unit testing in JSDOM is not supported by tinymce. // Consequently, mock the Editor out. jest.mock('@tinymce/tinymce-react', () => { @@ -48,7 +50,8 @@ jest.mock('./hooks', () => ({ setSelection: jest.fn().mockName('hooks.selectedImage.setSelection'), clearSelection: jest.fn().mockName('hooks.selectedImage.clearSelection'), })), - filterAssets: jest.fn(() => [{ staTICUrl: '/assets/sOmEaSsET' }]), + filterAssets: jest.fn(() => [{ staTICUrl: staticUrl }]), + useImages: jest.fn(() => ({ imagesRef: { current: [{ externalUrl: staticUrl }] } })), })); describe('TinyMceWidget', () => { @@ -56,7 +59,7 @@ describe('TinyMceWidget', () => { editorType: 'text', editorRef: { current: { value: 'something' } }, isLibrary: false, - assets: { sOmEaSsET: { staTICUrl: '/assets/sOmEaSsET' } }, + assets: { sOmEaSsET: { staTICUrl: staticUrl } }, lmsEndpointUrl: 'sOmEvaLue.cOm', studioEndpointUrl: 'sOmEoThERvaLue.cOm', disabled: false,