From a4f0a8f16291251252bc02081f86c7c3da262070 Mon Sep 17 00:00:00 2001 From: Ken Clary Date: Thu, 20 Oct 2022 10:10:21 -0400 Subject: [PATCH] feat: duration widget for video settings. TNL-9823. --- .../components/DurationWidget.jsx | 63 ++++++++++++---- .../VideoSettingsModal/components/duration.js | 29 +++++-- .../components/duration.test.js | 75 +++++++++++++------ .../VideoSettingsModal/components/messages.js | 40 ++++++++++ src/editors/data/redux/thunkActions/video.js | 7 +- .../data/redux/thunkActions/video.test.js | 6 +- src/editors/data/services/cms/api.js | 5 +- src/editors/data/services/cms/api.test.js | 4 +- 8 files changed, 175 insertions(+), 54 deletions(-) diff --git a/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/DurationWidget.jsx b/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/DurationWidget.jsx index 5043b0d52..c5895a9ab 100644 --- a/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/DurationWidget.jsx +++ b/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/DurationWidget.jsx @@ -3,50 +3,85 @@ import { useDispatch } from 'react-redux'; // import PropTypes from 'prop-types'; import { + Col, FormControl, FormGroup, + Row, } from '@edx/paragon'; +import { injectIntl, intlShape, FormattedMessage } from '@edx/frontend-platform/i18n'; import { keyStore } from '../../../../../utils'; import CollapsibleFormWidget from './CollapsibleFormWidget'; import hooks from './hooks'; +import { durationFromValue } from './duration'; +import messages from './messages'; /** * Collapsible Form widget controlling video start and end times * Also displays the total run time of the video. */ -export const DurationWidget = () => { +export const DurationWidget = ({ + // injected + intl, +}) => { const dispatch = useDispatch(); const { duration } = hooks.widgetValues({ dispatch, fields: { [hooks.selectorKeys.duration]: hooks.durationWidget }, }); const timeKeys = keyStore(duration.formValue); + + const getTotalLabel = (startTime, stopTime) => { + if (!stopTime) { + if (!startTime) { + return intl.formatMessage(messages.fullVideoLength); + } + return intl.formatMessage(messages.startsAt, { startTime: durationFromValue(startTime) }); + } + const total = stopTime - (startTime || 0); + return intl.formatMessage(messages.total, { total: durationFromValue(total) }); + }; + return ( - - -
+ + + + + + + + + -
-
- Total: {duration.formValue.total} -
- + + + + + +
+ {getTotalLabel(duration.formValue.startTime, duration.formValue.stopTime)} +
); }; -export default DurationWidget; +DurationWidget.propTypes = { + // injected + intl: intlShape.isRequired, +}; + +export default injectIntl(DurationWidget); diff --git a/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/duration.js b/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/duration.js index b55bdf87a..9f6de85c1 100644 --- a/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/duration.js +++ b/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/duration.js @@ -10,6 +10,9 @@ const durationMatcher = /^(\d+)?:?(\d+)?:?(\d+)?$/i; * @return {string} - duration in 'hh:mm:ss' format */ export const durationFromValue = (value) => { + if (!value || typeof value !== 'number' || value <= 0) { + return '00:00:00'; + } const seconds = Math.floor((value / 1000) % 60); const minutes = Math.floor((value / 60000) % 60); const hours = Math.floor((value / 3600000) % 24); @@ -26,7 +29,7 @@ export const durationFromValue = (value) => { export const valueFromDuration = (duration) => { let matches = duration.trim().match(durationMatcher); if (!matches) { - return null; + return 0; } matches = matches.slice(1).filter(v => v !== undefined); if (matches.length < 3) { @@ -68,14 +71,24 @@ export const updateDuration = ({ setLocal, }) => useCallback( (index, durationString) => { - const newValue = module.valueFromDuration(durationString); - if (newValue !== null) { - setLocal({ ...local, [index]: durationString }); - setFormValue({ ...formValue, [index]: newValue }); - } else { - // If invalid duration string, reset to last valid value - setLocal({ ...local, [index]: module.durationFromValue(formValue[index]) }); + let newDurationString = durationString; + let newValue = module.valueFromDuration(newDurationString); + // stopTime must be at least 1 second, if not zero + if (index === 'stopTime' && newValue > 0 && newValue < 1000) { + newValue = 1000; } + // stopTime must be at least 1 second after startTime, except 0 means no custom stopTime + if (index === 'stopTime' && newValue > 0 && newValue < (formValue.startTime + 1000)) { + newValue = formValue.startTime + 1000; + } + // startTime must be at least 1 second before stopTime, except when stopTime is less than a second + // (stopTime should only be less than a second if it's zero, but we're being paranoid) + if (index === 'startTime' && formValue.stopTime >= 1000 && newValue > (formValue.stopTime - 1000)) { + newValue = formValue.stopTime - 1000; + } + newDurationString = module.durationFromValue(newValue); + setLocal({ ...local, [index]: newDurationString }); + setFormValue({ ...formValue, [index]: newValue }); }, [formValue, local, setLocal, setFormValue], ); diff --git a/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/duration.test.js b/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/duration.test.js index 13affb448..684e0608c 100644 --- a/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/duration.test.js +++ b/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/duration.test.js @@ -42,9 +42,16 @@ describe('Video Settings Modal duration hooks', () => { }); describe('durationFromValue', () => { + beforeEach(() => { + hook = duration.durationFromValue; + }); + it('returns 00:00:00 if given a bad value', () => { + const badChecks = ['a', '', null, -1]; + badChecks.forEach(val => expect(hook(val)).toEqual('00:00:00')); + }); it('translates milliseconds into hh:mm:ss format', () => { durationPairs.forEach( - ([val, dur]) => expect(duration.durationFromValue(val)).toEqual(dur), + ([val, dur]) => expect(hook(val)).toEqual(dur), ); }); }); @@ -52,9 +59,9 @@ describe('Video Settings Modal duration hooks', () => { beforeEach(() => { hook = duration.valueFromDuration; }); - it('returns null if given a bad duration string', () => { + it('returns 0 if given a bad duration string', () => { const badChecks = ['a', '00:00:1f', '0adg:00:04']; - badChecks.forEach(dur => expect(hook(dur)).toEqual(null)); + badChecks.forEach(dur => expect(hook(dur)).toEqual(0)); }); it('returns simple durations', () => { durationPairs.forEach(([val, dur]) => expect(hook(dur)).toEqual(val)); @@ -77,19 +84,18 @@ describe('Video Settings Modal duration hooks', () => { }); }); describe('updateDuration', () => { - const testDuration = 'myDuration'; - const testIndex = 'startTime'; - const mockValueFromDuration = (dur) => ({ value: dur }); - const mockDurationFromValue = (value) => ({ duration: value }); + const testValidIndex = 'startTime'; + const testStopIndex = 'stopTime'; + const testValidDuration = '00:00:00'; + const testValidValue = 0; + const testInvalidDuration = 'abc'; beforeEach(() => { props = { - formValue: { startTime: 230000, stopTime: 0 }, - local: { startTime: '00:00:23', stopTime: '00:00:00' }, + formValue: { startTime: 23000, stopTime: 600000 }, + local: { startTime: '00:00:23', stopTime: '00:10:00' }, setLocal: jest.fn(), setFormValue: jest.fn(), }; - spies.valueFromDuration = jest.spyOn(duration, durationKeys.valueFromDuration) - .mockImplementation(mockValueFromDuration); hook = duration.updateDuration; ({ cb, prereqs } = hook(props).useCallback); }); @@ -104,29 +110,54 @@ describe('Video Settings Modal duration hooks', () => { describe('callback', () => { describe('if the passed durationString is valid', () => { it('sets the local value to updated strings and form value to new timestamp value', () => { - cb(testIndex, testDuration); - expect(duration.valueFromDuration).toHaveBeenCalledWith(testDuration); + cb(testValidIndex, testValidDuration); expect(props.setLocal).toHaveBeenCalledWith({ ...props.local, - [testIndex]: testDuration, + [testValidIndex]: testValidDuration, }); expect(props.setFormValue).toHaveBeenCalledWith({ ...props.formValue, - [testIndex]: mockValueFromDuration(testDuration), + [testValidIndex]: testValidValue, }); }); }); describe('if the passed durationString is not valid', () => { - it('updates local back to the string for the form-stored timestamp value', () => { - spies.valueFromDuration.mockReturnValue(null); - spies.durationFromValue = jest.spyOn(duration, durationKeys.durationFromValue) - .mockImplementationOnce(mockDurationFromValue); - hook(props).useCallback.cb(testIndex, testDuration); + it('updates local values to 0 (the default)', () => { + hook(props).useCallback.cb(testValidIndex, testInvalidDuration); expect(props.setLocal).toHaveBeenCalledWith({ ...props.local, - [testIndex]: mockDurationFromValue(props.formValue[testIndex]), + [testValidIndex]: testValidDuration, + }); + expect(props.setFormValue).toHaveBeenCalledWith({ + ...props.formValue, + [testValidIndex]: testValidValue, + }); + }); + }); + describe('if the passed startTime is after (or equal to) the stored non-zero stopTime', () => { + it('updates local startTime values to 1 second before stopTime', () => { + hook(props).useCallback.cb(testValidIndex, '00:10:00'); + expect(props.setLocal).toHaveBeenCalledWith({ + ...props.local, + [testValidIndex]: '00:09:59', + }); + expect(props.setFormValue).toHaveBeenCalledWith({ + ...props.formValue, + [testValidIndex]: 599000, + }); + }); + }); + describe('if the passed stopTime is before (or equal to) the stored startTime', () => { + it('updates local stopTime values to 1 second after startTime', () => { + hook(props).useCallback.cb(testStopIndex, '00:00:22'); + expect(props.setLocal).toHaveBeenCalledWith({ + ...props.local, + [testStopIndex]: '00:00:24', + }); + expect(props.setFormValue).toHaveBeenCalledWith({ + ...props.formValue, + [testStopIndex]: 24000, }); - expect(props.setFormValue).not.toHaveBeenCalled(); }); }); }); diff --git a/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/messages.js b/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/messages.js index a4e00f572..727012ea0 100644 --- a/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/messages.js +++ b/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/messages.js @@ -17,6 +17,46 @@ export const messages = { defaultMessage: 'Please check your entries and try again.', description: 'Body of validation error.', }, + durationTitle: { + id: 'authoring.videoeditor.duration.title', + defaultMessage: 'Duration', + description: 'Title of Duration widget', + }, + durationDescription: { + id: 'authoring.videoeditor.duration.description', + defaultMessage: 'Set a specific section of the video to play.', + description: 'Description of Duration widget', + }, + startTimeLabel: { + id: 'authoring.videoeditor.duration.startTime.label', + defaultMessage: 'Start time', + description: 'Label of start time input field', + }, + stopTimeLabel: { + id: 'authoring.videoeditor.duration.stopTime.label', + defaultMessage: 'Stop time', + description: 'Label of stop time input field', + }, + durationHint: { + id: 'authoring.videoeditor.duration.hint', + defaultMessage: 'Enter time as HH:MM:SS', + description: 'Hint text for start and stop time input fields', + }, + fullVideoLength: { + id: 'authoring.videoeditor.duration.fullVideoLength', + defaultMessage: 'Full video length', + description: 'Text describing a video with neither custom start time nor custom stop time', + }, + startsAt: { + id: 'authoring.videoeditor.duration.startsAt', + defaultMessage: 'Starts at {startTime}', + description: 'Text describing a video with custom start time and default stop time', + }, + total: { + id: 'authoring.videoeditor.duration.total', + defaultMessage: 'Total: {total}', + description: 'Text describing a video with custom start time and custom stop time, or just a custom stop time', + }, }; export default messages; diff --git a/src/editors/data/redux/thunkActions/video.js b/src/editors/data/redux/thunkActions/video.js index 55f557953..9de6194e5 100644 --- a/src/editors/data/redux/thunkActions/video.js +++ b/src/editors/data/redux/thunkActions/video.js @@ -1,6 +1,7 @@ import { actions, selectors } from '..'; import * as requests from './requests'; import * as module from './video'; +import { valueFromDuration } from '../../../containers/VideoEditor/components/VideoSettingsModal/components/duration'; export const loadVideoData = () => (dispatch, getState) => { const state = getState(); @@ -32,9 +33,9 @@ export const loadVideoData = () => (dispatch, getState) => { allowTranscriptDownloads: rawVideoData.download_track, showTranscriptByDefault: rawVideoData.show_captions, duration: { // TODO duration is not always sent so they should be calculated. - startTime: rawVideoData.start_time, - stopTime: rawVideoData.end_time, - total: null, // TODO can we get total duration? if not, probably dropping from widget + startTime: valueFromDuration(rawVideoData.start_time || '00:00:00'), + stopTime: valueFromDuration(rawVideoData.end_time || '00:00:00'), + total: 0, // TODO can we get total duration? if not, probably dropping from widget }, handout: rawVideoData.handout, licenseType, diff --git a/src/editors/data/redux/thunkActions/video.test.js b/src/editors/data/redux/thunkActions/video.test.js index ad9371048..6a1206152 100644 --- a/src/editors/data/redux/thunkActions/video.test.js +++ b/src/editors/data/redux/thunkActions/video.test.js @@ -40,12 +40,12 @@ const testMetadata = { download_track: 'dOWNlOAdTraCK', download_video: 'downLoaDViDEo', edx_video_id: 'soMEvIDEo', - end_time: 'StOpTIMe', + end_time: 0, handout: 'hANdoUT', html5_sources: [], license: 'liCENse', show_captions: 'shOWcapTIONS', - start_time: 'stARtTiME', + start_time: 0, transcripts: { la: 'test VALUE' }, thumbnail: 'thuMBNaIl', }; @@ -120,7 +120,7 @@ describe('video thunkActions', () => { duration: { startTime: testMetadata.start_time, stopTime: testMetadata.end_time, - total: null, + total: 0, }, handout: testMetadata.handout, licenseType: 'liCENSEtyPe', diff --git a/src/editors/data/services/cms/api.js b/src/editors/data/services/cms/api.js index c40e6db3e..9a7f2faea 100644 --- a/src/editors/data/services/cms/api.js +++ b/src/editors/data/services/cms/api.js @@ -3,6 +3,7 @@ import * as urls from './urls'; import { get, post, deleteObject } from './utils'; import * as module from './api'; import * as mockApi from './mockApi'; +import { durationFromValue } from '../../../containers/VideoEditor/components/VideoSettingsModal/components/duration'; export const apiMethods = { fetchBlockById: ({ blockId, studioEndpointUrl }) => get( @@ -121,8 +122,8 @@ export const apiMethods = { track: '', // TODO Downloadable Transcript URL. Backend expects a file name, for example: "something.srt" show_captions: content.showTranscriptByDefault, handout: content.handout, - start_time: content.duration.startTime, - end_time: content.duration.stopTime, + start_time: durationFromValue(content.duration.startTime), + end_time: durationFromValue(content.duration.stopTime), license: module.processLicense(content.licenseType, content.licenseDetails), }, }; diff --git a/src/editors/data/services/cms/api.test.js b/src/editors/data/services/cms/api.test.js index 49e449eac..1a94b1c3f 100644 --- a/src/editors/data/services/cms/api.test.js +++ b/src/editors/data/services/cms/api.test.js @@ -97,8 +97,8 @@ describe('cms api', () => { transcripts: 'traNScRiPts', allowTranscriptDownloads: 'aLloWTRaNScriPtdoWnlOADS', duration: { - startTime: 'StArTTime', - stopTime: 'sToPTiME', + startTime: '00:00:00', + stopTime: '00:00:00', }, showTranscriptByDefault: 'ShOWtrANscriPTBYDeFAulT', handout: 'HAnDOuT',