From 8c125df9aaf2b71acaa5969e615cbf5515ebe64c Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Mon, 7 Oct 2024 19:04:49 -0700 Subject: [PATCH] feat: Open Editors in a Modal (library components only) [FC-0062] (#1357) * feat: allow opening editors in modals * refactor: add an EditorContext * test: update tests accordingly * test: make testUtils call clearAllMocks() automatically :) --- .../ContentTagsDrawer.test.jsx | 4 - src/editors/Editor.test.jsx | 55 ------ src/editors/Editor.tsx | 34 ++-- src/editors/EditorContext.tsx | 39 +++++ src/editors/EditorPage.jsx | 58 ------- src/editors/EditorPage.test.jsx | 32 ---- src/editors/EditorPage.test.tsx | 98 +++++++++++ src/editors/EditorPage.tsx | 58 +++++++ .../__snapshots__/Editor.test.jsx.snap | 36 ---- .../__snapshots__/EditorPage.test.jsx.snap | 59 ------- .../__snapshots__/index.test.jsx.snap | 161 ------------------ .../__snapshots__/index.test.jsx.snap | 114 ------------- .../components/EditorFooter/index.jsx | 64 ------- .../components/EditorFooter/index.test.jsx | 33 ---- .../components/EditorFooter/messages.js | 32 ---- .../containers/EditorContainer/hooks.test.jsx | 1 + .../EditorContainer/{hooks.js => hooks.ts} | 15 +- .../containers/EditorContainer/index.jsx | 104 ----------- .../containers/EditorContainer/index.test.jsx | 68 -------- .../containers/EditorContainer/index.test.tsx | 97 +++++++++++ .../containers/EditorContainer/index.tsx | 158 +++++++++++++++++ .../containers/EditorContainer/messages.ts | 30 ++++ .../__snapshots__/index.test.jsx.snap | 8 +- .../SelectTypeWrapper/SelectTypeFooter.jsx | 63 ------- .../SelectTypeFooter.test.jsx | 45 ----- .../SelectTypeFooter.test.jsx.snap | 36 ---- .../__snapshots__/index.test.jsx.snap | 38 ----- .../__snapshots__/index.test.tsx.snap | 61 +++++++ .../SelectTypeWrapper/index.jsx | 57 ------- .../{index.test.jsx => index.test.tsx} | 0 .../SelectTypeWrapper/index.tsx | 85 +++++++++ .../components/SelectTypeModal/index.test.tsx | 9 +- src/editors/data/services/cms/api.ts | 20 ++- src/editors/{hooks.js => hooks.ts} | 11 +- src/editors/{messages.js => messages.ts} | 0 .../LibraryAuthoringPage.test.tsx | 6 +- src/library-authoring/LibraryLayout.tsx | 47 +---- .../add-content/AddContentContainer.tsx | 16 +- .../collections/CollectionDetails.test.tsx | 2 - .../LibraryCollectionPage.test.tsx | 4 - src/library-authoring/common/context.tsx | 13 ++ .../component-info/ComponentInfo.test.tsx | 63 +++++++ .../component-info/ComponentInfo.tsx | 12 +- .../components/CollectionCard.test.tsx | 4 - .../components/ComponentCard.tsx | 8 +- .../components/ComponentEditorModal.tsx | 42 +++++ .../components/LibraryComponents.test.tsx | 1 - .../components/utils.test.ts | 27 --- src/library-authoring/components/utils.ts | 20 --- src/library-authoring/data/api.mocks.ts | 14 ++ src/testUtils.tsx | 3 + 51 files changed, 845 insertions(+), 1220 deletions(-) delete mode 100644 src/editors/Editor.test.jsx create mode 100644 src/editors/EditorContext.tsx delete mode 100644 src/editors/EditorPage.jsx delete mode 100644 src/editors/EditorPage.test.jsx create mode 100644 src/editors/EditorPage.test.tsx create mode 100644 src/editors/EditorPage.tsx delete mode 100644 src/editors/__snapshots__/Editor.test.jsx.snap delete mode 100644 src/editors/__snapshots__/EditorPage.test.jsx.snap delete mode 100644 src/editors/containers/EditorContainer/__snapshots__/index.test.jsx.snap delete mode 100644 src/editors/containers/EditorContainer/components/EditorFooter/__snapshots__/index.test.jsx.snap delete mode 100644 src/editors/containers/EditorContainer/components/EditorFooter/index.jsx delete mode 100644 src/editors/containers/EditorContainer/components/EditorFooter/index.test.jsx delete mode 100644 src/editors/containers/EditorContainer/components/EditorFooter/messages.js rename src/editors/containers/EditorContainer/{hooks.js => hooks.ts} (81%) delete mode 100644 src/editors/containers/EditorContainer/index.jsx delete mode 100644 src/editors/containers/EditorContainer/index.test.jsx create mode 100644 src/editors/containers/EditorContainer/index.test.tsx create mode 100644 src/editors/containers/EditorContainer/index.tsx delete mode 100644 src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/SelectTypeFooter.jsx delete mode 100644 src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/SelectTypeFooter.test.jsx delete mode 100644 src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/__snapshots__/SelectTypeFooter.test.jsx.snap delete mode 100644 src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/__snapshots__/index.test.jsx.snap create mode 100644 src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/__snapshots__/index.test.tsx.snap delete mode 100644 src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/index.jsx rename src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/{index.test.jsx => index.test.tsx} (100%) create mode 100644 src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/index.tsx rename src/editors/{hooks.js => hooks.ts} (77%) rename src/editors/{messages.js => messages.ts} (100%) create mode 100644 src/library-authoring/component-info/ComponentInfo.test.tsx create mode 100644 src/library-authoring/components/ComponentEditorModal.tsx delete mode 100644 src/library-authoring/components/utils.test.ts delete mode 100644 src/library-authoring/components/utils.ts diff --git a/src/content-tags-drawer/ContentTagsDrawer.test.jsx b/src/content-tags-drawer/ContentTagsDrawer.test.jsx index 8abd78e1fb..75ffe35b79 100644 --- a/src/content-tags-drawer/ContentTagsDrawer.test.jsx +++ b/src/content-tags-drawer/ContentTagsDrawer.test.jsx @@ -55,10 +55,6 @@ describe('', () => { initializeMocks(); }); - afterEach(() => { - jest.clearAllMocks(); - }); - it('should render page and page title correctly', () => { renderDrawer(stagedTagsId); expect(screen.getByText('Manage tags')).toBeInTheDocument(); diff --git a/src/editors/Editor.test.jsx b/src/editors/Editor.test.jsx deleted file mode 100644 index fa19e60689..0000000000 --- a/src/editors/Editor.test.jsx +++ /dev/null @@ -1,55 +0,0 @@ -import 'CourseAuthoring/editors/setupEditorTest'; -import React from 'react'; -import { useDispatch } from 'react-redux'; -import { shallow } from '@edx/react-unit-test-utils'; -import Editor from './Editor'; -import supportedEditors from './supportedEditors'; -import * as hooks from './hooks'; -import { blockTypes } from './data/constants/app'; - -jest.mock('./hooks', () => ({ - initializeApp: jest.fn(), -})); - -jest.mock('./containers/TextEditor', () => 'TextEditor'); -jest.mock('./containers/VideoEditor', () => 'VideoEditor'); -jest.mock('./containers/ProblemEditor', () => 'ProblemEditor'); - -const initData = { - blockId: 'block-v1:edX+DemoX+Demo_Course+type@html+block@030e35c4756a4ddc8d40b95fbbfff4d4', - blockType: blockTypes.html, - learningContextId: 'course-v1:edX+DemoX+Demo_Course', - lmsEndpointUrl: 'evenfakerurl.com', - studioEndpointUrl: 'fakeurl.com', -}; -const props = { - initialize: jest.fn(), - onClose: jest.fn().mockName('props.onClose'), - courseId: 'course-v1:edX+DemoX+Demo_Course', - ...initData, -}; - -let el; -describe('Editor', () => { - describe('render', () => { - test('snapshot: renders correct editor given blockType (html -> TextEditor)', () => { - expect(shallow().snapshot).toMatchSnapshot(); - }); - test('presents error message if no relevant editor found and ref ready', () => { - expect(shallow().snapshot).toMatchSnapshot(); - }); - test.each(Object.values(blockTypes))('renders %p editor when ref is ready', (blockType) => { - el = shallow(); - expect(el.shallowWrapper.props.children.props.children.type).toBe(supportedEditors[blockType]); - }); - }); - describe('behavior', () => { - it('calls initializeApp hook with dispatch, and passed data', () => { - el = shallow(); - expect(hooks.initializeApp).toHaveBeenCalledWith({ - dispatch: useDispatch(), - data: initData, - }); - }); - }); -}); diff --git a/src/editors/Editor.tsx b/src/editors/Editor.tsx index cc42647f56..8e52448242 100644 --- a/src/editors/Editor.tsx +++ b/src/editors/Editor.tsx @@ -1,3 +1,5 @@ +// Note: there is no Editor.test.tsx. This component only works together with +// as its parent, so they are tested together in EditorPage.test.tsx import React from 'react'; import { useDispatch } from 'react-redux'; import { FormattedMessage } from '@edx/frontend-platform/i18n'; @@ -7,6 +9,7 @@ import * as hooks from './hooks'; import supportedEditors from './supportedEditors'; import type { EditorComponent } from './EditorComponent'; +import { useEditorContext } from './EditorContext'; export interface Props extends EditorComponent { blockType: string; @@ -14,6 +17,7 @@ export interface Props extends EditorComponent { learningContextId: string | null; lmsEndpointUrl: string | null; studioEndpointUrl: string | null; + fullScreen?: boolean; } const Editor: React.FC = ({ @@ -36,23 +40,29 @@ const Editor: React.FC = ({ studioEndpointUrl, }, }); + const { fullScreen } = useEditorContext(); const EditorComponent = supportedEditors[blockType]; - return ( -
+ const innerEditor = (EditorComponent !== undefined) + ? + : ; + + if (fullScreen) { + return (
- {(EditorComponent !== undefined) - ? - : } +
+ {innerEditor} +
-
- ); + ); + } + return innerEditor; }; export default Editor; diff --git a/src/editors/EditorContext.tsx b/src/editors/EditorContext.tsx new file mode 100644 index 0000000000..e43b60a815 --- /dev/null +++ b/src/editors/EditorContext.tsx @@ -0,0 +1,39 @@ +import React from 'react'; + +/** + * Shared context that's used by all our editors. + * + * Note: we're in the process of moving things from redux into this. + */ +export interface EditorContext { + learningContextId: string; + /** + * When editing components in the libraries part of the Authoring MFE, we show + * the editors in a modal (fullScreen = false). This is the preferred approach + * so that authors can see context behind the modal. + * However, when making edits from the legacy course view, we display the + * editors in a fullscreen view. This approach is deprecated. + */ + fullScreen: boolean; +} + +const context = React.createContext(undefined); + +/** Hook to get the editor context (shared context) */ +export function useEditorContext() { + const ctx = React.useContext(context); + if (ctx === undefined) { + /* istanbul ignore next */ + throw new Error('This component needs to be wrapped in '); + } + return ctx; +} + +export const EditorContextProvider: React.FC<{ + children: React.ReactNode, + learningContextId: string; + fullScreen: boolean; +}> = ({ children, ...contextData }) => { + const ctx: EditorContext = React.useMemo(() => ({ ...contextData }), []); + return {children}; +}; diff --git a/src/editors/EditorPage.jsx b/src/editors/EditorPage.jsx deleted file mode 100644 index 60de6e1cc6..0000000000 --- a/src/editors/EditorPage.jsx +++ /dev/null @@ -1,58 +0,0 @@ -import React from 'react'; -import PropTypes from 'prop-types'; -import { Provider } from 'react-redux'; - -import store from './data/store'; -import Editor from './Editor'; -import ErrorBoundary from './sharedComponents/ErrorBoundary'; - -const EditorPage = ({ - courseId, - blockType, - blockId, - lmsEndpointUrl, - studioEndpointUrl, - onClose, - returnFunction, -}) => ( - - - - - -); -EditorPage.defaultProps = { - blockId: null, - courseId: null, - lmsEndpointUrl: null, - onClose: null, - returnFunction: null, - studioEndpointUrl: null, -}; - -EditorPage.propTypes = { - blockId: PropTypes.string, - blockType: PropTypes.string.isRequired, - courseId: PropTypes.string, - lmsEndpointUrl: PropTypes.string, - onClose: PropTypes.func, - returnFunction: PropTypes.func, - studioEndpointUrl: PropTypes.string, -}; - -export default EditorPage; diff --git a/src/editors/EditorPage.test.jsx b/src/editors/EditorPage.test.jsx deleted file mode 100644 index 24dfffe293..0000000000 --- a/src/editors/EditorPage.test.jsx +++ /dev/null @@ -1,32 +0,0 @@ -import React from 'react'; -import { shallow } from '@edx/react-unit-test-utils'; -import EditorPage from './EditorPage'; - -const props = { - courseId: 'course-v1:edX+DemoX+Demo_Course', - blockType: 'html', - blockId: 'block-v1:edX+DemoX+Demo_Course+type@html+block@030e35c4756a4ddc8d40b95fbbfff4d4', - lmsEndpointUrl: 'evenfakerurl.com', - studioEndpointUrl: 'fakeurl.com', - onClose: jest.fn().mockName('props.onClose'), -}; -jest.mock('react-redux', () => ({ - Provider: 'Provider', - connect: (mapStateToProps, mapDispatchToProps) => (component) => ({ - mapStateToProps, - mapDispatchToProps, - component, - }), -})); -jest.mock('./Editor', () => 'Editor'); - -describe('Editor Page', () => { - describe('snapshots', () => { - test('rendering correctly with expected Input', () => { - expect(shallow().snapshot).toMatchSnapshot(); - }); - test('props besides blockType default to null', () => { - expect(shallow().snapshot).toMatchSnapshot(); - }); - }); -}); diff --git a/src/editors/EditorPage.test.tsx b/src/editors/EditorPage.test.tsx new file mode 100644 index 0000000000..e4d0de4091 --- /dev/null +++ b/src/editors/EditorPage.test.tsx @@ -0,0 +1,98 @@ +import { snakeCaseObject } from '@edx/frontend-platform'; +import { + render, + screen, + initializeMocks, +} from '../testUtils'; +import editorCmsApi from './data/services/cms/api'; + +import EditorPage from './EditorPage'; + +// Mock this plugins component: +jest.mock('frontend-components-tinymce-advanced-plugins', () => ({ a11ycheckerCss: '' })); +// Always mock out the "fetch course images" endpoint: +jest.spyOn(editorCmsApi, 'fetchImages').mockImplementation(async () => ( // eslint-disable-next-line + { data: { assets: [], start: 0, end: 0, page: 0, pageSize: 50, totalCount: 0 } } +)); +// Mock out the 'get ancestors' API: +jest.spyOn(editorCmsApi, 'fetchByUnitId').mockImplementation(async () => ({ + status: 200, + data: { + ancestors: [{ + id: 'block-v1:Org+TS100+24+type@vertical+block@parent', + display_name: 'You-Knit? The Test Unit', + category: 'vertical', + has_children: true, + }], + }, +})); + +const defaultPropsHtml = { + blockId: 'block-v1:Org+TS100+24+type@html+block@123456html', + blockType: 'html', + courseId: 'course-v1:Org+TS100+24', + lmsEndpointUrl: 'http://lms.test.none/', + studioEndpointUrl: 'http://cms.test.none/', + onClose: jest.fn(), + fullScreen: false, +}; +const fieldsHtml = { + displayName: 'Introduction to Testing', + data: '

This is a text component which uses HTML.

', + metadata: { displayName: 'Introduction to Testing' }, +}; + +describe('EditorPage', () => { + beforeEach(() => { + initializeMocks(); + }); + + test('it can display the Text (html) editor in a modal', async () => { + jest.spyOn(editorCmsApi, 'fetchBlockById').mockImplementationOnce(async () => ( + { status: 200, data: snakeCaseObject(fieldsHtml) } + )); + + render(); + + // Then the editor should open + expect(await screen.findByRole('heading', { name: /Introduction to Testing/ })).toBeInTheDocument(); + + const modalElement = screen.getByRole('dialog'); + expect(modalElement.classList).toContain('pgn__modal'); + expect(modalElement.classList).toContain('pgn__modal-xl'); + expect(modalElement.classList).not.toContain('pgn__modal-fullscreen'); + }); + + test('it can display the Text (html) editor as a full page (when coming from the legacy UI)', async () => { + jest.spyOn(editorCmsApi, 'fetchBlockById').mockImplementationOnce(async () => ( + { status: 200, data: snakeCaseObject(fieldsHtml) } + )); + + render(); + + // Then the editor should open + expect(await screen.findByRole('heading', { name: /Introduction to Testing/ })).toBeInTheDocument(); + + const modalElement = screen.getByRole('dialog'); + expect(modalElement.classList).toContain('pgn__modal-fullscreen'); + expect(modalElement.classList).not.toContain('pgn__modal'); + expect(modalElement.classList).not.toContain('pgn__modal-xl'); + }); + + test('it shows an error message if there is no corresponding editor', async () => { + // We can edit 'html', 'problem', and 'video' blocks. + // But if we try to edit some other type, say 'fake', we should get an error: + jest.spyOn(editorCmsApi, 'fetchBlockById').mockImplementationOnce(async () => ( // eslint-disable-next-line + { status: 200, data: { display_name: 'Fake Un-editable Block', category: 'fake', metadata: {}, data: '' } } + )); + + const defaultPropsFake = { + ...defaultPropsHtml, + blockId: 'block-v1:Org+TS100+24+type@fake+block@123456fake', + blockType: 'fake', + }; + render(); + + expect(await screen.findByText('Error: Could Not find Editor')).toBeInTheDocument(); + }); +}); diff --git a/src/editors/EditorPage.tsx b/src/editors/EditorPage.tsx new file mode 100644 index 0000000000..a0dd365954 --- /dev/null +++ b/src/editors/EditorPage.tsx @@ -0,0 +1,58 @@ +import React from 'react'; +import { Provider } from 'react-redux'; + +import store from './data/store'; +import Editor from './Editor'; +import ErrorBoundary from './sharedComponents/ErrorBoundary'; +import { EditorComponent } from './EditorComponent'; +import { EditorContextProvider } from './EditorContext'; + +interface Props extends EditorComponent { + blockId?: string; + blockType: string; + courseId: string; + lmsEndpointUrl?: string; + studioEndpointUrl?: string; + fullScreen?: boolean; + children?: never; +} + +/** + * Wraps the editors with the redux state provider. + * TODO: refactor some of this to be React Context and React Query + */ +const EditorPage: React.FC = ({ + courseId, + blockType, + blockId = null, + lmsEndpointUrl = null, + studioEndpointUrl = null, + onClose = null, + returnFunction = null, + fullScreen = true, +}) => ( + + + + + + + +); + +export default EditorPage; diff --git a/src/editors/__snapshots__/Editor.test.jsx.snap b/src/editors/__snapshots__/Editor.test.jsx.snap deleted file mode 100644 index 5c9cb23a39..0000000000 --- a/src/editors/__snapshots__/Editor.test.jsx.snap +++ /dev/null @@ -1,36 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`Editor render presents error message if no relevant editor found and ref ready 1`] = ` -
-
- -
-
-`; - -exports[`Editor render snapshot: renders correct editor given blockType (html -> TextEditor) 1`] = ` -
-
- -
-
-`; diff --git a/src/editors/__snapshots__/EditorPage.test.jsx.snap b/src/editors/__snapshots__/EditorPage.test.jsx.snap deleted file mode 100644 index 7e15005764..0000000000 --- a/src/editors/__snapshots__/EditorPage.test.jsx.snap +++ /dev/null @@ -1,59 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`Editor Page snapshots props besides blockType default to null 1`] = ` - - - - - -`; - -exports[`Editor Page snapshots rendering correctly with expected Input 1`] = ` - - - - - -`; diff --git a/src/editors/containers/EditorContainer/__snapshots__/index.test.jsx.snap b/src/editors/containers/EditorContainer/__snapshots__/index.test.jsx.snap deleted file mode 100644 index 02c89e55d7..0000000000 --- a/src/editors/containers/EditorContainer/__snapshots__/index.test.jsx.snap +++ /dev/null @@ -1,161 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`EditorContainer component render snapshot: initialized. enable save and pass to header 1`] = ` -
- - - - } - footerAction={null} - headerComponent={null} - isFullscreenScroll={true} - isOpen={false} - size="md" - title="Exit the editor?" - > - - - -
-

- -

- -
- - -

- My test content -

- - -
-`; - -exports[`EditorContainer component render snapshot: not initialized. disable save and pass to header 1`] = ` -
- - - - } - footerAction={null} - headerComponent={null} - isFullscreenScroll={true} - isOpen={false} - size="md" - title="Exit the editor?" - > - - - -
-

- -

- -
- - - -
-`; diff --git a/src/editors/containers/EditorContainer/components/EditorFooter/__snapshots__/index.test.jsx.snap b/src/editors/containers/EditorContainer/components/EditorFooter/__snapshots__/index.test.jsx.snap deleted file mode 100644 index dbfca713db..0000000000 --- a/src/editors/containers/EditorContainer/components/EditorFooter/__snapshots__/index.test.jsx.snap +++ /dev/null @@ -1,114 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`EditorFooter render snapshot: default args (disableSave: false, saveFailed: false) 1`] = ` -
- - - - - - -
-`; - -exports[`EditorFooter render snapshot: save disabled. Show button spinner 1`] = ` -
- - - - - - -
-`; - -exports[`EditorFooter render snapshot: save failed. Show error message 1`] = ` -
- - - - - - - - - -
-`; diff --git a/src/editors/containers/EditorContainer/components/EditorFooter/index.jsx b/src/editors/containers/EditorContainer/components/EditorFooter/index.jsx deleted file mode 100644 index 20c2f2d560..0000000000 --- a/src/editors/containers/EditorContainer/components/EditorFooter/index.jsx +++ /dev/null @@ -1,64 +0,0 @@ -import React from 'react'; -import PropTypes from 'prop-types'; - -import { - Spinner, - ActionRow, - Button, - ModalDialog, - Toast, -} from '@openedx/paragon'; -import { FormattedMessage, injectIntl, intlShape } from '@edx/frontend-platform/i18n'; - -import messages from './messages'; - -const EditorFooter = ({ - clearSaveFailed, - disableSave, - onCancel, - onSave, - saveFailed, - // injected - intl, -}) => ( -
- {saveFailed && ( - - - - )} - - - - - - -
-); - -EditorFooter.propTypes = { - clearSaveFailed: PropTypes.func.isRequired, - disableSave: PropTypes.bool.isRequired, - onCancel: PropTypes.func.isRequired, - onSave: PropTypes.func.isRequired, - saveFailed: PropTypes.bool.isRequired, - // injected - intl: intlShape.isRequired, -}; - -export const EditorFooterInternal = EditorFooter; // For testing only -export default injectIntl(EditorFooter); diff --git a/src/editors/containers/EditorContainer/components/EditorFooter/index.test.jsx b/src/editors/containers/EditorContainer/components/EditorFooter/index.test.jsx deleted file mode 100644 index aaa78980a2..0000000000 --- a/src/editors/containers/EditorContainer/components/EditorFooter/index.test.jsx +++ /dev/null @@ -1,33 +0,0 @@ -import 'CourseAuthoring/editors/setupEditorTest'; -import React from 'react'; -import { shallow } from '@edx/react-unit-test-utils'; - -import { formatMessage } from '../../../../testUtils'; -import { EditorFooterInternal as EditorFooter } from '.'; - -jest.mock('../../hooks', () => ({ - nullMethod: jest.fn().mockName('hooks.nullMethod'), -})); - -describe('EditorFooter', () => { - const props = { - intl: { formatMessage }, - disableSave: false, - onCancel: jest.fn().mockName('args.onCancel'), - onSave: jest.fn().mockName('args.onSave'), - saveFailed: false, - }; - describe('render', () => { - test('snapshot: default args (disableSave: false, saveFailed: false)', () => { - expect(shallow().snapshot).toMatchSnapshot(); - }); - - test('snapshot: save disabled. Show button spinner', () => { - expect(shallow().snapshot).toMatchSnapshot(); - }); - - test('snapshot: save failed. Show error message', () => { - expect(shallow().snapshot).toMatchSnapshot(); - }); - }); -}); diff --git a/src/editors/containers/EditorContainer/components/EditorFooter/messages.js b/src/editors/containers/EditorContainer/components/EditorFooter/messages.js deleted file mode 100644 index ce7503ff14..0000000000 --- a/src/editors/containers/EditorContainer/components/EditorFooter/messages.js +++ /dev/null @@ -1,32 +0,0 @@ -import { defineMessages } from '@edx/frontend-platform/i18n'; - -const messages = defineMessages({ - - contentSaveFailed: { - id: 'authoring.editorfooter.save.error', - defaultMessage: 'Error: Content save failed. Please check recent changes and try again later.', - description: 'Error message displayed when content fails to save.', - }, - cancelButtonAriaLabel: { - id: 'authoring.editorfooter.cancelButton.ariaLabel', - defaultMessage: 'Discard changes and return to learning context', - description: 'Screen reader label for cancel button', - }, - cancelButtonLabel: { - id: 'authoring.editorfooter.cancelButton.label', - defaultMessage: 'Cancel', - description: 'Label for cancel button', - }, - saveButtonAriaLabel: { - id: 'authoring.editorfooter.savebutton.ariaLabel', - defaultMessage: 'Save changes and return to learning context', - description: 'Screen reader label for save button', - }, - saveButtonLabel: { - id: 'authoring.editorfooter.savebutton.label', - defaultMessage: 'Save', - description: 'Label for Save button', - }, -}); - -export default messages; diff --git a/src/editors/containers/EditorContainer/hooks.test.jsx b/src/editors/containers/EditorContainer/hooks.test.jsx index e73534f5b3..815a4cc12a 100644 --- a/src/editors/containers/EditorContainer/hooks.test.jsx +++ b/src/editors/containers/EditorContainer/hooks.test.jsx @@ -118,6 +118,7 @@ describe('EditorContainer hooks', () => { destination: reactRedux.useSelector(selectors.app.returnUrl), analyticsEvent: analyticsEvt.editorCancelClick, analytics: reactRedux.useSelector(selectors.app.analytics), + returnFunction: null, }), ); }); diff --git a/src/editors/containers/EditorContainer/hooks.js b/src/editors/containers/EditorContainer/hooks.ts similarity index 81% rename from src/editors/containers/EditorContainer/hooks.js rename to src/editors/containers/EditorContainer/hooks.ts index 72b8a0e20f..935e3ad89d 100644 --- a/src/editors/containers/EditorContainer/hooks.js +++ b/src/editors/containers/EditorContainer/hooks.ts @@ -6,11 +6,6 @@ import { RequestKeys } from '../../data/constants/requests'; import { selectors } from '../../data/redux'; import { StrictDict } from '../../utils'; import * as appHooks from '../../hooks'; -// 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'; export const { clearSaveError, @@ -47,7 +42,7 @@ export const handleSaveClicked = ({ }; export const cancelConfirmModalToggle = () => { - const [isCancelConfirmOpen, setIsOpen] = module.state.isCancelConfirmModalOpen(false); + const [isCancelConfirmOpen, setIsOpen] = state.isCancelConfirmModalOpen(false); return { isCancelConfirmOpen, openCancelConfirmModal: () => setIsOpen(true), @@ -55,7 +50,13 @@ export const cancelConfirmModalToggle = () => { }; }; -export const handleCancel = ({ onClose, returnFunction }) => { +export const handleCancel = ({ + onClose = null, + returnFunction = null, +}: { + onClose?: (() => void) | null; + returnFunction?: (() => (result: any) => void) | null; +}): ((result?: any) => void) => { if (onClose) { return onClose; } diff --git a/src/editors/containers/EditorContainer/index.jsx b/src/editors/containers/EditorContainer/index.jsx deleted file mode 100644 index 9cfd96888b..0000000000 --- a/src/editors/containers/EditorContainer/index.jsx +++ /dev/null @@ -1,104 +0,0 @@ -import React from 'react'; -import { useDispatch } from 'react-redux'; -import PropTypes from 'prop-types'; - -import { - Icon, ModalDialog, IconButton, Button, -} from '@openedx/paragon'; -import { Close } from '@openedx/paragon/icons'; -import { injectIntl, intlShape, FormattedMessage } from '@edx/frontend-platform/i18n'; - -import BaseModal from '../../sharedComponents/BaseModal'; -import EditorFooter from './components/EditorFooter'; -import TitleHeader from './components/TitleHeader'; -import * as hooks from './hooks'; -import messages from './messages'; -import './index.scss'; - -const EditorContainer = ({ - children, - getContent, - onClose, - validateEntry, - returnFunction, - // injected - intl, -}) => { - const dispatch = useDispatch(); - const isInitialized = hooks.isInitialized(); - const { isCancelConfirmOpen, openCancelConfirmModal, closeCancelConfirmModal } = hooks.cancelConfirmModalToggle(); - const handleCancel = hooks.handleCancel({ onClose, returnFunction }); - return ( -
- { - handleCancel(); - if (returnFunction) { - closeCancelConfirmModal(); - } - }} - > - - - )} - isOpen={isCancelConfirmOpen} - close={closeCancelConfirmModal} - title={intl.formatMessage(messages.cancelConfirmTitle)} - > - - - -
-

- -

- -
- - - {isInitialized && children} - - -
- ); -}; -EditorContainer.defaultProps = { - onClose: null, - returnFunction: null, - validateEntry: null, -}; -EditorContainer.propTypes = { - children: PropTypes.node.isRequired, - getContent: PropTypes.func.isRequired, - onClose: PropTypes.func, - returnFunction: PropTypes.func, - validateEntry: PropTypes.func, - // injected - intl: intlShape.isRequired, -}; - -export const EditorContainerInternal = EditorContainer; // For testing only -export default injectIntl(EditorContainer); diff --git a/src/editors/containers/EditorContainer/index.test.jsx b/src/editors/containers/EditorContainer/index.test.jsx deleted file mode 100644 index 5360fdb7de..0000000000 --- a/src/editors/containers/EditorContainer/index.test.jsx +++ /dev/null @@ -1,68 +0,0 @@ -import 'CourseAuthoring/editors/setupEditorTest'; -import { shallow } from '@edx/react-unit-test-utils'; -import { useDispatch } from 'react-redux'; - -import { EditorContainerInternal as EditorContainer } from '.'; -import * as hooks from './hooks'; -import { formatMessage } from '../../testUtils'; - -const props = { - getContent: jest.fn().mockName('props.getContent'), - onClose: jest.fn().mockName('props.onClose'), - validateEntry: jest.fn().mockName('props.validateEntry'), - returnFunction: jest.fn().mockName('props.returnFunction'), - // inject - intl: { formatMessage }, -}; - -jest.mock('./hooks', () => ({ - clearSaveError: jest.fn().mockName('hooks.clearSaveError'), - isInitialized: jest.fn().mockReturnValue(true), - handleCancel: (args) => ({ handleCancel: args }), - handleSaveClicked: (args) => ({ handleSaveClicked: args }), - saveFailed: jest.fn().mockName('hooks.saveFailed'), - cancelConfirmModalToggle: jest.fn(() => ({ - isCancelConfirmOpen: false, - openCancelConfirmModal: jest.fn().mockName('openCancelConfirmModal'), - closeCancelConfirmModal: jest.fn().mockName('closeCancelConfirmModal'), - })), -})); - -let el; - -describe('EditorContainer component', () => { - describe('render', () => { - const testContent = (

My test content

); - test('snapshot: not initialized. disable save and pass to header', () => { - hooks.isInitialized.mockReturnValueOnce(false); - expect( - shallow({testContent}).snapshot, - ).toMatchSnapshot(); - }); - test('snapshot: initialized. enable save and pass to header', () => { - expect( - shallow({testContent}).snapshot, - ).toMatchSnapshot(); - }); - describe('behavior inspection', () => { - beforeEach(() => { - el = shallow({testContent}); - }); - test('save behavior is linked to footer onSave', () => { - const expected = hooks.handleSaveClicked({ - dispatch: useDispatch(), - getContent: props.getContent, - validateEntry: props.validateEntry, - returnFunction: props.returnFunction, - }); - expect(el.shallowWrapper.props.children[3] - .props.onSave).toEqual(expected); - }); - test('behavior is linked to clearSaveError', () => { - const expected = hooks.clearSaveError({ dispatch: useDispatch() }); - expect(el.shallowWrapper.props.children[3] - .props.clearSaveFailed).toEqual(expected); - }); - }); - }); -}); diff --git a/src/editors/containers/EditorContainer/index.test.tsx b/src/editors/containers/EditorContainer/index.test.tsx new file mode 100644 index 0000000000..2d63041341 --- /dev/null +++ b/src/editors/containers/EditorContainer/index.test.tsx @@ -0,0 +1,97 @@ +import { snakeCaseObject } from '@edx/frontend-platform'; +import { + render, + screen, + initializeMocks, + fireEvent, + waitFor, +} from '../../../testUtils'; +import editorCmsApi from '../../data/services/cms/api'; + +import EditorPage from '../../EditorPage'; + +// Mock this plugins component: +jest.mock('frontend-components-tinymce-advanced-plugins', () => ({ a11ycheckerCss: '' })); +// Always mock out the "fetch course images" endpoint: +jest.spyOn(editorCmsApi, 'fetchImages').mockImplementation(async () => ( // eslint-disable-next-line + { data: { assets: [], start: 0, end: 0, page: 0, pageSize: 50, totalCount: 0 } } +)); +// Mock out the 'get ancestors' API: +jest.spyOn(editorCmsApi, 'fetchByUnitId').mockImplementation(async () => ({ + status: 200, + data: { + ancestors: [{ + id: 'block-v1:Org+TS100+24+type@vertical+block@parent', + display_name: 'You-Knit? The Test Unit', + category: 'vertical', + has_children: true, + }], + }, +})); + +const defaultPropsHtml = { + blockId: 'block-v1:Org+TS100+24+type@html+block@123456html', + blockType: 'html', + courseId: 'course-v1:Org+TS100+24', + lmsEndpointUrl: 'http://lms.test.none/', + studioEndpointUrl: 'http://cms.test.none/', + onClose: jest.fn(), + fullScreen: false, +}; +const fieldsHtml = { + displayName: 'Introduction to Testing', + data: '

This is a text component which uses HTML.

', + metadata: { displayName: 'Introduction to Testing' }, +}; + +describe('EditorContainer', () => { + beforeEach(() => { + initializeMocks(); + }); + + test('it displays a confirmation dialog when closing the editor modal', async () => { + jest.spyOn(editorCmsApi, 'fetchBlockById').mockImplementationOnce(async () => ( + { status: 200, data: snakeCaseObject(fieldsHtml) } + )); + + render(); + + // Then the editor should open + expect(await screen.findByRole('heading', { name: /Introduction to Testing/ })).toBeInTheDocument(); + + // Assert the "are you sure?" message isn't visible yet + const confirmMessage = /Are you sure you want to exit the editor/; + expect(screen.queryByText(confirmMessage)).not.toBeInTheDocument(); + + // Find and click the close button + const closeButton = await screen.findByRole('button', { name: 'Exit the editor' }); + fireEvent.click(closeButton); + // Now we should see the confirmation message: + expect(await screen.findByText(confirmMessage)).toBeInTheDocument(); + + expect(defaultPropsHtml.onClose).not.toHaveBeenCalled(); + // And can confirm the cancelation: + const confirmButton = await screen.findByRole('button', { name: 'OK' }); + fireEvent.click(confirmButton); + expect(defaultPropsHtml.onClose).toHaveBeenCalled(); + }); + + test('it disables the save button until the fields have been loaded', async () => { + // Mock that loading the block data has begun but not completed yet: + let resolver: (result: { data: any }) => void; + jest.spyOn(editorCmsApi, 'fetchBlockById').mockImplementationOnce(() => new Promise((r) => { resolver = r; })); + + render(); + + // Then the editor should open. The "Save" button should be disabled + const saveButton = await screen.findByRole('button', { name: /Save changes and return/ }); + expect(saveButton).toBeDisabled(); + + // Now complete the loading of the data: + await waitFor(() => expect(resolver).not.toBeUndefined()); + resolver!({ data: snakeCaseObject(fieldsHtml) }); + + // Now the save button should be active: + await waitFor(() => expect(saveButton).not.toBeDisabled()); + }); +}); diff --git a/src/editors/containers/EditorContainer/index.tsx b/src/editors/containers/EditorContainer/index.tsx new file mode 100644 index 0000000000..3414680b58 --- /dev/null +++ b/src/editors/containers/EditorContainer/index.tsx @@ -0,0 +1,158 @@ +import React from 'react'; +import { useDispatch } from 'react-redux'; + +import { + ActionRow, + Button, + Icon, + IconButton, + ModalDialog, + Spinner, + Toast, +} from '@openedx/paragon'; +import { Close } from '@openedx/paragon/icons'; +import { useIntl, FormattedMessage } from '@edx/frontend-platform/i18n'; + +import { EditorComponent } from '../../EditorComponent'; +import { useEditorContext } from '../../EditorContext'; +import BaseModal from '../../sharedComponents/BaseModal'; +import TitleHeader from './components/TitleHeader'; +import * as hooks from './hooks'; +import messages from './messages'; +import './index.scss'; + +interface WrapperProps { + children: React.ReactNode; +} + +export const EditorModalWrapper: React.FC void }> = ({ children, onClose }) => { + const { fullScreen } = useEditorContext(); + const intl = useIntl(); + if (fullScreen) { + return ( +
+ {children} +
+ ); + } + const title = intl.formatMessage(messages.modalTitle); + return ( + {children} + ); +}; + +export const EditorModalBody: React.FC = ({ children }) => { + const { fullScreen } = useEditorContext(); + return { children }; +}; + +export const FooterWrapper: React.FC = ({ children }) => { + const { fullScreen } = useEditorContext(); + if (fullScreen) { + return
{children}
; + } + // eslint-disable-next-line react/jsx-no-useless-fragment + return <>{ children }; +}; + +interface Props extends EditorComponent { + children: React.ReactNode; + getContent: Function; + validateEntry?: Function | null; +} + +const EditorContainer: React.FC = ({ + children, + getContent, + onClose = null, + validateEntry = null, + returnFunction = null, +}) => { + const intl = useIntl(); + const dispatch = useDispatch(); + const isInitialized = hooks.isInitialized(); + const { isCancelConfirmOpen, openCancelConfirmModal, closeCancelConfirmModal } = hooks.cancelConfirmModalToggle(); + const handleCancel = hooks.handleCancel({ onClose, returnFunction }); + const disableSave = !isInitialized; + const saveFailed = hooks.saveFailed(); + const clearSaveFailed = hooks.clearSaveError({ dispatch }); + const onSave = hooks.handleSaveClicked({ + dispatch, + getContent, + validateEntry, + returnFunction, + }); + return ( + + {saveFailed && ( + + + + )} + { + handleCancel(); + if (returnFunction) { + closeCancelConfirmModal(); + } + }} + > + + + )} + isOpen={isCancelConfirmOpen} + close={closeCancelConfirmModal} + title={intl.formatMessage(messages.cancelConfirmTitle)} + > + + + +
+

+ +

+ +
+ + + {isInitialized && children} + + + + + + + + + + + ); +}; + +export default EditorContainer; diff --git a/src/editors/containers/EditorContainer/messages.ts b/src/editors/containers/EditorContainer/messages.ts index a6f1754fb2..55b4259ca9 100644 --- a/src/editors/containers/EditorContainer/messages.ts +++ b/src/editors/containers/EditorContainer/messages.ts @@ -22,6 +22,36 @@ const messages = defineMessages({ defaultMessage: 'OK', description: 'Label for OK button', }, + modalTitle: { + id: 'authoring.editorContainer.accessibleTitle', + defaultMessage: 'Editor Dialog', + description: 'Text that labels the the editor modal dialog for non-visual users', + }, + contentSaveFailed: { + id: 'authoring.editorfooter.save.error', + defaultMessage: 'Error: Content save failed. Please check recent changes and try again later.', + description: 'Error message displayed when content fails to save.', + }, + cancelButtonAriaLabel: { + id: 'authoring.editorfooter.cancelButton.ariaLabel', + defaultMessage: 'Discard changes and return to learning context', + description: 'Screen reader label for cancel button', + }, + cancelButtonLabel: { + id: 'authoring.editorfooter.cancelButton.label', + defaultMessage: 'Cancel', + description: 'Label for cancel button', + }, + saveButtonAriaLabel: { + id: 'authoring.editorfooter.savebutton.ariaLabel', + defaultMessage: 'Save changes and return to learning context', + description: 'Screen reader label for save button', + }, + saveButtonLabel: { + id: 'authoring.editorfooter.savebutton.label', + defaultMessage: 'Save', + description: 'Label for Save button', + }, }); export default messages; diff --git a/src/editors/containers/ProblemEditor/components/EditProblemView/__snapshots__/index.test.jsx.snap b/src/editors/containers/ProblemEditor/components/EditProblemView/__snapshots__/index.test.jsx.snap index 24c6543af9..428feabce6 100644 --- a/src/editors/containers/ProblemEditor/components/EditProblemView/__snapshots__/index.test.jsx.snap +++ b/src/editors/containers/ProblemEditor/components/EditProblemView/__snapshots__/index.test.jsx.snap @@ -1,7 +1,7 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`EditorProblemView component renders raw editor 1`] = ` - @@ -66,11 +66,11 @@ exports[`EditorProblemView component renders raw editor 1`] = ` /> - + `; exports[`EditorProblemView component renders simple view 1`] = ` - @@ -139,5 +139,5 @@ exports[`EditorProblemView component renders simple view 1`] = ` /> - + `; diff --git a/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/SelectTypeFooter.jsx b/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/SelectTypeFooter.jsx deleted file mode 100644 index cd41940bb3..0000000000 --- a/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/SelectTypeFooter.jsx +++ /dev/null @@ -1,63 +0,0 @@ -import React from 'react'; -import PropTypes from 'prop-types'; -import { useDispatch, useSelector } from 'react-redux'; -import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n'; -import { - ActionRow, - Button, - ModalDialog, -} from '@openedx/paragon'; -import messages from './messages'; -import * as hooks from '../hooks'; - -import { actions, selectors } from '../../../../../data/redux'; - -const SelectTypeFooter = ({ - onCancel, - selected, -}) => { - const intl = useIntl(); - const defaultSettings = useSelector(selectors.problem.defaultSettings); - const dispatch = useDispatch(); - const updateField = React.useCallback((data) => dispatch(actions.problem.updateField(data)), [dispatch]); - const setBlockTitle = React.useCallback((title) => dispatch(actions.app.setBlockTitle(title)), [dispatch]); - return ( -
- - - - - - - -
- ); -}; - -SelectTypeFooter.defaultProps = { - selected: null, -}; - -SelectTypeFooter.propTypes = { - onCancel: PropTypes.func.isRequired, - selected: PropTypes.string, -}; - -export default SelectTypeFooter; diff --git a/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/SelectTypeFooter.test.jsx b/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/SelectTypeFooter.test.jsx deleted file mode 100644 index d0795c36ba..0000000000 --- a/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/SelectTypeFooter.test.jsx +++ /dev/null @@ -1,45 +0,0 @@ -import 'CourseAuthoring/editors/setupEditorTest'; -import { shallow } from '@edx/react-unit-test-utils'; - -import { Button } from '@openedx/paragon'; -import { formatMessage } from '../../../../../testUtils'; -import SelectTypeFooter from './SelectTypeFooter'; -import * as hooks from '../hooks'; - -jest.mock('../hooks', () => ({ - onSelect: jest.fn().mockName('onSelect'), -})); - -describe('SelectTypeFooter', () => { - const props = { - onCancel: jest.fn().mockName('onCancel'), - selected: null, - // redux - defaultSettings: {}, - updateField: jest.fn().mockName('UpdateField'), - // inject - intl: { formatMessage }, - }; - - test('snapshot', () => { - expect(shallow().snapshot).toMatchSnapshot(); - }); - - describe('behavior', () => { - let el; - beforeEach(() => { - el = shallow(); - }); - test('close behavior is linked to modal onCancel', () => { - const expected = props.onCancel; - expect(el.instance.findByType(Button)[0].props.onClick) - .toEqual(expected); - }); - test('select behavior is linked to modal onSelect', () => { - const expected = hooks.onSelect(props.selected, props.updateField); - const button = el.instance.findByType(Button); - expect(button[button.length - 1].props.onClick) - .toEqual(expected); - }); - }); -}); diff --git a/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/__snapshots__/SelectTypeFooter.test.jsx.snap b/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/__snapshots__/SelectTypeFooter.test.jsx.snap deleted file mode 100644 index 5d136ff50b..0000000000 --- a/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/__snapshots__/SelectTypeFooter.test.jsx.snap +++ /dev/null @@ -1,36 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`SelectTypeFooter snapshot 1`] = ` -
- - - - - - - -
-`; diff --git a/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/__snapshots__/index.test.jsx.snap b/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/__snapshots__/index.test.jsx.snap deleted file mode 100644 index fb228f3205..0000000000 --- a/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/__snapshots__/index.test.jsx.snap +++ /dev/null @@ -1,38 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`SelectTypeWrapper snapshot 1`] = ` -
- - - -
- -
- - - -

- test child -

- - -
-`; diff --git a/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/__snapshots__/index.test.tsx.snap b/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/__snapshots__/index.test.tsx.snap new file mode 100644 index 0000000000..44c6994873 --- /dev/null +++ b/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/__snapshots__/index.test.tsx.snap @@ -0,0 +1,61 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`SelectTypeWrapper snapshot 1`] = ` + + + + +
+ +
+ + + +

+ test child +

+ + + + + + + + + + + +`; diff --git a/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/index.jsx b/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/index.jsx deleted file mode 100644 index e452967172..0000000000 --- a/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/index.jsx +++ /dev/null @@ -1,57 +0,0 @@ -import React from 'react'; -import PropTypes from 'prop-types'; -import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n'; -import { Icon, ModalDialog, IconButton } from '@openedx/paragon'; -import { Close } from '@openedx/paragon/icons'; -import SelectTypeFooter from './SelectTypeFooter'; - -import * as hooks from '../../../../EditorContainer/hooks'; -import ecMessages from '../../../../EditorContainer/messages'; -import messages from './messages'; - -const SelectTypeWrapper = ({ - children, - onClose, - selected, -}) => { - const handleCancel = hooks.handleCancel({ onClose }); - const intl = useIntl(); - - return ( -
- - - -
- -
- - - - {children} - - -
- ); -}; - -SelectTypeWrapper.defaultProps = { - onClose: null, -}; -SelectTypeWrapper.propTypes = { - selected: PropTypes.string.isRequired, - children: PropTypes.node.isRequired, - onClose: PropTypes.func, -}; - -export default SelectTypeWrapper; diff --git a/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/index.test.jsx b/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/index.test.tsx similarity index 100% rename from src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/index.test.jsx rename to src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/index.test.tsx diff --git a/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/index.tsx b/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/index.tsx new file mode 100644 index 0000000000..5df0c0be09 --- /dev/null +++ b/src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/index.tsx @@ -0,0 +1,85 @@ +import React from 'react'; +import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n'; +import { useDispatch, useSelector } from 'react-redux'; +import { + ActionRow, + Button, + Icon, + ModalDialog, + IconButton, +} from '@openedx/paragon'; +import { Close } from '@openedx/paragon/icons'; + +import { EditorModalBody, EditorModalWrapper, FooterWrapper } from '../../../../EditorContainer'; +import { actions, selectors } from '../../../../../data/redux'; +import * as containerHooks from '../../../../EditorContainer/hooks'; +import * as hooks from '../hooks'; +import ecMessages from '../../../../EditorContainer/messages'; +import messages from './messages'; + +interface Props { + selected: string; + onClose: (() => void) | null; +} + +const SelectTypeWrapper: React.FC = ({ + children, + onClose = null, + selected, +}) => { + const handleCancel = containerHooks.handleCancel({ onClose }); + const intl = useIntl(); + const defaultSettings = useSelector(selectors.problem.defaultSettings); + const dispatch = useDispatch(); + const updateField = React.useCallback((data) => dispatch(actions.problem.updateField(data)), [dispatch]); + const setBlockTitle = React.useCallback((title) => dispatch(actions.app.setBlockTitle(title)), [dispatch]); + + return ( + + + + +
+ +
+ + + + {children} + + + + + + + + + + + + ); +}; + +export default SelectTypeWrapper; diff --git a/src/editors/containers/ProblemEditor/components/SelectTypeModal/index.test.tsx b/src/editors/containers/ProblemEditor/components/SelectTypeModal/index.test.tsx index 9be7578c5f..ce2fbd7037 100644 --- a/src/editors/containers/ProblemEditor/components/SelectTypeModal/index.test.tsx +++ b/src/editors/containers/ProblemEditor/components/SelectTypeModal/index.test.tsx @@ -6,6 +6,7 @@ import { initializeMocks, } from '../../../../../testUtils'; import editorStore from '../../../../data/store'; +import { EditorContextProvider } from '../../../../EditorContext'; import * as hooks from './hooks'; import SelectTypeModal from '.'; @@ -18,7 +19,13 @@ describe('SelectTypeModal', () => { const mockSelect = jest.fn(); jest.spyOn(hooks, 'onSelect').mockImplementation(mockSelect); // This is a new-style test, unlike most of the old snapshot-based editor tests. - render(); + render( + + + + + , + ); // First we see the menu of problem types: expect(await screen.findByRole('button', { name: 'Numerical input' })).toBeInTheDocument(); diff --git a/src/editors/data/services/cms/api.ts b/src/editors/data/services/cms/api.ts index 673309bb00..d40c9d5f36 100644 --- a/src/editors/data/services/cms/api.ts +++ b/src/editors/data/services/cms/api.ts @@ -19,6 +19,21 @@ interface AssetResponse { assets: Record[]; // In the raw response here, these are NOT camel-cased yet. } +type FieldsResponse = { + display_name: string; // In the raw response here, these are NOT camel-cased yet. + data: any; + metadata: Record; +} & Record; // In courses (but not in libraries), there are many other fields returned here. + +interface AncestorsResponse { + ancestors: { + id: string; + display_name: string; // In the raw response here, these are NOT camel-cased yet. + category: string; + has_children: boolean; + }[]; +} + export const loadImage = (imageData) => ({ ...imageData, dateAdded: new Date(imageData.dateAdded.replace(' at', '')).getTime(), @@ -89,10 +104,11 @@ export const processLicense = (licenseType, licenseDetails) => { }; export const apiMethods = { - fetchBlockById: ({ blockId, studioEndpointUrl }) => get( + fetchBlockById: ({ blockId, studioEndpointUrl }): Promise<{ data: FieldsResponse }> => get( urls.block({ blockId, studioEndpointUrl }), ), - fetchByUnitId: ({ blockId, studioEndpointUrl }) => get( + /** A better name for this would be 'get ancestors of block' */ + fetchByUnitId: ({ blockId, studioEndpointUrl }): Promise<{ data: AncestorsResponse }> => get( urls.blockAncestor({ studioEndpointUrl, blockId }), fetchByUnitIdOptions, ), diff --git a/src/editors/hooks.js b/src/editors/hooks.ts similarity index 77% rename from src/editors/hooks.js rename to src/editors/hooks.ts index c0d1b70a87..d2a7403d87 100644 --- a/src/editors/hooks.js +++ b/src/editors/hooks.ts @@ -4,11 +4,6 @@ import { sendTrackEvent } from '@edx/frontend-platform/analytics'; import analyticsEvt from './data/constants/analyticsEvt'; import { actions, thunkActions } from './data/redux'; -// 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'; import { RequestKeys } from './data/constants/requests'; // eslint-disable-next-line react-hooks/rules-of-hooks @@ -17,7 +12,7 @@ export const initializeApp = ({ dispatch, data }) => useEffect( [data], ); -export const navigateTo = (destination) => { +export const navigateTo = (destination: string | URL) => { window.location.assign(destination); }; @@ -34,7 +29,7 @@ export const navigateCallback = ({ returnFunction()(response); return; } - module.navigateTo(destination); + navigateTo(destination); }; export const nullMethod = () => ({}); @@ -61,7 +56,7 @@ export const saveBlock = ({ if (attemptSave) { dispatch(thunkActions.app.saveBlock( content, - module.navigateCallback({ + navigateCallback({ destination, analyticsEvent: analyticsEvt.editorSaveClick, analytics, diff --git a/src/editors/messages.js b/src/editors/messages.ts similarity index 100% rename from src/editors/messages.js rename to src/editors/messages.ts diff --git a/src/library-authoring/LibraryAuthoringPage.test.tsx b/src/library-authoring/LibraryAuthoringPage.test.tsx index 9fc69521a5..bd0d911ed5 100644 --- a/src/library-authoring/LibraryAuthoringPage.test.tsx +++ b/src/library-authoring/LibraryAuthoringPage.test.tsx @@ -80,6 +80,7 @@ describe('', () => { initializeMocks(); // The Meilisearch client-side API uses fetch, not Axios. + fetchMock.mockReset(); fetchMock.post(searchEndpoint, (_url, req) => { const requestData = JSON.parse(req.body?.toString() ?? ''); const query = requestData?.queries[0]?.q ?? ''; @@ -94,11 +95,6 @@ describe('', () => { }); }); - afterEach(() => { - jest.clearAllMocks(); - fetchMock.mockReset(); - }); - const renderLibraryPage = async () => { render(, { path, params: { libraryId: mockContentLibrary.libraryId } }); diff --git a/src/library-authoring/LibraryLayout.tsx b/src/library-authoring/LibraryLayout.tsx index 56718cc5ef..78d60674ae 100644 --- a/src/library-authoring/LibraryLayout.tsx +++ b/src/library-authoring/LibraryLayout.tsx @@ -1,70 +1,26 @@ -import React from 'react'; import { Route, Routes, - useNavigate, useParams, } from 'react-router-dom'; -import { PageWrap } from '@edx/frontend-platform/react'; -import { useQueryClient } from '@tanstack/react-query'; -import EditorContainer from '../editors/EditorContainer'; import LibraryAuthoringPage from './LibraryAuthoringPage'; import { LibraryProvider } from './common/context'; import { CreateCollectionModal } from './create-collection'; -import { invalidateComponentData } from './data/apiHooks'; import LibraryCollectionPage from './collections/LibraryCollectionPage'; +import { ComponentEditorModal } from './components/ComponentEditorModal'; const LibraryLayout = () => { const { libraryId } = useParams(); - const queryClient = useQueryClient(); if (libraryId === undefined) { // istanbul ignore next - This shouldn't be possible; it's just here to satisfy the type checker. throw new Error('Error: route is missing libraryId.'); } - const navigate = useNavigate(); - const goBack = React.useCallback((prevPath?: string) => { - if (prevPath) { - // Redirects back to the previous route like collection page or library page - navigate(prevPath); - } else { - // Go back to the library - navigate(`/library/${libraryId}`); - } - }, []); - - const returnFunction = React.useCallback((prevPath?: string) => { - // When changes are cancelled, either onClose (goBack) or this returnFunction will be called. - // When changes are saved, this returnFunction is called. - goBack(prevPath); - return (args) => { - if (args === undefined) { - return; // Do nothing - the user cancelled the changes - } - const { id: usageKey } = args; - // invalidate any queries that involve this XBlock: - invalidateComponentData(queryClient, libraryId, usageKey); - }; - }, [goBack]); - return ( - {/* - TODO: we should be opening this editor as a modal, not making it a separate page/URL. - That will be a much nicer UX because users can just close the modal and be on the same page they were already - on, instead of always getting sent back to the library home. - */} - - - - )} - /> } @@ -75,6 +31,7 @@ const LibraryLayout = () => { /> + ); }; diff --git a/src/library-authoring/add-content/AddContentContainer.tsx b/src/library-authoring/add-content/AddContentContainer.tsx index fe88ba0cdc..d37fd75f05 100644 --- a/src/library-authoring/add-content/AddContentContainer.tsx +++ b/src/library-authoring/add-content/AddContentContainer.tsx @@ -16,14 +16,14 @@ import { ContentPaste, } from '@openedx/paragon/icons'; import { v4 as uuid4 } from 'uuid'; -import { useLocation, useNavigate, useParams } from 'react-router-dom'; +import { useParams } from 'react-router-dom'; import { ToastContext } from '../../generic/toast-context'; import { useCopyToClipboard } from '../../generic/clipboard'; import { getCanEdit } from '../../course-unit/data/selectors'; import { useCreateLibraryBlock, useLibraryPasteClipboard, useUpdateCollectionComponents } from '../data/apiHooks'; -import { getEditUrl } from '../components/utils'; import { useLibraryContext } from '../common/context'; +import { canEditComponent } from '../components/ComponentEditorModal'; import messages from './messages'; @@ -61,9 +61,6 @@ const AddContentButton = ({ contentType, onCreateContent } : AddContentButtonPro const AddContentContainer = () => { const intl = useIntl(); - const navigate = useNavigate(); - const location = useLocation(); - const currentPath = location.pathname; const { libraryId, collectionId } = useParams(); const createBlockMutation = useCreateLibraryBlock(); const updateComponentsMutation = useUpdateCollectionComponents(libraryId, collectionId); @@ -73,6 +70,7 @@ const AddContentContainer = () => { const { showPasteXBlock } = useCopyToClipboard(canEdit); const { openCreateCollectionModal, + openComponentEditor, } = useLibraryContext(); const collectionButtonData = { @@ -151,14 +149,12 @@ const AddContentContainer = () => { blockType, definitionId: `${uuid4()}`, }).then((data) => { - const editUrl = getEditUrl(data.id); + const hasEditor = canEditComponent(data.id); updateComponentsMutation.mutateAsync([data.id]).catch(() => { showToast(intl.formatMessage(messages.errorAssociateComponentMessage)); }); - if (editUrl) { - // Pass currentPath in state so that we can come back to - // current page on save or cancel - navigate(editUrl, { state: { from: currentPath } }); + if (hasEditor) { + openComponentEditor(data.id); } else { // We can't start editing this right away so just show a toast message: showToast(intl.formatMessage(messages.successCreateMessage)); diff --git a/src/library-authoring/collections/CollectionDetails.test.tsx b/src/library-authoring/collections/CollectionDetails.test.tsx index 5c4e69064e..ad68c091d3 100644 --- a/src/library-authoring/collections/CollectionDetails.test.tsx +++ b/src/library-authoring/collections/CollectionDetails.test.tsx @@ -34,8 +34,6 @@ describe('', () => { }); afterEach(() => { - jest.clearAllMocks(); - axiosMock.restore(); fetchMock.mockReset(); }); diff --git a/src/library-authoring/collections/LibraryCollectionPage.test.tsx b/src/library-authoring/collections/LibraryCollectionPage.test.tsx index c1cfb9602f..b242601474 100644 --- a/src/library-authoring/collections/LibraryCollectionPage.test.tsx +++ b/src/library-authoring/collections/LibraryCollectionPage.test.tsx @@ -77,10 +77,6 @@ describe('', () => { }); }); - afterEach(() => { - jest.clearAllMocks(); - }); - const renderLibraryCollectionPage = async (collectionId?: string, libraryId?: string) => { const libId = libraryId || mockContentLibrary.libraryId; const colId = collectionId || mockCollection.collectionId; diff --git a/src/library-authoring/common/context.tsx b/src/library-authoring/common/context.tsx index 772eaf17fb..5c4b1938db 100644 --- a/src/library-authoring/common/context.tsx +++ b/src/library-authoring/common/context.tsx @@ -25,6 +25,11 @@ export interface LibraryContextData { // Current collection openCollectionInfoSidebar: (collectionId: string) => void; currentCollectionId?: string; + // Editor modal - for editing some component + /** If the editor is open and the user is editing some component, this is its usageKey */ + componentBeingEdited: string | undefined; + openComponentEditor: (usageKey: string) => void; + closeComponentEditor: () => void; } /** @@ -45,6 +50,8 @@ export const LibraryProvider = (props: { children?: React.ReactNode, libraryId: const [currentComponentUsageKey, setCurrentComponentUsageKey] = React.useState(); const [currentCollectionId, setcurrentCollectionId] = React.useState(); const [isCreateCollectionModalOpen, openCreateCollectionModal, closeCreateCollectionModal] = useToggle(false); + const [componentBeingEdited, openComponentEditor] = React.useState(); + const closeComponentEditor = React.useCallback(() => openComponentEditor(undefined), []); const resetSidebar = React.useCallback(() => { setCurrentComponentUsageKey(undefined); @@ -91,6 +98,9 @@ export const LibraryProvider = (props: { children?: React.ReactNode, libraryId: closeCreateCollectionModal, openCollectionInfoSidebar, currentCollectionId, + componentBeingEdited, + openComponentEditor, + closeComponentEditor, }), [ props.libraryId, sidebarBodyComponent, @@ -104,6 +114,9 @@ export const LibraryProvider = (props: { children?: React.ReactNode, libraryId: closeCreateCollectionModal, openCollectionInfoSidebar, currentCollectionId, + componentBeingEdited, + openComponentEditor, + closeComponentEditor, ]); return ( diff --git a/src/library-authoring/component-info/ComponentInfo.test.tsx b/src/library-authoring/component-info/ComponentInfo.test.tsx new file mode 100644 index 0000000000..f200777bc3 --- /dev/null +++ b/src/library-authoring/component-info/ComponentInfo.test.tsx @@ -0,0 +1,63 @@ +import { + initializeMocks, + render, + screen, + waitFor, +} from '../../testUtils'; +import { mockContentLibrary, mockLibraryBlockMetadata } from '../data/api.mocks'; +import { mockBroadcastChannel } from '../../generic/data/api.mock'; +import { LibraryProvider } from '../common/context'; +import ComponentInfo from './ComponentInfo'; + +mockBroadcastChannel(); +mockContentLibrary.applyMock(); +mockLibraryBlockMetadata.applyMock(); +jest.mock('./ComponentPreview', () => ({ + __esModule: true, // Required when mocking 'default' export + default: () =>
Mocked preview
, +})); +jest.mock('./ComponentManagement', () => ({ + __esModule: true, // Required when mocking 'default' export + default: () =>
Mocked management tab
, +})); + +const withLibraryId = (libraryId: string) => ({ + extraWrapper: ({ children }: { children: React.ReactNode }) => ( + {children} + ), +}); + +describe(' Sidebar', () => { + it('should show a disabled "Edit" button when the component type is not editable', async () => { + initializeMocks(); + render( + , + withLibraryId(mockContentLibrary.libraryId), + ); + + const editButton = await screen.findByRole('button', { name: /Edit component/ }); + expect(editButton).toBeDisabled(); + }); + + it('should show a disabled "Edit" button when the library is read-only', async () => { + initializeMocks(); + render( + , + withLibraryId(mockContentLibrary.libraryIdReadOnly), + ); + + const editButton = await screen.findByRole('button', { name: /Edit component/ }); + expect(editButton).toBeDisabled(); + }); + + it('should show a working "Edit" button for a normal component', async () => { + initializeMocks(); + render( + , + withLibraryId(mockContentLibrary.libraryId), + ); + + const editButton = await screen.findByRole('button', { name: /Edit component/ }); + await waitFor(() => expect(editButton).not.toBeDisabled()); + }); +}); diff --git a/src/library-authoring/component-info/ComponentInfo.tsx b/src/library-authoring/component-info/ComponentInfo.tsx index f503e4df99..19257b5de6 100644 --- a/src/library-authoring/component-info/ComponentInfo.tsx +++ b/src/library-authoring/component-info/ComponentInfo.tsx @@ -1,4 +1,3 @@ -import React from 'react'; import { useIntl } from '@edx/frontend-platform/i18n'; import { Button, @@ -6,14 +5,15 @@ import { Tabs, Stack, } from '@openedx/paragon'; -import { Link } from 'react-router-dom'; -import { getEditUrl } from '../components/utils'; import { ComponentMenu } from '../components'; import ComponentDetails from './ComponentDetails'; import ComponentManagement from './ComponentManagement'; import ComponentPreview from './ComponentPreview'; import messages from './messages'; +import { canEditComponent } from '../components/ComponentEditorModal'; +import { useLibraryContext } from '../common/context'; +import { useContentLibrary } from '../data/apiHooks'; interface ComponentInfoProps { usageKey: string; @@ -21,13 +21,15 @@ interface ComponentInfoProps { const ComponentInfo = ({ usageKey }: ComponentInfoProps) => { const intl = useIntl(); - const editUrl = getEditUrl(usageKey); + const { libraryId, openComponentEditor } = useLibraryContext(); + const { data: libraryData } = useContentLibrary(libraryId); + const canEdit = libraryData?.canEditLibrary && canEditComponent(usageKey); return (