Skip to content

Commit

Permalink
♻️(lib-video) refacto LanguageSelect timed text track
Browse files Browse the repository at this point in the history
We had some states flipping with the LanguageSelect component
with "no language available" option. The no language available option
was selectable, and would create an error in the upload process
if selected.
We remove the "no language available" option, and let the Cunningham
component handle the case when no options are available.
  • Loading branch information
AntoLC committed Sep 14, 2023
1 parent f44f591 commit 363bd08
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 103 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ describe('<LanguageSelect />', () => {
expect(within(button).getByText('French')).toBeInTheDocument();
});

it('renders the component with instructor local language unavailable', async () => {
it('propagates correctly undefined when the local language is unavailable', async () => {
render(
<LanguageSelect
onChange={onChangeMock}
Expand All @@ -64,16 +64,14 @@ describe('<LanguageSelect />', () => {
);

await waitFor(() =>
expect(onChangeMock).toHaveBeenLastCalledWith({
label: 'English',
value: 'en',
}),
expect(onChangeMock).toHaveBeenLastCalledWith(undefined),
);

const button = await screen.findByRole('combobox', {
name: 'Choose the language',
});
expect(within(button).getByText('English')).toBeInTheDocument();
expect(
await screen.findByRole('combobox', {
name: 'Choose the language',
}),
).toBeInTheDocument();
});

it('renders the component with some languages already having some subtitles uploaded', async () => {
Expand Down Expand Up @@ -120,41 +118,6 @@ describe('<LanguageSelect />', () => {
).not.toBeInTheDocument();
});

it('renders the component with no languages', async () => {
render(
<LanguageSelect
onChange={onChangeMock}
timedTextModeWidget={timedTextMode.SUBTITLE}
/>,
{ intlOptions: { locale: 'fr-FR' } },
);

await waitFor(() =>
expect(onChangeMock).toHaveBeenLastCalledWith({
label: 'No language availables',
value: 'error',
}),
);

expect(
screen.queryByRole('button', {
name: 'Select the language for which you want to upload a timed text file; Selected: fr',
}),
).not.toBeInTheDocument();
expect(
screen.queryByRole('textbox', {
name: 'Select the language for which you want to upload a timed text file, fr',
}),
).not.toBeInTheDocument();

const button = await screen.findByRole('combobox', {
name: 'Choose the language',
});
expect(
within(button).getByText('No language availables'),
).toBeInTheDocument();
});

it('changes the selected language', async () => {
render(
<LanguageSelect
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
import { Select } from '@openfun/cunningham-react';
import { timedTextMode, useTimedTextTrack } from 'lib-components';
import { useEffect, useMemo, useState } from 'react';
import { Maybe } from 'lib-common';
import {
TimedTextTrackState,
timedTextMode,
useTimedTextTrack,
} from 'lib-components';
import { useCallback, useEffect, useMemo, useState } from 'react';
import { defineMessages, useIntl } from 'react-intl';

import { LanguageChoice } from '@lib-video/types/SelectOptions';
Expand All @@ -19,16 +24,10 @@ const messages = defineMessages({
'The text under the select used for choosing the language for which the user wants to upload a file.',
id: 'components.LanguageSelect.selectLanguageInfo',
},
noLanguageAvailableLabel: {
defaultMessage: 'No language availables',
description:
'The label displayed in the select when there is no available language.',
id: 'components.LanguageSelect.noLanguageAvailableLabel',
},
});

interface LanguageSelectProps {
onChange: (selection: LanguageChoice) => void;
onChange: (selection?: LanguageChoice) => void;
timedTextModeWidget: timedTextMode;
choices?: LanguageChoice[];
}
Expand All @@ -40,17 +39,14 @@ export const LanguageSelect = ({
}: LanguageSelectProps) => {
const intl = useIntl();

const errorLanguageChoice = useMemo(
() => ({
label: intl.formatMessage(messages.noLanguageAvailableLabel),
value: 'error',
const timedTextTrackFn = useCallback(
(state: TimedTextTrackState) => ({
timedTextTracks: state.getTimedTextTracks(),
}),
[intl],
[],
);

const { timedTextTracks } = useTimedTextTrack((state) => ({
timedTextTracks: state.getTimedTextTracks(),
}));
const { timedTextTracks } = useTimedTextTrack(timedTextTrackFn);

const availableSelectableLanguages = useMemo(() => {
const filteredTimedTextTracks = timedTextTracks.filter(
Expand All @@ -61,59 +57,42 @@ export const LanguageSelect = ({
);

return (
choices &&
choices
.filter((lang) => !excludedLanguages.includes(lang.value))
.sort((a, b) => a.label.localeCompare(b.label))
?.filter((lang) => !excludedLanguages.includes(lang.value))
.sort((a, b) => a.label.localeCompare(b.label)) || []
);
}, [choices, timedTextTracks, timedTextModeWidget]);

const userLocalAvailableLanguage = useMemo(() => {
const userLocalLanguage = intl.locale;

return availableSelectableLanguages
? availableSelectableLanguages.find((availableLanguage) =>
userLocalLanguage.startsWith(availableLanguage.value),
)
: errorLanguageChoice;
}, [availableSelectableLanguages, intl, errorLanguageChoice]);

const [selectedLanguage, setSelectedLanguage] = useState<{
label: string;
value: string;
}>(
userLocalAvailableLanguage ??
(availableSelectableLanguages
? availableSelectableLanguages[0]
: errorLanguageChoice),
const [selectedLanguage, setSelectedLanguage] = useState<Maybe<string>>(
availableSelectableLanguages?.[0]?.value,
);

useEffect(() => {
onChange(selectedLanguage);
}, [onChange, selectedLanguage]);

if (
availableSelectableLanguages &&
availableSelectableLanguages.length > 0 &&
!availableSelectableLanguages.includes(selectedLanguage)
) {
setSelectedLanguage(
userLocalAvailableLanguage ?? availableSelectableLanguages[0],
const userLocalAvailableLanguage = availableSelectableLanguages?.find(
(availableLanguage) => intl.locale.startsWith(availableLanguage.value),
);
}

setSelectedLanguage(userLocalAvailableLanguage?.value);
onChange(userLocalAvailableLanguage);
}, [intl.locale, availableSelectableLanguages, onChange]);

return (
<Select
aria-label={intl.formatMessage(messages.selectLanguageLabel)}
label={intl.formatMessage(messages.selectLanguageLabel)}
options={availableSelectableLanguages ?? [errorLanguageChoice]}
value={selectedLanguage.value}
options={availableSelectableLanguages}
value={selectedLanguage}
onChange={(evt) => {
setSelectedLanguage(
availableSelectableLanguages?.find(
(lang) => lang.value === evt.target.value,
) ?? errorLanguageChoice,
if (selectedLanguage === evt.target.value) {
return;
}

const optionSelectedLanguage = availableSelectableLanguages?.find(
(lang) => lang.value === evt.target.value,
);

setSelectedLanguage(optionSelectedLanguage?.value);
onChange(optionSelectedLanguage);
}}
fullWidth
clearable={false}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { Box, Button } from 'grommet';
import { Nullable } from 'lib-common';
import { Maybe, Nullable } from 'lib-common';
import {
ItemList,
TimedTextTrackState,
formatSizeErrorScale,
modelName,
report,
Expand Down Expand Up @@ -61,9 +62,15 @@ export const LocalizedTimedTextTrackUpload = ({
const intl = useIntl();
const video = useCurrentVideo();
const { addUpload, resetUpload, uploadManagerState } = useUploadManager();
const timedTextTracks = useTimedTextTrack((state) =>
state.getTimedTextTracks(),

const timedTextTrackFn = useCallback(
(state: TimedTextTrackState) => ({
timedTextTracks: state.getTimedTextTracks(),
}),
[],
);

const { timedTextTracks } = useTimedTextTrack(timedTextTrackFn);
const filteredTimedTextTracks = timedTextTracks.filter(
(track) => track.mode === timedTextModeWidget,
);
Expand Down Expand Up @@ -160,7 +167,7 @@ export const LocalizedTimedTextTrackUpload = ({
]);

const onChangeLanguageSelect = useCallback(
(option: LanguageChoice) => setSelectedLanguage(option),
(option: Maybe<LanguageChoice>) => setSelectedLanguage(option || null),
[],
);

Expand Down Expand Up @@ -213,6 +220,7 @@ export const LocalizedTimedTextTrackUpload = ({
primary
style={{ height: '50px', fontFamily: 'Roboto-Medium' }}
title={intl.formatMessage(messages.uploadButtonLabel)}
disabled={!selectedLanguage}
/>
</Box>
);
Expand Down

0 comments on commit 363bd08

Please sign in to comment.