Skip to content

Commit dd3cdaf

Browse files
committed
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.
1 parent a5f342b commit dd3cdaf

11 files changed

Lines changed: 402 additions & 1182 deletions

File tree

src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/settingsComponents/SwitchEditorCard.jsx

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import React from 'react';
22
import { useDispatch } from 'react-redux';
3-
import { FormattedMessage } from '@edx/frontend-platform/i18n';
3+
import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n';
44
import { Card } from '@openedx/paragon';
55
import PropTypes from 'prop-types';
66
import { thunkActions } from '@src/editors/data/redux';
@@ -15,6 +15,7 @@ const SwitchEditorCard = ({
1515
editorType,
1616
problemType,
1717
}) => {
18+
const intl = useIntl();
1819
const [isConfirmOpen, setConfirmOpen] = React.useState(false);
1920
const dispatch = useDispatch();
2021
const { editorRef } = useProblemEditorContext();
@@ -24,23 +25,20 @@ const SwitchEditorCard = ({
2425
<Card className="border border-light-700 shadow-none">
2526
<BaseModal
2627
isOpen={isConfirmOpen}
27-
close={() => {
28-
setConfirmOpen(false);
29-
}}
30-
title={<FormattedMessage {...messages[`ConfirmSwitchMessageTitle-${editorType}`]} />}
31-
confirmAction={
28+
close={() => { setConfirmOpen(false); }}
29+
title={intl.formatMessage(messages[`ConfirmSwitchMessageTitle-${editorType}`])}
30+
confirmAction={(
3231
<Button
3332
/* istanbul ignore next */
34-
onClick={() =>
35-
handleConfirmEditorSwitch({
36-
switchEditor: () => dispatch(thunkActions.problem.switchEditor(editorType, editorRef)),
37-
setConfirmOpen,
38-
})}
33+
onClick={() => handleConfirmEditorSwitch({
34+
switchEditor: () => dispatch(thunkActions.problem.switchEditor(editorType, editorRef)),
35+
setConfirmOpen,
36+
})}
3937
variant="primary"
4038
>
4139
<FormattedMessage {...messages[`ConfirmSwitchButtonLabel-${editorType}`]} />
4240
</Button>
43-
}
41+
)}
4442
size="md"
4543
>
4644
<FormattedMessage {...messages[`ConfirmSwitchMessage-${editorType}`]} />
@@ -49,9 +47,7 @@ const SwitchEditorCard = ({
4947
className="my-3 ml-2 py-0"
5048
variant="link"
5149
size="sm"
52-
onClick={() => {
53-
setConfirmOpen(true);
54-
}}
50+
onClick={() => { setConfirmOpen(true); }}
5551
>
5652
<FormattedMessage {...messages[`SwitchButtonLabel-${editorType}`]} />
5753
</Button>
Lines changed: 21 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import React from 'react';
2-
import PropTypes from 'prop-types';
32

43
import {
54
ActionRow,
@@ -10,20 +9,35 @@ import { FormattedMessage } from '@edx/frontend-platform/i18n';
109

1110
import messages from './messages';
1211

12+
interface BaseModalProps {
13+
isOpen: boolean;
14+
close: () => void;
15+
title: string;
16+
children: React.ReactNode;
17+
confirmAction: React.ReactNode;
18+
footerAction?: React.ReactNode;
19+
headerComponent?: React.ReactNode;
20+
size?: 'sm' | 'md' | 'lg' | 'xl' | 'fullscreen';
21+
isFullscreenScroll?: boolean;
22+
bodyStyle?: React.CSSProperties;
23+
className?: string;
24+
hideCancelButton?: boolean;
25+
}
26+
1327
const BaseModal = ({
1428
isOpen,
1529
close,
1630
title,
1731
children,
1832
headerComponent,
1933
confirmAction,
20-
footerAction,
21-
size,
22-
isFullscreenScroll,
34+
footerAction = null,
35+
size = 'lg',
36+
isFullscreenScroll = true,
2337
bodyStyle,
2438
className,
25-
hideCancelButton,
26-
}) => (
39+
hideCancelButton = false,
40+
}: BaseModalProps) => (
2741
<ModalDialog
2842
isOpen={isOpen}
2943
onClose={close}
@@ -34,6 +48,7 @@ const BaseModal = ({
3448
isFullscreenScroll={isFullscreenScroll}
3549
title={title}
3650
className={className}
51+
isOverflowVisible={false}
3752
>
3853
<ModalDialog.Header style={{ zIndex: 1, boxShadow: '2px 2px 5px rgba(0, 0, 0, 0.3)' }}>
3954
<ModalDialog.Title>
@@ -65,29 +80,4 @@ const BaseModal = ({
6580
</ModalDialog>
6681
);
6782

68-
BaseModal.defaultProps = {
69-
footerAction: null,
70-
headerComponent: null,
71-
size: 'lg',
72-
isFullscreenScroll: true,
73-
bodyStyle: null,
74-
className: undefined,
75-
hideCancelButton: false,
76-
};
77-
78-
BaseModal.propTypes = {
79-
isOpen: PropTypes.bool.isRequired,
80-
close: PropTypes.func.isRequired,
81-
title: PropTypes.node.isRequired,
82-
children: PropTypes.node.isRequired,
83-
confirmAction: PropTypes.node.isRequired,
84-
footerAction: PropTypes.node,
85-
headerComponent: PropTypes.node,
86-
size: PropTypes.string,
87-
isFullscreenScroll: PropTypes.bool,
88-
bodyStyle: PropTypes.shape({}),
89-
className: PropTypes.string,
90-
hideCancelButton: PropTypes.bool,
91-
};
92-
9383
export default BaseModal;

src/editors/sharedComponents/ImageUploadModal/ImageSettingsModal/AltTextControls.test.tsx

Lines changed: 0 additions & 66 deletions
This file was deleted.
Lines changed: 37 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -1,73 +1,52 @@
1+
import { useFormikContext } from 'formik';
12
import React from 'react';
2-
import PropTypes from 'prop-types';
33

44
import { Form } from '@openedx/paragon';
5-
import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n';
5+
import { useIntl } from '@edx/frontend-platform/i18n';
6+
import { ImageConfig } from './types';
67

7-
import * as hooks from './hooks';
88
import messages from './messages';
99

10-
/**
11-
* Wrapper for alt-text input and isDecorative checkbox control
12-
* @param {obj} errorProps - props for error handling
13-
* {bool} isValid - are alt-text fields valid for saving?
14-
* @param {bool} isDecorative - is the image decorative?
15-
* @param {func} setIsDecorative - handle isDecorative change event
16-
* @param {func} setValue - update alt-text value
17-
* @param {string} value - current alt-text value
18-
*/
19-
const AltTextControls = ({
20-
isDecorative,
21-
setIsDecorative,
22-
setValue,
23-
validation,
24-
value,
25-
}) => {
10+
const AltTextControls = () => {
2611
const intl = useIntl();
12+
const formik = useFormikContext<ImageConfig>();
2713
return (
28-
<Form.Group className="mt-4.5">
29-
<Form.Label as="h4">
30-
<FormattedMessage {...messages.accessibilityLabel} />
31-
</Form.Label>
32-
<Form.Control
33-
className="mt-4.5"
34-
disabled={isDecorative}
35-
floatingLabel={intl.formatMessage(messages.altTextFloatingLabel)}
36-
isInvalid={validation.show}
37-
onChange={hooks.onInputChange(setValue)}
38-
type="input"
39-
value={value}
40-
/>
41-
{validation.show
42-
&& (
43-
<Form.Control.Feedback type="invalid">
44-
<FormattedMessage {...messages.altTextLocalFeedback} />
45-
</Form.Control.Feedback>
46-
)}
47-
<Form.Checkbox
48-
checked={isDecorative}
49-
className="mt-4.5 decorative-control-label"
50-
onChange={hooks.onCheckboxChange(setIsDecorative)}
51-
>
14+
<>
15+
<Form.Group className="mt-1.5">
5216
<Form.Label>
53-
<FormattedMessage {...messages.decorativeAltTextCheckboxLabel} />
17+
{intl.formatMessage(messages.accessibilityLabel)}
5418
</Form.Label>
55-
</Form.Checkbox>
56-
</Form.Group>
19+
<Form.Control
20+
name="altText"
21+
className="mt-1.5"
22+
floatingLabel={intl.formatMessage(messages.altTextFloatingLabel)}
23+
disabled={formik.values.isDecorative}
24+
isInvalid={Boolean(formik.errors.altText)}
25+
onChange={formik.handleChange}
26+
type="input"
27+
value={formik.values.altText}
28+
/>
29+
{formik.errors.altText && (
30+
<Form.Control.Feedback type="invalid">
31+
{formik.errors.altText}
32+
</Form.Control.Feedback>
33+
)}
34+
</Form.Group>
35+
<Form.Group>
36+
<Form.Checkbox
37+
name="isDecorative"
38+
checked={formik.values.isDecorative}
39+
className="mt-2.5 decorative-control-label"
40+
onChange={formik.handleChange}
41+
>
42+
<Form.Label>
43+
{intl.formatMessage(messages.decorativeAltTextCheckboxLabel)}
44+
</Form.Label>
45+
</Form.Checkbox>
46+
</Form.Group>
47+
</>
5748
);
5849
};
59-
AltTextControls.propTypes = {
60-
error: PropTypes.shape({
61-
show: PropTypes.bool,
62-
}).isRequired,
63-
isDecorative: PropTypes.bool.isRequired,
64-
setValue: PropTypes.func.isRequired,
65-
setIsDecorative: PropTypes.func.isRequired,
66-
validation: PropTypes.shape({
67-
show: PropTypes.bool,
68-
}).isRequired,
69-
value: PropTypes.string.isRequired,
70-
};
7150

7251
export const AltTextControlsInternal = AltTextControls; // For testing only
7352
export default AltTextControls;

0 commit comments

Comments
 (0)