Skip to content

Commit db07092

Browse files
jacobo-dominguez-wgujacobo-dominguez-wgu
authored
fix: various i18n issues
* fix: adding messages for i18n issues related to placeholders * fix: adding messages for i18n issues related to import tag wizard stepper titles * fix: changing name to duplicated id on i18n message * fix: replacing hardcoded string with constants to solve i18n issue * fix: typo on title prop * fix: adding components prop name correctly * test: adding ut for select video modal * chore: adding description to placeholder, changing extension to constant file and adding uts for code coverage * chore: removing outdated comment lines
1 parent 26c6a71 commit db07092

13 files changed

Lines changed: 257 additions & 63 deletions

File tree

‎src/accessibility-page/AccessibilityPage.jsx‎

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -9,30 +9,30 @@ import messages from './messages';
99
import AccessibilityBody from './AccessibilityBody';
1010
import AccessibilityForm from './AccessibilityForm';
1111

12+
import { COMMUNITY_ACCESSIBILITY_LINK, ACCESSIBILITY_EMAIL } from './constants';
13+
1214
const AccessibilityPage = ({
1315
// injected
1416
intl,
15-
}) => {
16-
const communityAccessibilityLink = 'https://www.edx.org/accessibility';
17-
const email = 'accessibility@edx.org';
18-
return (
19-
<>
20-
<Helmet>
21-
<title>
22-
{intl.formatMessage(messages.pageTitle, {
23-
siteName: process.env.SITE_NAME,
24-
})}
25-
</title>
26-
</Helmet>
27-
<Header isHiddenMainMenu />
28-
<Container size="xl" classNamae="px-4">
29-
<AccessibilityBody {...{ email, communityAccessibilityLink }} />
30-
<AccessibilityForm accessibilityEmail={email} />
31-
</Container>
32-
<StudioFooterSlot />
33-
</>
34-
);
35-
};
17+
}) => (
18+
<>
19+
<Helmet>
20+
<title>
21+
{intl.formatMessage(messages.pageTitle, {
22+
siteName: process.env.SITE_NAME,
23+
})}
24+
</title>
25+
</Helmet>
26+
<Header isHiddenMainMenu />
27+
<Container size="xl" classNamae="px-4">
28+
<AccessibilityBody
29+
{...{ email: ACCESSIBILITY_EMAIL, communityAccessibilityLink: COMMUNITY_ACCESSIBILITY_LINK }}
30+
/>
31+
<AccessibilityForm accessibilityEmail={ACCESSIBILITY_EMAIL} />
32+
</Container>
33+
<StudioFooterSlot />
34+
</>
35+
);
3636

3737
AccessibilityPage.propTypes = {
3838
// injected

‎src/accessibility-page/constants.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
export const COMMUNITY_ACCESSIBILITY_LINK = 'https://www.edx.org/accessibility';
2+
export const ACCESSIBILITY_EMAIL = 'accessibility@edx.org';

‎src/editors/containers/EditorContainer/components/TitleHeader/EditableHeader.jsx‎

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,29 +2,35 @@ import React from 'react';
22
import PropTypes from 'prop-types';
33

44
import { Form } from '@openedx/paragon';
5+
import { useIntl } from '@edx/frontend-platform/i18n';
56
import EditConfirmationButtons from './EditConfirmationButtons';
67

8+
import messages from './messages';
9+
710
const EditableHeader = ({
811
handleChange,
912
updateTitle,
1013
handleKeyDown,
1114
inputRef,
1215
localTitle,
1316
cancelEdit,
14-
}) => (
15-
<Form.Group onBlur={(e) => updateTitle(e)}>
16-
<Form.Control
17-
style={{ paddingInlineEnd: 'calc(1rem + 84px)' }}
18-
autoFocus
19-
trailingElement={<EditConfirmationButtons {...{ updateTitle, cancelEdit }} />}
20-
onChange={handleChange}
21-
onKeyDown={handleKeyDown}
22-
placeholder="Title"
23-
ref={inputRef}
24-
value={localTitle}
25-
/>
26-
</Form.Group>
27-
);
17+
}) => {
18+
const intl = useIntl();
19+
return (
20+
<Form.Group onBlur={(e) => updateTitle(e)}>
21+
<Form.Control
22+
style={{ paddingInlineEnd: 'calc(1rem + 84px)' }}
23+
autoFocus
24+
trailingElement={<EditConfirmationButtons {...{ updateTitle, cancelEdit }} />}
25+
onChange={handleChange}
26+
onKeyDown={handleKeyDown}
27+
placeholder={intl.formatMessage(messages.editTitlePlaceholder)}
28+
ref={inputRef}
29+
value={localTitle}
30+
/>
31+
</Form.Group>
32+
);
33+
};
2834
EditableHeader.defaultProps = {
2935
inputRef: null,
3036
};

‎src/editors/containers/EditorContainer/components/TitleHeader/messages.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,11 @@ const messages = defineMessages({
1717
defaultMessage: 'Edit Title',
1818
description: 'Screen reader label title for icon button to edit the xblock title',
1919
},
20+
editTitlePlaceholder: {
21+
id: 'authoring.texteditor.header.editTitleLabelPlaceholder',
22+
defaultMessage: 'Title',
23+
description: 'Screen reader label title for icon button to edit the xblock title',
24+
},
2025
cancelTitleEdit: {
2126
id: 'authoring.texteditor.header.cancelTitleEdit',
2227
defaultMessage: 'Cancel',

‎src/editors/containers/VideoEditor/components/SelectVideoModal.jsx‎

Lines changed: 17 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3,36 +3,33 @@ import { connect } from 'react-redux';
33
import PropTypes from 'prop-types';
44

55
import { Button } from '@openedx/paragon';
6+
import { useIntl } from '@edx/frontend-platform/i18n';
67

78
import { thunkActions } from '../../../data/redux';
89
import BaseModal from '../../../sharedComponents/BaseModal';
9-
// This 'module' self-import hack enables mocking during tests.
10-
// See src/editors/decisions/0005-internal-editor-testability-decisions.md. The whole approach to how hooks are tested
11-
// should be re-thought and cleaned up to avoid this pattern.
12-
// eslint-disable-next-line import/no-self-import
13-
import * as module from './SelectVideoModal';
10+
import messages from './messages';
1411

15-
export const hooks = {
16-
videoList: ({ fetchVideos }) => {
12+
export const useVideoList = ({ fetchVideos }) => {
1713
// eslint-disable-next-line react-hooks/rules-of-hooks
18-
const [videos, setVideos] = React.useState(null);
19-
// eslint-disable-next-line react-hooks/rules-of-hooks
20-
React.useEffect(() => {
21-
fetchVideos({ onSuccess: setVideos });
22-
}, []);
23-
return videos;
24-
},
25-
onSelectClick: ({ setSelection, videos }) => () => setSelection(videos[0]),
14+
const [videos, setVideos] = React.useState(null);
15+
// eslint-disable-next-line react-hooks/rules-of-hooks
16+
React.useEffect(() => {
17+
fetchVideos({ onSuccess: setVideos });
18+
}, []);
19+
return videos;
2620
};
2721

22+
export const useOnSelectClick = ({ setSelection, videos }) => () => setSelection(videos[0]);
23+
2824
export const SelectVideoModal = ({
2925
fetchVideos,
3026
isOpen,
3127
close,
3228
setSelection,
3329
}) => {
34-
const videos = module.hooks.videoList({ fetchVideos });
35-
const onSelectClick = module.hooks.onSelectClick({
30+
const intl = useIntl();
31+
const videos = useVideoList({ fetchVideos });
32+
const onSelectClick = useOnSelectClick({
3633
setSelection,
3734
videos,
3835
});
@@ -41,13 +38,13 @@ export const SelectVideoModal = ({
4138
<BaseModal
4239
isOpen={isOpen}
4340
close={close}
44-
title="Add a video"
41+
title={intl.formatMessage(messages.selectVideoModalTitle)}
4542
confirmAction={<Button variant="primary" onClick={onSelectClick}>Next</Button>}
4643
>
4744
{/* Content selection */}
4845
{videos && (videos.map(
4946
img => (
50-
<div key={img.externalUrl} />
47+
<div key={img.externalUrl}>{img.externalUrl}</div>
5148
),
5249
))}
5350
</BaseModal>
@@ -62,7 +59,7 @@ SelectVideoModal.propTypes = {
6259
fetchVideos: PropTypes.func.isRequired,
6360
};
6461

65-
export const mapStateToProps = () => ({});
62+
export const mapStateToProps = null;
6663
export const mapDispatchToProps = {
6764
fetchVideos: thunkActions.app.fetchVideos,
6865
};

‎src/editors/containers/VideoEditor/components/SelectVideoModal.test.jsx‎

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
import React from 'react';
2+
import {
3+
render, screen, fireEvent, waitFor,
4+
} from '@testing-library/react';
5+
import { IntlProvider } from '@edx/frontend-platform/i18n';
6+
import { Provider } from 'react-redux';
7+
import configureStore from 'redux-mock-store';
8+
import { SelectVideoModal, useVideoList, useOnSelectClick } from './SelectVideoModal';
9+
import messages from './messages';
10+
11+
describe('SelectVideoModal', () => {
12+
const mockStore = configureStore([]);
13+
const mockFetchVideos = jest.fn();
14+
const mockSetSelection = jest.fn();
15+
const mockClose = jest.fn();
16+
let store;
17+
18+
beforeEach(() => {
19+
store = mockStore({});
20+
jest.clearAllMocks();
21+
});
22+
23+
it('renders the modal with the correct title', () => {
24+
render(
25+
<Provider store={store}>
26+
<IntlProvider locale="en" messages={messages}>
27+
<SelectVideoModal
28+
isOpen
29+
close={mockClose}
30+
setSelection={mockSetSelection}
31+
fetchVideos={mockFetchVideos}
32+
/>
33+
</IntlProvider>
34+
</Provider>,
35+
);
36+
37+
expect(screen.getByText(messages.selectVideoModalTitle.defaultMessage)).toBeInTheDocument();
38+
});
39+
40+
it('calls close when the modal is closed', () => {
41+
render(
42+
<Provider store={store}>
43+
<IntlProvider locale="en" messages={messages}>
44+
<SelectVideoModal
45+
isOpen
46+
close={mockClose}
47+
setSelection={mockSetSelection}
48+
fetchVideos={mockFetchVideos}
49+
/>
50+
</IntlProvider>
51+
</Provider>,
52+
);
53+
54+
fireEvent.click(screen.getByRole('button', { name: /close/i }));
55+
expect(mockClose).toHaveBeenCalled();
56+
});
57+
58+
it('renders a div for each video in the videos array', () => {
59+
const videos = [
60+
{ externalUrl: 'video1.mp4' },
61+
{ externalUrl: 'video2.mp4' },
62+
];
63+
const fetchVideos = jest.fn(({ onSuccess }) => onSuccess(videos));
64+
65+
render(
66+
<Provider store={store}>
67+
<IntlProvider locale="en" messages={messages}>
68+
<SelectVideoModal
69+
isOpen
70+
close={jest.fn()}
71+
setSelection={jest.fn()}
72+
fetchVideos={fetchVideos}
73+
/>
74+
</IntlProvider>
75+
</Provider>,
76+
);
77+
78+
// Assert that a div is rendered for each video
79+
videos.forEach((video) => {
80+
expect(screen.getByText(video.externalUrl)).toBeInTheDocument();
81+
});
82+
});
83+
});
84+
85+
describe('SelectVideoModal hooks', () => {
86+
describe('useVideoList', () => {
87+
it('fetches videos and sets them in state', async () => {
88+
// eslint-disable-next-line react/prop-types
89+
const TestComponent = ({ fetchVideos }) => {
90+
const videos = useVideoList({ fetchVideos });
91+
return (
92+
<div data-testid="videos">
93+
{videos ? videos.map((v) => v.externalUrl).join(', ') : 'Loading'}
94+
</div>
95+
);
96+
};
97+
98+
const videos = [
99+
{ externalUrl: 'video1.mp4' },
100+
{ externalUrl: 'video2.mp4' },
101+
];
102+
const fetchVideos = jest.fn(({ onSuccess }) => {
103+
onSuccess(videos);
104+
});
105+
106+
render(<TestComponent fetchVideos={fetchVideos} />);
107+
108+
await waitFor(() => expect(screen.getByTestId('videos').textContent).toBe(videos.map((v) => v.externalUrl).join(', ')));
109+
110+
expect(fetchVideos).toHaveBeenCalled();
111+
});
112+
});
113+
114+
describe('useOnSelectClick', () => {
115+
it('calls setSelection with the first video', () => {
116+
const mockSetSelection = jest.fn();
117+
const videos = [
118+
{ externalUrl: 'video1.mp4' },
119+
{ externalUrl: 'video2.mp4' },
120+
];
121+
122+
const onSelectClick = useOnSelectClick({
123+
setSelection: mockSetSelection,
124+
videos,
125+
});
126+
127+
onSelectClick();
128+
expect(mockSetSelection).toHaveBeenCalledWith(videos[0]);
129+
});
130+
});
131+
});

‎src/editors/containers/VideoEditor/components/messages.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
import { defineMessages } from '@edx/frontend-platform/i18n';
2+
3+
const messages = defineMessages({
4+
selectVideoModalTitle: {
5+
id: 'authoring.videoEditor.selectVideoModal.title',
6+
defaultMessage: 'Add a video',
7+
description: 'Title for the select video modal.',
8+
},
9+
});
10+
11+
export default messages;

‎src/generic/configure-modal/AdvancedTab.jsx‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { Alert, Form, Hyperlink } from '@openedx/paragon';
55
import {
66
Warning as WarningIcon,
77
} from '@openedx/paragon/icons';
8-
import { FormattedMessage, injectIntl } from '@edx/frontend-platform/i18n';
8+
import { FormattedMessage, injectIntl, useIntl } from '@edx/frontend-platform/i18n';
99
import messages from './messages';
1010

1111
import PrereqSettings from './PrereqSettings';
@@ -32,6 +32,8 @@ const AdvancedTab = ({
3232
} = values;
3333
let examTypeValue = 'none';
3434

35+
const intl = useIntl();
36+
3537
if (isTimeLimited && isProctoredExam) {
3638
if (isOnboardingExam) {
3739
examTypeValue = 'onboardingExam';
@@ -183,7 +185,7 @@ const AdvancedTab = ({
183185
<Form.Control
184186
onChange={setCurrentTimeLimit}
185187
value={timeLimit}
186-
placeholder="HH:MM"
188+
placeholder={intl.formatMessage(messages.timeLimitPlaceholder)}
187189
pattern="^[0-9][0-9]:[0-5][0-9]$"
188190
/>
189191
</Form.Group>

‎src/generic/configure-modal/messages.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,11 @@ const messages = defineMessages({
239239
id: 'course-authoring.course-outline.configure-modal.advanced-tab.time-allotted',
240240
defaultMessage: 'Time allotted (HH:MM):',
241241
},
242+
timeLimitPlaceholder: {
243+
id: 'course-authoring.course-outline.configure-modal.advanced-tab.time-limit-placeholder',
244+
defaultMessage: 'HH:MM',
245+
description: 'The placeholder for the time limit input field, two digits for hours and two digits for minutes colons in between',
246+
},
242247
timeLimitDescription: {
243248
id: 'course-authoring.course-outline.configure-modal.advanced-tab.time-limit-description',
244249
defaultMessage: 'Select a time allotment for the exam. If it is over 24 hours, type in the amount of time. You can grant individual learners extra time to complete the exam through the Instructor Dashboard.',

0 commit comments

Comments
 (0)