fix: propTypes warnings in Problem Editor, refactor some code to TS (#…

…1280)

* fix: a11y - missing 'alt' text for Problem Editor IconButton
* fix: warning in <ProblemTypeSelect> component - missing key prop in list
* fix: warning: `problemType` required in `ProblemEditor`, but is `null`
* fix: warning: The prop `onClose` marked as required in `SelectTypeModal`
* fix: warning: prop `name` is marked as required in `ForwardRef(_c)`
* fix: warning: props `alt`, `id`, and `key` are required
* test: improve test coverage of SelectTypeModal, refactor some code
* test: improve test coverage

src/custom.d.ts

@@ -3,6 +3,11 @@ declare module '*.svg' {
   export default content;
 }
 
+declare module '*.png' {
+  const content: string;
+  export default content;
+}
+
 declare module '*.json' {
   const value: any;
   export default value;

src/editors/Editor.jsx → src/editors/Editor.tsx

@@ -1,21 +1,29 @@
 import React from 'react';
 import { useDispatch } from 'react-redux';
-import PropTypes from 'prop-types';
 import { FormattedMessage } from '@edx/frontend-platform/i18n';
 
 import messages from './messages';
 import * as hooks from './hooks';
 
 import supportedEditors from './supportedEditors';
+import type { EditorComponent } from './EditorComponent';
 
-const Editor = ({
+export interface Props extends EditorComponent {
+  blockType: string;
+  blockId: string | null;
+  learningContextId: string | null;
+  lmsEndpointUrl: string | null;
+  studioEndpointUrl: string | null;
+}
+
+const Editor: React.FC<Props> = ({
   learningContextId,
   blockType,
   blockId,
   lmsEndpointUrl,
   studioEndpointUrl,
-  onClose,
-  returnFunction,
+  onClose = null,
+  returnFunction = null,
 }) => {
   const dispatch = useDispatch();
   hooks.initializeApp({
@@ -46,23 +54,5 @@ const Editor = ({
     </div>
   );
 };
-Editor.defaultProps = {
-  blockId: null,
-  learningContextId: null,
-  lmsEndpointUrl: null,
-  onClose: null,
-  returnFunction: null,
-  studioEndpointUrl: null,
-};
-
-Editor.propTypes = {
-  blockId: PropTypes.string,
-  blockType: PropTypes.string.isRequired,
-  learningContextId: PropTypes.string,
-  lmsEndpointUrl: PropTypes.string,
-  onClose: PropTypes.func,
-  returnFunction: PropTypes.func,
-  studioEndpointUrl: PropTypes.string,
-};
 
 export default Editor;

src/editors/EditorComponent.ts

@@ -0,0 +1,6 @@
+/** Shared interface that all Editor components (like ProblemEditor) adhere to */
+export interface EditorComponent {
+  onClose: (() => void) | null;
+  // TODO: get a better type for the 'result' here
+  returnFunction?: (() => (result: any) => void) | null;
+}

src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/SelectTypeFooter.jsx

@@ -1,13 +1,12 @@
 import React from 'react';
 import PropTypes from 'prop-types';
-import { connect } from 'react-redux';
-
+import { useDispatch, useSelector } from 'react-redux';
+import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n';
 import {
   ActionRow,
   Button,
   ModalDialog,
 } from '@openedx/paragon';
-import { FormattedMessage, injectIntl, intlShape } from '@edx/frontend-platform/i18n';
 import messages from './messages';
 import * as hooks from '../hooks';
 
@@ -16,68 +15,49 @@ import { actions, selectors } from '../../../../../data/redux';
 const SelectTypeFooter = ({
   onCancel,
   selected,
-  // redux
-  defaultSettings,
-  updateField,
-  setBlockTitle,
-  // injected,
-  intl,
-}) => (
-  <div className="editor-footer fixed-bottom">
-    <ModalDialog.Footer className="border-top-0">
-      <ActionRow>
-        <ActionRow.Spacer />
-        <Button
-          aria-label={intl.formatMessage(messages.cancelButtonAriaLabel)}
-          variant="tertiary"
-          onClick={onCancel}
-        >
-          <FormattedMessage {...messages.cancelButtonLabel} />
-        </Button>
-        <Button
-          aria-label={intl.formatMessage(messages.selectButtonAriaLabel)}
-          onClick={hooks.onSelect({
-            selected,
-            updateField,
-            setBlockTitle,
-            defaultSettings,
-          })}
-          disabled={!selected}
-        >
-          <FormattedMessage {...messages.selectButtonLabel} />
-        </Button>
-      </ActionRow>
-    </ModalDialog.Footer>
-  </div>
-);
+}) => {
+  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 (
+    <div className="editor-footer fixed-bottom">
+      <ModalDialog.Footer className="border-top-0">
+        <ActionRow>
+          <ActionRow.Spacer />
+          <Button
+            aria-label={intl.formatMessage(messages.cancelButtonAriaLabel)}
+            variant="tertiary"
+            onClick={onCancel}
+          >
+            <FormattedMessage {...messages.cancelButtonLabel} />
+          </Button>
+          <Button
+            aria-label={intl.formatMessage(messages.selectButtonAriaLabel)}
+            onClick={hooks.onSelect({
+              selected,
+              updateField,
+              setBlockTitle,
+              defaultSettings,
+            })}
+            disabled={!selected}
+          >
+            <FormattedMessage {...messages.selectButtonLabel} />
+          </Button>
+        </ActionRow>
+      </ModalDialog.Footer>
+    </div>
+  );
+};
 
 SelectTypeFooter.defaultProps = {
   selected: null,
 };
 
 SelectTypeFooter.propTypes = {
-  defaultSettings: PropTypes.shape({
-    maxAttempts: PropTypes.number,
-    rerandomize: PropTypes.string,
-    showResetButton: PropTypes.bool,
-    showanswer: PropTypes.string,
-  }).isRequired,
   onCancel: PropTypes.func.isRequired,
   selected: PropTypes.string,
-  updateField: PropTypes.func.isRequired,
-  setBlockTitle: PropTypes.func.isRequired,
-  // injected
-  intl: intlShape.isRequired,
-};
-
-export const mapStateToProps = (state) => ({
-  defaultSettings: selectors.problem.defaultSettings(state),
-});
-
-export const mapDispatchToProps = {
-  updateField: actions.problem.updateField,
-  setBlockTitle: actions.app.setBlockTitle,
 };
 
-export const SelectTypeFooterInternal = SelectTypeFooter; // For testing only
-export default injectIntl(connect(mapStateToProps, mapDispatchToProps)(SelectTypeFooter));
+export default SelectTypeFooter;

src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/SelectTypeFooter.test.jsx

@@ -1,14 +1,10 @@
 import 'CourseAuthoring/editors/setupEditorTest';
-import React from 'react';
 import { shallow } from '@edx/react-unit-test-utils';
 
 import { Button } from '@openedx/paragon';
 import { formatMessage } from '../../../../../testUtils';
-import * as module from './SelectTypeFooter';
+import SelectTypeFooter from './SelectTypeFooter';
 import * as hooks from '../hooks';
-import { actions } from '../../../../../data/redux';
-
-const SelectTypeFooter = module.SelectTypeFooterInternal;
 
 jest.mock('../hooks', () => ({
   onSelect: jest.fn().mockName('onSelect'),
@@ -46,15 +42,4 @@ describe('SelectTypeFooter', () => {
         .toEqual(expected);
     });
   });
-
-  describe('mapStateToProps', () => {
-    test('is empty', () => {
-      expect(module.mapDispatchToProps.defaultSettings).toEqual(actions.problem.defaultSettings);
-    });
-  });
-  describe('mapDispatchToProps', () => {
-    test('loads updateField from problem.updateField actions', () => {
-      expect(module.mapDispatchToProps.updateField).toEqual(actions.problem.updateField);
-    });
-  });
 });

src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/__snapshots__/index.test.jsx.snap

@@ -17,6 +17,7 @@ exports[`SelectTypeWrapper snapshot 1`] = `
         className="pgn__modal-close-container"
       >
         <IconButton
+          alt="Exit the editor"
           iconAs="Icon"
           src={[MockFunction icons.Close]}
         />
@@ -30,7 +31,7 @@ exports[`SelectTypeWrapper snapshot 1`] = `
       test child
     </h1>
   </ModalDialog.Body>
-  <injectIntl(ShimmedIntlComponent)
+  <SelectTypeFooter
     selected="iMAsElecTedValUE"
   />
 </div>

src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/index.jsx

@@ -1,11 +1,12 @@
 import React from 'react';
 import PropTypes from 'prop-types';
-import { injectIntl, FormattedMessage } from '@edx/frontend-platform/i18n';
+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 = ({
@@ -14,6 +15,7 @@ const SelectTypeWrapper = ({
   selected,
 }) => {
   const handleCancel = hooks.handleCancel({ onClose });
+  const intl = useIntl();
 
   return (
     <div
@@ -27,6 +29,7 @@ const SelectTypeWrapper = ({
               src={Close}
               iconAs={Icon}
               onClick={handleCancel}
+              alt={intl.formatMessage(ecMessages.exitButtonAlt)}
             />
           </div>
         </ModalDialog.Title>
@@ -51,5 +54,4 @@ SelectTypeWrapper.propTypes = {
   onClose: PropTypes.func,
 };
 
-export const SelectTypeWrapperInternal = SelectTypeWrapper; // For testing only
-export default injectIntl(SelectTypeWrapper);
+export default SelectTypeWrapper;

src/editors/containers/ProblemEditor/components/SelectTypeModal/SelectTypeWrapper/index.test.jsx

@@ -2,7 +2,7 @@ import 'CourseAuthoring/editors/setupEditorTest';
 import React from 'react';
 import { shallow } from '@edx/react-unit-test-utils';
 import { IconButton } from '@openedx/paragon';
-import { SelectTypeWrapperInternal as SelectTypeWrapper } from '.';
+import SelectTypeWrapper from '.';
 import { handleCancel } from '../../../../EditorContainer/hooks';
 
 jest.mock('../../../../EditorContainer/hooks', () => ({

src/editors/containers/ProblemEditor/components/SelectTypeModal/content/AdvanceTypeSelect.test.jsx → src/editors/containers/ProblemEditor/components/SelectTypeModal/content/AdvanceTypeSelect.test.tsx

@@ -1,16 +1,11 @@
 import 'CourseAuthoring/editors/setupEditorTest';
-import React from 'react';
 import { shallow } from '@edx/react-unit-test-utils';
 
-import { formatMessage } from '../../../../../testUtils';
-import * as module from './AdvanceTypeSelect';
-
-const AdvanceTypeSelect = module.AdvanceTypeSelectInternal;
+import AdvanceTypeSelect from './AdvanceTypeSelect';
 
 describe('AdvanceTypeSelect', () => {
   const props = {
-    intl: { formatMessage },
-    selected: 'blankadvanced',
+    selected: 'blankadvanced' as const,
     setSelected: jest.fn().mockName('setSelect'),
   };
   describe('snapshots', () => {
@@ -29,11 +24,6 @@ describe('AdvanceTypeSelect', () => {
         shallow(<AdvanceTypeSelect {...props} selected="customgrader" />).snapshot,
       ).toMatchSnapshot();
     });
-    test('snapshots: renders as expected with problemType is drag_and_drop', () => {
-      expect(
-        shallow(<AdvanceTypeSelect {...props} selected="drag_and_drop" />).snapshot,
-      ).toMatchSnapshot();
-    });
     test('snapshots: renders as expected with problemType is formularesponse', () => {
       expect(
         shallow(<AdvanceTypeSelect {...props} selected="formularesponse" />).snapshot,
@@ -44,14 +34,14 @@ describe('AdvanceTypeSelect', () => {
         shallow(<AdvanceTypeSelect {...props} selected="imageresponse" />).snapshot,
       ).toMatchSnapshot();
     });
-    test('snapshots: renders as expected with problemType is jsinput_response', () => {
+    test('snapshots: renders as expected with problemType is jsinputresponse', () => {
       expect(
-        shallow(<AdvanceTypeSelect {...props} selected="jsinput_response" />).snapshot,
+        shallow(<AdvanceTypeSelect {...props} selected="jsinputresponse" />).snapshot,
       ).toMatchSnapshot();
     });
-    test('snapshots: renders as expected with problemType is problem_with_hint', () => {
+    test('snapshots: renders as expected with problemType is problemwithhint', () => {
       expect(
-        shallow(<AdvanceTypeSelect {...props} selected="problem_with_hint" />).snapshot,
+        shallow(<AdvanceTypeSelect {...props} selected="problemwithhint" />).snapshot,
       ).toMatchSnapshot();
     });
   });