From a5f342bc1bb25e151e6bfc067c2d8e5b9faafacd Mon Sep 17 00:00:00 2001 From: "kshitij.sobti" Date: Sat, 11 Apr 2026 09:34:45 +0530 Subject: [PATCH 1/3] chore: rename ImageSettingModal components from jsx tsx before refactor This commit is mainly to get git to reconise the rename so file history is maintained. --- .../{AltTextControls.jsx => AltTextControls.tsx} | 0 .../{DimensionControls.jsx => DimensionControls.tsx} | 0 .../ImageUploadModal/ImageSettingsModal/{index.jsx => index.tsx} | 0 3 files changed, 0 insertions(+), 0 deletions(-) rename src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/{AltTextControls.jsx => AltTextControls.tsx} (100%) rename src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/{DimensionControls.jsx => DimensionControls.tsx} (100%) rename src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/{index.jsx => index.tsx} (100%) diff --git a/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/AltTextControls.jsx b/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/AltTextControls.tsx similarity index 100% rename from src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/AltTextControls.jsx rename to src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/AltTextControls.tsx diff --git a/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/DimensionControls.jsx b/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/DimensionControls.tsx similarity index 100% rename from src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/DimensionControls.jsx rename to src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/DimensionControls.tsx diff --git a/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/index.jsx b/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/index.tsx similarity index 100% rename from src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/index.jsx rename to src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/index.tsx From dd3cdaf7a887e8c3b22dd4b216a97c26c5bd2391 Mon Sep 17 00:00:00 2001 From: "kshitij.sobti" Date: Sat, 11 Apr 2026 09:51:43 +0530 Subject: [PATCH 2/3] refactor: Use formik and yup, and simplify hooks This change converts the Image Editor Modal to typescript and simplifies the hooks switching to Formik + Yup for form validation. It updates the tests to check the behaviour of the component as a whole rather than testing small components. --- .../settingsComponents/SwitchEditorCard.jsx | 26 +- .../BaseModal/{index.jsx => index.tsx} | 52 +-- .../AltTextControls.test.tsx | 66 --- .../ImageSettingsModal/AltTextControls.tsx | 95 ++-- .../DimensionControls.test.tsx | 115 ----- .../ImageSettingsModal/DimensionControls.tsx | 132 +++--- .../ImageSettingsModal/hooks.js | 347 --------------- .../ImageSettingsModal/hooks.test.js | 414 ------------------ .../ImageSettingsModal/index.test.tsx | 145 +++++- .../ImageSettingsModal/index.tsx | 174 +++++--- .../ImageSettingsModal/types.ts | 18 + 11 files changed, 402 insertions(+), 1182 deletions(-) rename src/editors/sharedComponents/BaseModal/{index.jsx => index.tsx} (64%) delete mode 100644 src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/AltTextControls.test.tsx delete mode 100644 src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/DimensionControls.test.tsx delete mode 100644 src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/hooks.js delete mode 100644 src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/hooks.test.js create mode 100644 src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/types.ts diff --git a/src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/settingsComponents/SwitchEditorCard.jsx b/src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/settingsComponents/SwitchEditorCard.jsx index df3a9bcac3..6e8b085366 100644 --- a/src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/settingsComponents/SwitchEditorCard.jsx +++ b/src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/settingsComponents/SwitchEditorCard.jsx @@ -1,6 +1,6 @@ import React from 'react'; import { useDispatch } from 'react-redux'; -import { FormattedMessage } from '@edx/frontend-platform/i18n'; +import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n'; import { Card } from '@openedx/paragon'; import PropTypes from 'prop-types'; import { thunkActions } from '@src/editors/data/redux'; @@ -15,6 +15,7 @@ const SwitchEditorCard = ({ editorType, problemType, }) => { + const intl = useIntl(); const [isConfirmOpen, setConfirmOpen] = React.useState(false); const dispatch = useDispatch(); const { editorRef } = useProblemEditorContext(); @@ -24,23 +25,20 @@ const SwitchEditorCard = ({ { - setConfirmOpen(false); - }} - title={} - confirmAction={ + close={() => { setConfirmOpen(false); }} + title={intl.formatMessage(messages[`ConfirmSwitchMessageTitle-${editorType}`])} + confirmAction={( - } + )} size="md" > @@ -49,9 +47,7 @@ const SwitchEditorCard = ({ className="my-3 ml-2 py-0" variant="link" size="sm" - onClick={() => { - setConfirmOpen(true); - }} + onClick={() => { setConfirmOpen(true); }} > diff --git a/src/editors/sharedComponents/BaseModal/index.jsx b/src/editors/sharedComponents/BaseModal/index.tsx similarity index 64% rename from src/editors/sharedComponents/BaseModal/index.jsx rename to src/editors/sharedComponents/BaseModal/index.tsx index 455e604a9b..b1bbd4fb92 100644 --- a/src/editors/sharedComponents/BaseModal/index.jsx +++ b/src/editors/sharedComponents/BaseModal/index.tsx @@ -1,5 +1,4 @@ import React from 'react'; -import PropTypes from 'prop-types'; import { ActionRow, @@ -10,6 +9,21 @@ import { FormattedMessage } from '@edx/frontend-platform/i18n'; import messages from './messages'; +interface BaseModalProps { + isOpen: boolean; + close: () => void; + title: string; + children: React.ReactNode; + confirmAction: React.ReactNode; + footerAction?: React.ReactNode; + headerComponent?: React.ReactNode; + size?: 'sm' | 'md' | 'lg' | 'xl' | 'fullscreen'; + isFullscreenScroll?: boolean; + bodyStyle?: React.CSSProperties; + className?: string; + hideCancelButton?: boolean; +} + const BaseModal = ({ isOpen, close, @@ -17,13 +31,13 @@ const BaseModal = ({ children, headerComponent, confirmAction, - footerAction, - size, - isFullscreenScroll, + footerAction = null, + size = 'lg', + isFullscreenScroll = true, bodyStyle, className, - hideCancelButton, -}) => ( + hideCancelButton = false, +}: BaseModalProps) => ( @@ -65,29 +80,4 @@ const BaseModal = ({ ); -BaseModal.defaultProps = { - footerAction: null, - headerComponent: null, - size: 'lg', - isFullscreenScroll: true, - bodyStyle: null, - className: undefined, - hideCancelButton: false, -}; - -BaseModal.propTypes = { - isOpen: PropTypes.bool.isRequired, - close: PropTypes.func.isRequired, - title: PropTypes.node.isRequired, - children: PropTypes.node.isRequired, - confirmAction: PropTypes.node.isRequired, - footerAction: PropTypes.node, - headerComponent: PropTypes.node, - size: PropTypes.string, - isFullscreenScroll: PropTypes.bool, - bodyStyle: PropTypes.shape({}), - className: PropTypes.string, - hideCancelButton: PropTypes.bool, -}; - export default BaseModal; diff --git a/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/AltTextControls.test.tsx b/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/AltTextControls.test.tsx deleted file mode 100644 index 72763a267f..0000000000 --- a/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/AltTextControls.test.tsx +++ /dev/null @@ -1,66 +0,0 @@ -import React from 'react'; -import { - render, - screen, - initializeMocks, - fireEvent, -} from '@src/testUtils'; -import AltTextControls from './AltTextControls'; - -describe('AltTextControls', () => { - const props = { - isDecorative: true, - value: 'props.value', - setValue: jest.fn().mockName('props.setValue'), - setIsDecorative: jest.fn().mockName('props.setIsDecorative'), - validation: { show: false }, - error: { show: false }, - }; - - beforeEach(() => { - initializeMocks(); - }); - - test('renders component on screen', () => { - render(); - expect(screen.getByRole('checkbox', { name: 'This image is decorative (no alt text required).' })) - .toBeInTheDocument(); - expect(screen.getByRole('textbox', { name: 'Accessibility' })).toBeInTheDocument(); - }); - - test('renders validation feedback when validation.show is true', () => { - const feedbackMessage = 'Enter alt text'; - render(); - expect(screen.getByText(feedbackMessage)).toBeInTheDocument(); - }); - - test('does not render validation feedback when validation.show is false', () => { - const feedbackMessage = 'Enter alt text'; - render(); - expect(screen.queryByText(feedbackMessage)).not.toBeInTheDocument(); - }); - - test('disables textbox when isDecorative is true', () => { - render(); - expect(screen.getByRole('textbox', { name: 'Accessibility' })).toBeDisabled(); - }); - - test('enables textbox when isDecorative is false', () => { - render(); - expect(screen.getByRole('textbox', { name: 'Accessibility' })).not.toBeDisabled(); - }); - - test('calls setValue on textbox change', () => { - render(); - const textbox = screen.getByRole('textbox', { name: 'Accessibility' }); - fireEvent.change(textbox, { target: { value: 'new alt text' } }); - expect(props.setValue).toHaveBeenCalled(); - }); - - test('calls setIsDecorative on checkbox change', () => { - render(); - const checkbox = screen.getByRole('checkbox', { name: 'This image is decorative (no alt text required).' }); - checkbox.click(); - expect(props.setIsDecorative).toHaveBeenCalled(); - }); -}); diff --git a/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/AltTextControls.tsx b/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/AltTextControls.tsx index 00518a8c9e..c9d999471d 100644 --- a/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/AltTextControls.tsx +++ b/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/AltTextControls.tsx @@ -1,73 +1,52 @@ +import { useFormikContext } from 'formik'; import React from 'react'; -import PropTypes from 'prop-types'; import { Form } from '@openedx/paragon'; -import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n'; +import { useIntl } from '@edx/frontend-platform/i18n'; +import { ImageConfig } from './types'; -import * as hooks from './hooks'; import messages from './messages'; -/** - * Wrapper for alt-text input and isDecorative checkbox control - * @param {obj} errorProps - props for error handling - * {bool} isValid - are alt-text fields valid for saving? - * @param {bool} isDecorative - is the image decorative? - * @param {func} setIsDecorative - handle isDecorative change event - * @param {func} setValue - update alt-text value - * @param {string} value - current alt-text value - */ -const AltTextControls = ({ - isDecorative, - setIsDecorative, - setValue, - validation, - value, -}) => { +const AltTextControls = () => { const intl = useIntl(); + const formik = useFormikContext(); return ( - - - - - - {validation.show - && ( - - - - )} - + <> + - + {intl.formatMessage(messages.accessibilityLabel)} - - + + {formik.errors.altText && ( + + {formik.errors.altText} + + )} + + + + + {intl.formatMessage(messages.decorativeAltTextCheckboxLabel)} + + + + ); }; -AltTextControls.propTypes = { - error: PropTypes.shape({ - show: PropTypes.bool, - }).isRequired, - isDecorative: PropTypes.bool.isRequired, - setValue: PropTypes.func.isRequired, - setIsDecorative: PropTypes.func.isRequired, - validation: PropTypes.shape({ - show: PropTypes.bool, - }).isRequired, - value: PropTypes.string.isRequired, -}; export const AltTextControlsInternal = AltTextControls; // For testing only export default AltTextControls; diff --git a/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/DimensionControls.test.tsx b/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/DimensionControls.test.tsx deleted file mode 100644 index ec726ae9fe..0000000000 --- a/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/DimensionControls.test.tsx +++ /dev/null @@ -1,115 +0,0 @@ -import React, { useEffect } from 'react'; -import { - fireEvent, - render, - screen, - waitFor, - initializeMocks, -} from '@src/testUtils'; -import DimensionControls from './DimensionControls'; -import * as hooks from './hooks'; - -const WrappedDimensionControls = () => { - const dimensions = hooks.dimensionHooks('altText'); - - useEffect(() => { - dimensions.onImgLoad({})({ target: { naturalWidth: 1517, naturalHeight: 803 } }); - }, []); - - return ; -}; - -const UnlockedDimensionControls = () => { - const dimensions = hooks.dimensionHooks('altText'); - - useEffect(() => { - dimensions.onImgLoad({})({ target: { naturalWidth: 1517, naturalHeight: 803 } }); - dimensions.unlock(); - }, []); - - return ; -}; - -describe('DimensionControls', () => { - describe('render', () => { - const props = { - lockAspectRatio: { width: 4, height: 5 }, - locked: { 'props.locked': 'lockedValue' }, - isLocked: true, - value: { width: '20', height: '40' }, - setWidth: jest.fn(), - setHeight: jest.fn(), - lock: jest.fn(), - unlock: jest.fn(), - updateDimensions: jest.fn(), - }; - beforeEach(() => { - jest.spyOn(hooks, 'onInputChange').mockImplementation((handler) => ({ 'hooks.onInputChange': handler })); - initializeMocks(); - }); - afterEach(() => { - jest.spyOn(hooks, 'onInputChange').mockRestore(); - }); - test('renders component', () => { - render(); - expect(screen.getByText('Image Dimensions')).toBeInTheDocument(); - }); - test('renders nothing with null value', () => { - const reduxProviderWrapper = '
'; - const { container } = render(); - expect(screen.queryByText('Image Dimensions')).not.toBeInTheDocument(); - expect(container.innerHTML).toBe(reduxProviderWrapper); - expect(container.firstChild?.textContent).toBe(''); - }); - - test('renders locked and unlocked icon button according to isLocked prop', () => { - const { rerender } = render(); - expect(screen.getByRole('button', { name: 'lock dimensions' })).toBeInTheDocument(); - rerender(); - expect(screen.getByRole('button', { name: 'unlock dimensions' })).toBeInTheDocument(); - }); - }); - - describe('component tests for dimensions', () => { - beforeEach(() => { - initializeMocks(); - }); - - it('renders with initial dimensions', () => { - const { container } = render(); - const widthInput = container.querySelector('input.form-control'); - expect(widthInput).not.toBeNull(); - expect((widthInput as HTMLInputElement).value).toBe('1517'); - }); - - it('resizes dimensions proportionally', async () => { - const { container } = render(); - const widthInput = container.querySelector('input.form-control') as HTMLInputElement; - expect((widthInput as HTMLInputElement).value).toBe('1517'); - fireEvent.change(widthInput, { target: { value: 758 } }); - await waitFor(() => { - expect((container.querySelectorAll('input.form-control')[0] as HTMLInputElement).value).toBe('758'); - }); - fireEvent.blur(widthInput); - await waitFor(() => { - expect((container.querySelectorAll('input.form-control')[0] as HTMLInputElement).value).toBe('758'); - expect((container.querySelectorAll('input.form-control')[1] as HTMLInputElement).value).toBe('401'); - }); - }); - - it('resizes only changed dimension when unlocked', async () => { - const { container } = render(); - const widthInput = container.querySelector('input.form-control') as HTMLInputElement; - expect(widthInput.value).toBe('1517'); - fireEvent.change(widthInput, { target: { value: 758 } }); - await waitFor(() => { - expect((container.querySelectorAll('input.form-control')[0] as HTMLInputElement).value).toBe('758'); - }); - fireEvent.blur(widthInput); - await waitFor(() => { - expect((container.querySelectorAll('input.form-control')[0] as HTMLInputElement).value).toBe('758'); - expect((container.querySelectorAll('input.form-control')[1] as HTMLInputElement).value).toBe('803'); - }); - }); - }); -}); diff --git a/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/DimensionControls.tsx b/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/DimensionControls.tsx index 29f4ea4fa2..71aeb5041f 100644 --- a/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/DimensionControls.tsx +++ b/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/DimensionControls.tsx @@ -1,5 +1,5 @@ +import { useFormikContext } from 'formik'; import React from 'react'; -import PropTypes from 'prop-types'; import { Form, Icon, @@ -9,82 +9,90 @@ import { Locked, Unlocked, } from '@openedx/paragon/icons'; -import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n'; +import { useIntl } from '@edx/frontend-platform/i18n'; +import { + ImageConfig, OrigImageDimensions, +} from './types'; -import * as hooks from './hooks'; import messages from './messages'; /** * Wrapper for image dimension inputs and the lock checkbox. - * @param {bool} isLocked - are dimensions locked - * @param {func} lock - lock dimensions - * @param {func} setHeight - updates dimensions based on new height - * @param {func} setWidth - updates dimensions based on new width - * @param {func} unlock - unlock dimensions - * @param {func} updateDimensions - update dimensions callback - * @param {obj} value - local dimension values { height, width } + * @param {ImageDimensions} originalDimensions - original dimensions of the image */ -const DimensionControls = ({ - isLocked, - lock, - setHeight, - setWidth, - unlock, - updateDimensions, - value, -}) => { +const DimensionControls = ({ originalDimensions }: { originalDimensions: OrigImageDimensions }) => { + const { width: originalWidth, height: originalHeight } = originalDimensions; const intl = useIntl(); - if (!value) { return null; } + const formik = useFormikContext(); + if (!(originalWidth && originalHeight)) { + return null; + } + const handleUpdateDimensions = async ({ target }) => { + const { name } = target; + let { value } = target; + // If dimensions are locked just set the value and return. + await formik.setFieldValue(name, value); + if (!formik.values.isLocked) { + return; + } + // For percentages, both values need to be ratio is locked + if (value.trim().endsWith('%')) { + await formik.setFieldValue('width', value); + await formik.setFieldValue('height', value); + return; + } + if (!value) { + return; + } + // For numerical values we calculate the other dimension based on the original ratio. + value = parseInt(value, 10); + if (name === 'height') { + await formik.setFieldValue('width', Math.round((value / originalHeight) * originalWidth)); + } else if (name === 'width') { + await formik.setFieldValue('height', Math.round((value / originalWidth) * originalHeight)); + } + }; return ( - - - + <> + + {intl.formatMessage(messages.imageDimensionsLabel)} -
- - +
+ + + + + + formik.setFieldValue('isLocked', !formik.values.isLocked)} />
- + ); }; -DimensionControls.defaultProps = { - value: { - height: '100', - width: '100', - }, -}; -DimensionControls.propTypes = { - value: PropTypes.shape({ - height: PropTypes.string, - width: PropTypes.string, - }), - setHeight: PropTypes.func.isRequired, - setWidth: PropTypes.func.isRequired, - isLocked: PropTypes.bool.isRequired, - lock: PropTypes.func.isRequired, - unlock: PropTypes.func.isRequired, - updateDimensions: PropTypes.func.isRequired, -}; export default DimensionControls; diff --git a/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/hooks.js b/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/hooks.js deleted file mode 100644 index 5d9e26657e..0000000000 --- a/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/hooks.js +++ /dev/null @@ -1,347 +0,0 @@ -import React from 'react'; - -import { StrictDict } from '../../../utils'; -// This 'module' self-import hack enables mocking during tests. -// See src/editors/decisions/0005-internal-editor-testability-decisions.md. The whole approach to how hooks are tested -// should be re-thought and cleaned up to avoid this pattern. -// eslint-disable-next-line import/no-self-import -import * as module from './hooks'; - -// Simple wrappers for useState to allow easy mocking for tests. -export const state = { - // eslint-disable-next-line react-hooks/rules-of-hooks - altText: (val) => React.useState(val), - // eslint-disable-next-line react-hooks/rules-of-hooks - dimensions: (val) => React.useState(val), - // eslint-disable-next-line react-hooks/rules-of-hooks - showAltTextDismissibleError: (val) => React.useState(val), - // eslint-disable-next-line react-hooks/rules-of-hooks - showAltTextSubmissionError: (val) => React.useState(val), - // eslint-disable-next-line react-hooks/rules-of-hooks - isDecorative: (val) => React.useState(val), - // eslint-disable-next-line react-hooks/rules-of-hooks - isLocked: (val) => React.useState(val), - // eslint-disable-next-line react-hooks/rules-of-hooks - local: (val) => React.useState(val), - // eslint-disable-next-line react-hooks/rules-of-hooks - lockAspectRatio: (val) => React.useState(val), -}; - -export const dimKeys = StrictDict({ - height: 'height', - width: 'width', -}); - -/** - * findGcd(numerator, denominator) - * 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) => { - 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); - -/** - * getValidDimensions({ dimensions, local, locked }) - * Find valid ending dimensions based on start state, request, and lock state - * @param {obj} dimensions - current stored dimensions - * @param {obj} local - local (active) dimensions in the inputs - * @param {obj} locked - locked dimensions - * @return {obj} - output dimensions after move ({ height, width }) - */ -export const getValidDimensions = ({ - dimensions, - local, - isLocked, - 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 = {}; - - // 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 }; - - 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. - * Locks the dimensions initially, on lock initialization. - * @param {obj} dimensions - current stored dimensions - * @return {obj} - dimension lock hooks - * {func} initializeLock - enable the lock mechanism - * {bool} isLocked - are dimensions locked? - * {obj} lockAspectRatio - image dimensions ({ height, width }) - * {func} lock - lock the dimensions - * {func} unlock - unlock the dimensions - */ -export const dimensionLockHooks = () => { - const [lockAspectRatio, setLockAspectRatio] = module.state.lockAspectRatio(null); - const [isLocked, setIsLocked] = module.state.isLocked(true); - - const initializeLock = ({ width, height }) => { - // 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), - lockAspectRatio, - unlock: () => setIsLocked(false), - }; -}; - -/** - * dimensionHooks() - * Returns an object of dimension-focused react hooks. - * @return {obj} - dimension hooks - * {func} onImgLoad - initializes image dimension fields - * @param {object} selection - selected image object with possible override dimensions. - * @return {callback} - image load event callback that loads dimensions. - * {object} locked - current locked state - * {func} lock - lock current dimensions - * {func} unlock - unlock dimensions - * {object} value - current dimension values - * {func} setHeight - set height - * @param {string} - new height string - * {func} setWidth - set width - * @param {string} - new width string - * {func} updateDimensions - set dimensions based on state - * {obj} errorProps - props for user feedback error - * {bool} isError - true if dimensions are blank - * {func} setError - sets isError to true - * {func} dismissError - sets isError to false - * {bool} isHeightValid - true if height field is ready to save - * {func} setHeightValid - sets isHeightValid to true - * {func} setHeightNotValid - sets isHeightValid to false - * {bool} isWidthValid - true if width field is ready to save - * {func} setWidthValid - sets isWidthValid to true - * {func} setWidthNotValid - sets isWidthValid to false - */ -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 === '') { - altTextHook.setIsDecorative(true); - } - altTextHook.setValue(altText); - } - 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, - lockAspectRatio, - unlock, - } = module.dimensionLockHooks({ dimensions }); - - return { - onImgLoad: (selection) => ({ target: img }) => { - const imageDims = { height: img.naturalHeight, width: img.naturalWidth }; - setAll(selection.height ? selection : imageDims); - initializeLock(selection.height ? selection : imageDims); - }, - isLocked, - lock, - unlock, - value: local, - setHeight, - setWidth, - updateDimensions: () => - setAll(module.getValidDimensions({ - dimensions, - local, - isLocked, - lockAspectRatio, - })), - }; -}; - -/** - * altTextHooks(savedText) - * Returns a set of react hooks focused around alt text - * @return {obj} - alt text hooks - * {string} value - alt text value - * {func} setValue - set alt test value - * @param {string} - new alt text - * {bool} isDecorative - is the image decorative? - * {func} setIsDecorative - set isDecorative field - * {obj} error - error at top of page - * {bool} show - is error being displayed? - * {func} set - set show to true - * {func} dismiss - set show to false - * {obj} validation - local alt text error - * {bool} show - is validation error being displayed? - * {func} set - set validation to true - * {func} dismiss - set validation to false - */ -export const altTextHooks = (savedText) => { - const [value, setValue] = module.state.altText(savedText || ''); - const [isDecorative, setIsDecorative] = module.state.isDecorative(false); - const [showAltTextDismissibleError, setShowAltTextDismissibleError] = module.state.showAltTextDismissibleError(false); - const [showAltTextSubmissionError, setShowAltTextSubmissionError] = module.state.showAltTextSubmissionError(false); - - const validateAltText = (newVal, newDecorative) => { - if (showAltTextSubmissionError) { - if (newVal || newDecorative) { - setShowAltTextSubmissionError(false); - } - } - }; - - return { - value, - setValue: (val) => { - setValue(val); - validateAltText(val, null); - }, - isDecorative, - setIsDecorative: (decorative) => { - setIsDecorative(decorative); - validateAltText(null, decorative); - }, - error: { - show: showAltTextDismissibleError, - set: () => setShowAltTextDismissibleError(true), - dismiss: () => setShowAltTextDismissibleError(false), - }, - validation: { - show: showAltTextSubmissionError, - set: () => setShowAltTextSubmissionError(true), - dismiss: () => setShowAltTextSubmissionError(false), - }, - }; -}; - -/** - * onInputChange(handleValue) - * Simple event handler forwarding the event target value to a given callback - * @param {func} handleValue - event value handler - * @return {func} - evt callback that will call handleValue with the event target value. - */ -export const onInputChange = (handleValue) => (e) => handleValue(e.target.value); - -/** - * onCheckboxChange(handleValue) - * Simple event handler forwarding the event target checked prop to a given callback - * @param {func} handleValue - event value handler - * @return {func} - evt callback that will call handleValue with the event target checked prop. - */ -export const onCheckboxChange = (handleValue) => (e) => handleValue(e.target.checked); - -/** - * checkFormValidation({ altText, isDecorative, onAltTextFail }) - * Handle saving the image context to the text editor - * @param {string} altText - image alt text - * @param {bool} isDecorative - is the image decorative? - * @param {func} onAltTextFail - called if alt text validation fails - */ -export const checkFormValidation = ({ - altText, - isDecorative, - onAltTextFail, -}) => { - if (!isDecorative && altText === '') { - onAltTextFail(); - return false; - } - return true; -}; - -/** - * onSave({ altText, dimensions, isDecorative, saveToEditor }) - * Handle saving the image context to the text editor - * @param {string} altText - image alt text - * @param {object} dimensions - image dimensions ({ width, height }) - * @param {bool} isDecorative - is the image decorative? - * @param {func} saveToEditor - save method for submitting image settings. - */ -export const onSaveClick = ({ - altText, - dimensions, - isDecorative, - saveToEditor, -}) => -() => { - if ( - module.checkFormValidation({ - altText: altText.value, - isDecorative, - onAltTextFail: () => { - altText.error.set(); - altText.validation.set(); - }, - }) - ) { - altText.error.dismiss(); - altText.validation.dismiss(); - // Replaces double quotes with " to prevent the alt text from being truncated - // or breaking the image tag structure. - const altTextValue = altText.value.replace(/"/g, '"'); - saveToEditor({ - altText: altTextValue, - dimensions, - isDecorative, - }); - } -}; diff --git a/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/hooks.test.js b/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/hooks.test.js deleted file mode 100644 index 300ceec33d..0000000000 --- a/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/hooks.test.js +++ /dev/null @@ -1,414 +0,0 @@ -import React from 'react'; -import { StrictDict } from '../../../utils'; -import { MockUseState } from '../../../testUtils'; -import * as hooks from './hooks'; - -jest.mock('react', () => ({ - ...jest.requireActual('react'), - useEffect: jest.fn(), - useState: (val) => ({ useState: val }), -})); - -const simpleDims = { width: 3, height: 4 }; -const reducedDims = { width: 7, height: 13 }; -const gcd = 7; -const multiDims = { - width: reducedDims.width * gcd, - height: reducedDims.height * gcd, -}; - -const state = new MockUseState(hooks); - -const hookKeys = StrictDict( - Object.keys(hooks).reduce( - (obj, key) => ({ ...obj, [key]: key }), - {}, - ), -); - -let hook; - -const testVal = 'MY test VALUE'; - -describe('state values', () => { - const testStateMethod = (key) => { - // eslint-disable-next-line react-hooks/rules-of-hooks - expect(hooks.state[key](testVal)).toEqual(React.useState(testVal)); - }; - test('provides altText state value', () => testStateMethod(state.keys.altText)); - test('provides dimensions state value', () => testStateMethod(state.keys.dimensions)); - test('provides showAltTextDismissibleError state value', () => - testStateMethod(state.keys.showAltTextDismissibleError)); - test('provides showAltTextSubmissionError state value', () => testStateMethod(state.keys.showAltTextSubmissionError)); - 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 lockAspectRatio state value', () => testStateMethod(state.keys.lockAspectRatio)); -}); - -describe('ImageSettingsModal hooks', () => { - beforeEach(() => { - jest.clearAllMocks(); - }); - describe('dimensions-related hooks', () => { - describe('getValidDimensions', () => { - it('returns local dimensions if not locked', () => { - expect(hooks.getValidDimensions({ - dimensions: simpleDims, - local: reducedDims, - isLocked: false, - lockAspectRatio: simpleDims, - })).toEqual(reducedDims); - }); - it('returns local dimensions if the same as stored', () => { - expect(hooks.getValidDimensions({ - dimensions: simpleDims, - local: simpleDims, - isLocked: true, - lockAspectRatio: reducedDims, - })).toEqual(simpleDims); - }); - 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 testDimensions = (newDimensions, expected) => { - const dimensions = { width: w, height: h }; - expect(hooks.getValidDimensions({ - 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(() => { - state.mock(); - hook = hooks.dimensionLockHooks({ dimensions: simpleDims }); - }); - afterEach(() => { - state.restore(); - }); - test('lockAspectRatio defaults to null', () => { - expect(hook.lockAspectRatio).toEqual(null); - }); - test('isLocked defaults to true', () => { - expect(hook.isLocked).toEqual(true); - }); - describe('initializeLock', () => { - it('calls setLockAspectRatio with the passed dimensions divided by their gcd', () => { - hook.initializeLock(multiDims); - expect(state.setState.lockAspectRatio).toHaveBeenCalledWith(reducedDims); - }); - it('returns the values themselves if they have no gcd', () => { - jest.spyOn(hooks, hookKeys.findGcd).mockReturnValueOnce(1); - hook.initializeLock(simpleDims); - expect(state.setState.lockAspectRatio).toHaveBeenCalledWith(simpleDims); - }); - }); - test('lock sets isLocked to true', () => { - hook = hooks.dimensionLockHooks({ dimensions: simpleDims }); - hook.lock(); - expect(state.setState.isLocked).toHaveBeenCalledWith(true); - }); - test('unlock sets locked to null', () => { - hook = hooks.dimensionLockHooks({ dimensions: simpleDims }); - hook.unlock(); - expect(state.setState.isLocked).toHaveBeenCalledWith(false); - }); - }); - describe('dimensionHooks', () => { - let lockHooks; - beforeEach(() => { - state.mock(); - lockHooks = { - initializeLock: jest.fn(), - lock: jest.fn(), - unlock: jest.fn(), - locked: { ...reducedDims }, - }; - jest.spyOn(hooks, hookKeys.dimensionLockHooks).mockReturnValueOnce(lockHooks); - hook = hooks.dimensionHooks(); - }); - afterEach(() => { - state.restore(); - }); - it('initializes dimension lock hooks with incoming dimension value', () => { - state.mockVal(state.keys.dimensions, reducedDims); - hook = hooks.dimensionHooks(); - expect(hooks.dimensionLockHooks).toHaveBeenCalledWith({ dimensions: reducedDims }); - }); - test('value is tied to local state', () => { - state.mockVal(state.keys.local, simpleDims); - hook = hooks.dimensionHooks(); - expect(hook.value).toEqual(simpleDims); - }); - describe('onImgLoad', () => { - const img = { naturalHeight: 200, naturalWidth: 345 }; - const evt = { target: img }; - it('calls initializeDimensions with selection dimensions if passed', () => { - hook.onImgLoad(simpleDims)(evt); - expect(state.setState.dimensions).toHaveBeenCalledWith(simpleDims); - expect(state.setState.local).toHaveBeenCalledWith(simpleDims); - }); - it('calls initializeDimensions with target image dimensions if no selection', () => { - hook.onImgLoad({})(evt); - const expected = { width: img.naturalWidth, height: img.naturalHeight }; - expect(state.setState.dimensions).toHaveBeenCalledWith(expected); - expect(state.setState.local).toHaveBeenCalledWith(expected); - }); - it('calls initializeLock', () => { - const initializeDimensions = jest.fn(); - hook.onImgLoad(initializeDimensions, simpleDims)(evt); - expect(lockHooks.initializeLock).toHaveBeenCalled(); - }); - }); - describe('setHeight', () => { - it('sets local height to int value of argument', () => { - state.mockVal(state.keys.local, simpleDims); - hooks.dimensionHooks().setHeight('23.4'); - expect(state.setState.local).toHaveBeenCalledWith({ ...simpleDims, height: 23 }); - }); - }); - describe('setWidth', () => { - it('sets local width to int value of argument', () => { - state.mockVal(state.keys.local, simpleDims); - hooks.dimensionHooks().setWidth('34.5'); - expect(state.setState.local).toHaveBeenCalledWith({ ...simpleDims, width: 34 }); - }); - }); - describe('updateDimensions', () => { - it('sets local and stored dimensions to newDimensions output', () => { - // store values we care about under height or width, and add junk data to be stripped out. - const testDims = (args) => ({ ...simpleDims, height: args }); - const getValidDimensions = (args) => ({ ...testDims(args), junk: 'data' }); - state.mockVal(state.keys.isLocked, true); - state.mockVal(state.keys.dimensions, simpleDims); - 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, - lockAspectRatio: reducedDims, - local: multiDims, - isLocked: true, - }); - expect(state.setState.local).toHaveBeenCalledWith(expected); - expect(state.setState.dimensions).toHaveBeenCalledWith(expected); - }); - }); - }); - }); - describe('altTextHooks', () => { - const value = 'myVAL'; - const isDecorative = true; - const showAltTextDismissibleError = true; - const showAltTextSubmissionError = true; - beforeEach(() => { - state.mock(); - hook = hooks.altTextHooks(); - }); - afterEach(() => { - state.restore(); - }); - it('returns value and isDecorative', () => { - state.mockVal(state.keys.altText, value); - state.mockVal(state.keys.isDecorative, isDecorative); - hook = hooks.altTextHooks(); - expect(hook.value).toEqual(value); - expect(hook.isDecorative).toEqual(isDecorative); - }); - test('setValue sets value', () => { - state.mockVal(state.keys.altText, value); - hook = hooks.altTextHooks(); - hook.setValue(value); - expect(state.setState.altText).toHaveBeenCalledWith(value); - }); - test('setIsDecorative sets isDecorative', () => { - state.mockVal(state.keys.altText, value); - hook = hooks.altTextHooks(); - hook.setIsDecorative(value); - expect(state.setState.isDecorative).toHaveBeenCalledWith(value); - }); - describe('error', () => { - test('show is initialized to false and returns properly', () => { - expect(hook.error.show).toEqual(false); - state.mockVal(state.keys.showAltTextDismissibleError, showAltTextDismissibleError); - hook = hooks.altTextHooks(); - expect(hook.error.show).toEqual(showAltTextDismissibleError); - }); - test('set sets showAltTextDismissibleError to true', () => { - hook.error.set(); - expect(state.setState.showAltTextDismissibleError).toHaveBeenCalledWith(true); - }); - test('dismiss sets showAltTextDismissibleError to false', () => { - hook.error.dismiss(); - expect(state.setState.showAltTextDismissibleError).toHaveBeenCalledWith(false); - }); - }); - describe('validation', () => { - test('show is initialized to false and returns properly', () => { - expect(hook.validation.show).toEqual(false); - state.mockVal(state.keys.showAltTextSubmissionError, showAltTextSubmissionError); - hook = hooks.altTextHooks(); - expect(hook.validation.show).toEqual(showAltTextSubmissionError); - }); - test('set sets showAltTextSubmissionError to true', () => { - hook.validation.set(); - expect(state.setState.showAltTextSubmissionError).toHaveBeenCalledWith(true); - }); - test('dismiss sets showAltTextSubmissionError to false', () => { - hook.validation.dismiss(); - expect(state.setState.showAltTextSubmissionError).toHaveBeenCalledWith(false); - }); - }); - }); - describe('onInputChange', () => { - it('calls handleValue with event value prop', () => { - const value = 'TEST value'; - const onChange = jest.fn(); - hooks.onInputChange(onChange)({ target: { value } }); - expect(onChange).toHaveBeenCalledWith(value); - }); - }); - describe('onCheckboxChange', () => { - it('calls handleValue with event checked prop', () => { - const checked = 'TEST value'; - const onChange = jest.fn(); - hooks.onCheckboxChange(onChange)({ target: { checked } }); - expect(onChange).toHaveBeenCalledWith(checked); - }); - }); - describe('checkFormValidation', () => { - const props = { - onAltTextFail: jest.fn().mockName('onAltTextFail'), - }; - beforeEach(() => { - props.altText = ''; - props.isDecorative = false; - }); - it('calls onAltTextFail when isDecorative is false and altText is an empty string', () => { - hooks.checkFormValidation({ ...props }); - expect(props.onAltTextFail).toHaveBeenCalled(); - }); - it('returns false when isDeocrative is false and altText is an empty string', () => { - expect(hooks.checkFormValidation({ ...props })).toEqual(false); - }); - it('returns true when isDecorative is true', () => { - props.isDecorative = true; - expect(hooks.checkFormValidation({ ...props })).toEqual(true); - }); - }); - describe('onSaveClick', () => { - const props = { - altText: { - error: { - show: true, - set: jest.fn(), - dismiss: jest.fn(), - }, - validation: { - show: true, - set: jest.fn(), - dismiss: jest.fn(), - }, - }, - dimensions: simpleDims, - saveToEditor: jest.fn().mockName('saveToEditor'), - }; - beforeEach(() => { - props.altText.value = 'What is this?'; - props.isDecorative = false; - }); - it('calls checkFormValidation', () => { - jest.spyOn(hooks, hookKeys.checkFormValidation); - hooks.onSaveClick({ ...props })(); - expect(hooks.checkFormValidation).toHaveBeenCalled(); - }); - it('calls saveToEditor with dimensions, altText and isDecorative when checkFormValidation is true', () => { - jest.spyOn(hooks, hookKeys.checkFormValidation).mockReturnValueOnce(true); - hooks.onSaveClick({ ...props })(); - expect(props.saveToEditor).toHaveBeenCalledWith({ - altText: props.altText.value, - dimensions: props.dimensions, - isDecorative: props.isDecorative, - }); - }); - it('replaces double quotes with " before saving to editor', () => { - props.altText.value = 'The "Submit For Grading" button'; - jest.spyOn(hooks, hookKeys.checkFormValidation).mockReturnValueOnce(true); - - hooks.onSaveClick({ ...props })(); - - expect(props.saveToEditor).toHaveBeenCalledWith({ - altText: 'The "Submit For Grading" button', - dimensions: props.dimensions, - isDecorative: props.isDecorative, - }); - }); - it('does not modify altText if there are no double quotes', () => { - props.altText.value = 'The Submit For Grading button'; - jest.spyOn(hooks, hookKeys.checkFormValidation).mockReturnValueOnce(true); - - hooks.onSaveClick({ ...props })(); - - expect(props.saveToEditor).toHaveBeenCalledWith({ - altText: 'The Submit For Grading button', - dimensions: props.dimensions, - isDecorative: props.isDecorative, - }); - }); - it('calls dismissError and sets showAltTextSubmissionError to false when checkFormValidation is true', () => { - jest.spyOn(hooks, hookKeys.checkFormValidation).mockReturnValueOnce(true); - hooks.onSaveClick({ ...props })(); - expect(props.altText.error.dismiss).toHaveBeenCalled(); - expect(props.altText.validation.dismiss).toHaveBeenCalled(); - }); - it('does not call saveEditor when checkFormValidation is false', () => { - jest.spyOn(hooks, hookKeys.checkFormValidation).mockReturnValueOnce(false); - hooks.onSaveClick({ ...props })(); - 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.tsx b/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/index.test.tsx index a55a365b3c..1de0fe20d0 100644 --- a/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/index.test.tsx +++ b/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/index.test.tsx @@ -1,27 +1,9 @@ +import { initializeMocks, render, screen } from '@src/testUtils'; +import { fireEvent } from '@testing-library/react'; +import { userEvent } from '@testing-library/user-event'; import React from 'react'; -import { render, screen, initializeMocks } from '@src/testUtils'; import ImageSettingsModal from '.'; - -jest.mock('./AltTextControls', () => 'AltTextControls'); -jest.mock('./DimensionControls', () => 'DimensionControls'); - -jest.mock('./hooks', () => ({ - altTextHooks: () => ({ - error: { - show: true, - dismiss: jest.fn(), - }, - isDecorative: false, - value: 'alternative Taxes', - }), - dimensionHooks: () => ({ - onImgLoad: jest.fn( - (selection) => ({ 'hooks.dimensions.onImgLoad.callback': { selection } }), - ).mockName('hooks.dimensions.onImgLoad'), - value: { width: 12, height: 13 }, - }), - onSaveClick: (args) => ({ 'hooks.onSaveClick': args }), -})); +import messages from './messages'; describe('ImageSettingsModal', () => { const props = { @@ -35,11 +17,126 @@ describe('ImageSettingsModal', () => { returnToSelection: jest.fn().mockName('props.returnToSelector'), saveToEditor: jest.fn().mockName('props.saveToEditor'), }; + const user = userEvent.setup(); beforeEach(() => { initializeMocks(); }); - test('renders component', () => { - render(); + test('Test null altText', () => { + render(); expect(screen.getByText('Image Settings')).toBeInTheDocument(); }); + test('Test clicking replace image', async () => { + render(); + await user.click(screen.getByRole('button', { name: /replace image/i })); + expect(props.returnToSelection).toHaveBeenCalled(); + }); + test('Test clicking cancel', async () => { + render(); + await user.click(screen.getByRole('button', { name: /cancel/i })); + expect(props.close).toHaveBeenCalled(); + }); + describe('Alt Text Editing', () => { + test('Empty Alt Text raises error if image is nto decorative', async () => { + render(); + await user.clear(screen.getByRole('textbox', { name: /alt text/i })); + await user.click(screen.getByRole('button', { name: 'Save' })); + expect(screen.getByText(messages.altTextLocalFeedback.defaultMessage)).toBeInTheDocument(); + expect(screen.getByText(messages.altTextError.defaultMessage)).toBeInTheDocument(); + expect(props.saveToEditor).not.toHaveBeenCalled(); + }); + test('Error can be dismissed', async () => { + render(); + await user.clear(screen.getByRole('textbox', { name: /alt text/i })); + await user.click(screen.getByRole('button', { name: 'Save' })); + expect(screen.getByText(messages.altTextError.defaultMessage)).toBeInTheDocument(); + await user.click(screen.getByRole('button', { name: 'Dismiss' })); + expect(screen.queryByText(messages.altTextError.defaultMessage)).not.toBeInTheDocument(); + }); + test('Empty Alt Text doesn\'t raise error if image is decorative', async () => { + render(); + await user.clear(screen.getByRole('textbox', { name: /alt text/i })); + await user.click(screen.getByRole('checkbox', { name: /decorative/i })); + await user.click(screen.getByRole('button', { name: 'Save' })); + expect(screen.queryByText(messages.altTextLocalFeedback.defaultMessage)).not.toBeInTheDocument(); + expect(screen.queryByText(messages.altTextError.defaultMessage)).not.toBeInTheDocument(); + expect(props.saveToEditor).toHaveBeenCalled(); + }); + test('If alt text is entered it does not raise any error', async () => { + render(); + await user.type(screen.getByRole('textbox', { name: /alt text/i }), 'some text'); + await user.click(screen.getByRole('button', { name: 'Save' })); + expect(screen.queryByText(messages.altTextLocalFeedback.defaultMessage)).not.toBeInTheDocument(); + expect(screen.queryByText(messages.altTextError.defaultMessage)).not.toBeInTheDocument(); + expect(props.saveToEditor).toHaveBeenCalled(); + }); + }); + describe('Image Dimensions Editing', () => { + function mockImageLoad(naturalWidth: number, naturalHeight: number) { + const img = screen.getByRole('img'); + Object.defineProperty(img, 'naturalWidth', { value: naturalWidth }); + Object.defineProperty(img, 'naturalHeight', { value: naturalHeight }); + fireEvent.load(img); + } + + test('Image dimensions are editable and saved correctly', async () => { + render(); + mockImageLoad(1920, 1080); + await user.type(await screen.findByRole('textbox', { name: /width/i }), '1280'); + await user.type(screen.getByRole('textbox', { name: /height/i }), '720'); + await user.click(screen.getByRole('button', { name: 'Save' })); + expect(screen.queryByText(messages.dimensionLocalFeedback.defaultMessage)).not.toBeInTheDocument(); + expect(props.saveToEditor).toHaveBeenCalled(); + }); + + test('Image dimensions are editable and saved correctly with percentages', async () => { + render(); + mockImageLoad(1920, 1080); + await user.type(await screen.findByRole('textbox', { name: /width/i }), '75%'); + await user.click(screen.getByRole('button', { name: 'Save' })); + expect(screen.queryByText(messages.dimensionLocalFeedback.defaultMessage)).not.toBeInTheDocument(); + expect(props.saveToEditor).toHaveBeenCalled(); + }); + + describe('Dimension Locking', () => { + test('When dimensions are locked it maintains the original ratio', async () => { + render(); + mockImageLoad(1920, 1080); + const widthInput = await screen.findByRole('textbox', { name: /width/i }); + const heightInput = await screen.findByRole('textbox', { name: /height/i }); + await user.clear(widthInput); + await user.type(widthInput, '1280'); + expect(heightInput).toHaveValue('720'); + await user.clear(heightInput); + await user.type(heightInput, '900'); + expect(widthInput).toHaveValue('1600'); + }); + + test('When dimensions are locked it maintains the original ratio with percentages', async () => { + render(); + mockImageLoad(1920, 1080); + const widthInput = await screen.findByRole('textbox', { name: /width/i }); + const heightInput = await screen.findByRole('textbox', { name: /height/i }); + await user.clear(widthInput); + await user.type(widthInput, '75%'); + expect(heightInput).toHaveValue('75%'); + await user.clear(heightInput); + await user.type(heightInput, '90%'); + expect(widthInput).toHaveValue('90%'); + }); + + test('When dimensions are not locked it allows any ratio', async () => { + render(); + mockImageLoad(1920, 1080); + await user.click(await screen.findByRole('button', { name: /unlock dimensions/i })); + const widthInput = await screen.findByRole('textbox', { name: /width/i }); + const heightInput = await screen.findByRole('textbox', { name: /height/i }); + await user.clear(widthInput); + await user.type(widthInput, '1280'); + expect(heightInput).toHaveValue('1080'); + await user.clear(heightInput); + await user.type(heightInput, '900'); + expect(widthInput).toHaveValue('1280'); + }); + }); + }); }); diff --git a/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/index.tsx b/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/index.tsx index 4d41f03fbd..5e997280d2 100644 --- a/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/index.tsx +++ b/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/index.tsx @@ -1,64 +1,81 @@ -import React from 'react'; -import PropTypes from 'prop-types'; +import { useIntl } from '@edx/frontend-platform/i18n'; + +import './index.scss'; import { Button, Image } from '@openedx/paragon'; import { ArrowBackIos } from '@openedx/paragon/icons'; -import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n'; - -import './index.scss'; -import * as hooks from './hooks'; -import messages from './messages'; +import { Formik, useFormikContext } from 'formik'; +import React from 'react'; +import * as Yup from 'yup'; import BaseModal from '../../BaseModal'; +import ErrorAlert from '../../ErrorAlerts/ErrorAlert'; import AltTextControls from './AltTextControls'; import DimensionControls from './DimensionControls'; -import ErrorAlert from '../../ErrorAlerts/ErrorAlert'; +import messages from './messages'; +import { + AltText, ImageConfig, ImageDimensions, OrigImageDimensions, +} from './types'; -/** - * Modal display wrapping the dimension and alt-text controls for image tags - * inserted into the TextEditor TinyMCE context. - * Provides a thumbnail and populates dimension and alt-text controls. - * @param {bool} isOpen - is the modal open? - * @param {func} close - close the modal - * @param {obj} selection - current image selection object - * @param {func} saveToEditor - save the current settings to the editor - * @param {func} returnToSelection - return to image selection - */ -const ImageSettingsModal = ({ - close, +interface ImageSettingsModalCommonProps { + close: () => void; + isOpen: boolean; + returnToSelection: () => void; + selection: { + altText: string | null; + externalUrl: string; + url: string; + }; +} + +interface ImageSettingsModalInternalProps extends ImageSettingsModalCommonProps { + imgDimensions: OrigImageDimensions; + setImgDimensions: (dimensions: OrigImageDimensions) => void; +} + +interface ImageSettingsModalProps extends ImageSettingsModalCommonProps { + saveToEditor: (config: AltText & { dimensions: ImageDimensions }) => void; +} + +const ImageSettingsModalInternal = ({ isOpen, + close, returnToSelection, - saveToEditor, selection, -}) => { + imgDimensions, + setImgDimensions, +}: ImageSettingsModalInternalProps) => { const intl = useIntl(); - const altText = hooks.altTextHooks(selection.altText); - const dimensions = hooks.dimensionHooks(altText); - const onSaveClick = hooks.onSaveClick({ - altText, - dimensions: dimensions.value, - isDecorative: altText.isDecorative, - saveToEditor, - }); + const formik = useFormikContext(); + + const setOriginalDimensions = React.useCallback(async (event: React.ChangeEvent) => { + const img = event.currentTarget as HTMLImageElement; + setImgDimensions({ width: img.naturalWidth, height: img.naturalHeight }); + await formik.setFieldValue('width', img.naturalWidth, false); + await formik.setFieldValue('height', img.naturalHeight, false); + }, [formik]); + + const handleDismissError = React.useCallback(() => formik.setFieldError('altText', ''), [formik]); + return ( - + {intl.formatMessage(messages.saveButtonLabel)} } isOpen={isOpen} title={intl.formatMessage(messages.titleLabel)} > - + {intl.formatMessage(messages.altTextError)}
- - + +
); }; -ImageSettingsModal.propTypes = { - close: PropTypes.func.isRequired, - isOpen: PropTypes.bool.isRequired, - returnToSelection: PropTypes.func.isRequired, - saveToEditor: PropTypes.func.isRequired, - selection: PropTypes.shape({ - altText: PropTypes.string, - externalUrl: PropTypes.string, - url: PropTypes.string, - }).isRequired, +/** + * Modal display wrapping the dimension and alt-text controls for image tags + * inserted into the TextEditor TinyMCE context. + * Provides a thumbnail and populates dimension and alt-text controls. + * @param {Object} props + * @param {bool} props.isOpen - is the modal open? + * @param {func} props.close - close the modal + * @param {Object} props.selection - current image selection object + * @param {func} props.saveToEditor - save the current settings to the editor + * @param {func} props.returnToSelection - return to image selection + */ +const ImageSettingsModal = ({ + close, + isOpen, + returnToSelection, + saveToEditor, + selection, +}: ImageSettingsModalProps) => { + const intl = useIntl(); + const [imgDimensions, setImgDimensions] = React.useState({ width: 0, height: 0 }); + const initialValues: ImageConfig = { + isLocked: true, + width: imgDimensions.width, + height: imgDimensions.height, + isDecorative: false, + altText: selection.altText || '', + }; + const validationSchema = Yup.object().shape({ + isLocked: Yup.boolean(), + isDecorative: Yup.boolean(), + width: Yup.string().trim().matches(/^[0-9]+%?$/).required(), + height: Yup.string().trim().matches(/^[0-9]+%?$/).required(), + altText: Yup.string().when('isDecorative', { + is: true, + otherwise: (schema) => schema.required(intl.formatMessage(messages.altTextLocalFeedback)), + }), + }); + + const handleSubmit = React.useCallback((value, actions) => { + saveToEditor({ + altText: value.altText, + dimensions: { + width: value.width, + height: value.height, + }, + isDecorative: value.isDecorative, + }); + actions.setSubmitting(false); + }, [saveToEditor]); + + return ( + + + + ); }; + export default ImageSettingsModal; diff --git a/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/types.ts b/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/types.ts new file mode 100644 index 0000000000..ffddfd8201 --- /dev/null +++ b/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/types.ts @@ -0,0 +1,18 @@ +export interface ImageDimensions { + width: string | number; + height: string | number; +} + +export interface OrigImageDimensions { + width: number; + height: number; +} + +export interface AltText { + altText: string; + isDecorative: boolean; +} + +export interface ImageConfig extends ImageDimensions, AltText { + isLocked: boolean; +} From ab1351a4d35f7f923d4b43978116312021d7ae3a Mon Sep 17 00:00:00 2001 From: "kshitij.sobti" Date: Tue, 14 Apr 2026 00:27:42 +0530 Subject: [PATCH 3/3] fix: Fix the spacing and ordering of UI elements in image editor --- .../ImageSettingsModal/AltTextControls.tsx | 33 +++++++++---------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/AltTextControls.tsx b/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/AltTextControls.tsx index c9d999471d..b57c983f6f 100644 --- a/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/AltTextControls.tsx +++ b/src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/AltTextControls.tsx @@ -12,13 +12,24 @@ const AltTextControls = () => { const formik = useFormikContext(); return ( <> - - - {intl.formatMessage(messages.accessibilityLabel)} - + + {intl.formatMessage(messages.accessibilityLabel)} + + + + + {intl.formatMessage(messages.decorativeAltTextCheckboxLabel)} + + + + { )} - - - - {intl.formatMessage(messages.decorativeAltTextCheckboxLabel)} - - - ); };