-
Notifications
You must be signed in to change notification settings - Fork 4.3k
[BD-14] [WIP] [SE-2936] Modernize Video Block editor for Blockstore runtime. #25125
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| module.exports = { | ||
| extends: 'eslint-config-edx', | ||
| root: true, | ||
| settings: { | ||
| 'import/resolver': { | ||
| webpack: { | ||
| config: 'webpack.dev.config.js', | ||
| }, | ||
| }, | ||
| }, | ||
| rules: { | ||
| 'import/prefer-default-export': 'off', | ||
| }, | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,220 @@ | ||
| import React from 'react'; | ||
| import PropTypes from 'prop-types'; | ||
| import { fetchSettings, submitSettings, emit } from '../data/thunks'; | ||
| import { connect } from "react-redux"; | ||
| import Tabs from "@edx/paragon/src/Tabs"; | ||
| // // According to the docs for the version of Paragon installed, this should import versions of the components with | ||
| // // style names that are scoped to paragon. This is true, but... | ||
| // import {InputText, Button, CheckBox} from "@edx/paragon/static"; | ||
| // // ...The following import line fails from what appears to be a path issue. | ||
| // import "@edx/paragon/static/paragon.min.css" | ||
| // // So, we're falling back the base component imports. | ||
| import {InputText, Button, CheckBox} from "@edx/paragon/src"; | ||
| import { VideoListContainer } from "./VideoList"; | ||
| import {SettingsShape} from "../data/shapes"; | ||
|
|
||
|
|
||
| export const VideoBlockEditor = ({changes, updateForm, saveSettings, errors}) => { | ||
| return ( | ||
| <div className="px-3"> | ||
| <Tabs labels={['Basic', 'Advanced']}> | ||
| <div className="row"> | ||
| <div className="col col-12"> | ||
| <InputText | ||
| value={changes.display_name} | ||
| name="display_name" | ||
| isValid={!errors.display_name} | ||
| validationMessage={errors.display_name} | ||
| description={gettext('The display name for this component.')} | ||
| label={gettext('Component Display Name')} | ||
| onChange={updateForm('display_name')} | ||
| /> | ||
| </div> | ||
| <div className="col col-12"> | ||
| { /* need to figure out way of handling error messages here. */} | ||
| <VideoListContainer | ||
| name="video_url" | ||
| label={gettext('Default Video URL')} | ||
| value={changes.video_url} | ||
| update={updateForm('video_url')} | ||
| /> | ||
| </div> | ||
| <div className="col col-12"> | ||
| <InputText | ||
| value={changes.edx_video_id} | ||
| name="video_id" | ||
| label={gettext('Video ID')} | ||
| isValid={!errors.video_id} | ||
| description={gettext('If you were assigned a Video ID by edX for the video to play in this component, enter the ID here. In this case, do not enter values in the Default Video URL, the Video File URLs, and the YouTube ID fields. If you were not assigned a Video ID, enter values in those other fields and ignore this field.')} | ||
| onChange={updateForm('video_id')} | ||
| /> | ||
| </div> | ||
| </div> | ||
| <div className="row"> | ||
| <div className="col col-12"> | ||
| <InputText | ||
| value={changes.display_name} | ||
| name="display_name" | ||
| isValid={!errors.display_name} | ||
| validationMessage={errors.display_name} | ||
| description={gettext('The display name for this component.')} | ||
| label={gettext('Component Display Name')} | ||
| onChange={updateForm('display_name')} | ||
| /> | ||
| </div> | ||
| <div className="col col-12"> | ||
| <CheckBox | ||
| checked={changes.only_on_web} | ||
| name="only_on_web" | ||
| label={gettext('Video Available on Web Only')} | ||
| isValid={!errors.only_on_web} | ||
| validationMessage={errors.only_on_web} | ||
| description={gettext('Specify whether access to this video is limited to browsers only, or if it can be accessed from other applications including mobile apps.')} | ||
| onChange={updateForm('only_on_web')} | ||
| /> | ||
| </div> | ||
| <div className="col col-12"> | ||
| <CheckBox | ||
| checked={changes.download_track} | ||
| name="download_track" | ||
| label={gettext('Download Transcript Allowed')} | ||
| isValid={!errors.download_track} | ||
| validationMessage={errors.download_track} | ||
| description={gettext('Allow students to download the timed transcript. A link to download the file appears below the video. By default, the transcript is an .srt or .txt file. If you want to provide the transcript for download in a different format, upload a file by using the Upload Handout field.')} | ||
| onChange={updateForm('download_track')} | ||
| /> | ||
| </div> | ||
| <div className="col col-12"> | ||
| <CheckBox | ||
| checked={changes.download_video} | ||
| name="download_video" | ||
| label={gettext('Video Download Allowed')} | ||
| isValid={!errors.download_video} | ||
| validationMessage={errors.download_video} | ||
| description={gettext('Allow students to download versions of this video in different formats if they cannot use the edX video player or do not have access to YouTube. You must add at least one non-YouTube URL in the Video File URLs field.')} | ||
| onChange={updateForm('download_video')} | ||
| /> | ||
| </div> | ||
| <div className="col col-12"> | ||
| <InputText | ||
| value={changes.track} | ||
| name="track" | ||
| isValid={!errors.track} | ||
| validationMessage={errors.track} | ||
| description={gettext('By default, students can download an .srt or .txt transcript when you set Download Transcript Allowed to True. If you want to provide a downloadable transcript in a different format, we recommend that you upload a handout by using the Upload a Handout field. If this isn\'t possible, you can post a transcript file on the Files & Uploads page or on the Internet, and then add the URL for the transcript here. Students see a link to download that transcript below the video.')} | ||
| label={gettext('Downloadable Transcript URL')} | ||
| onChange={updateForm('track')} | ||
| /> | ||
| </div> | ||
| <div className="col col-12"> | ||
| <InputText | ||
| value={changes.edx_video_id} | ||
| name="edx_video_id" | ||
| label={gettext('Video ID')} | ||
| isValid={!errors.edx_video_id} | ||
| validationMessage={errors.edx_video_id} | ||
| description={gettext('If you were assigned a Video ID by edX for the video to play in this component, enter the ID here. In this case, do not enter values in the Default Video URL, the Video File URLs, and the YouTube ID fields. If you were not assigned a Video ID, enter values in those other fields and ignore this field.')} | ||
| onChange={updateForm('edx_video_id')} | ||
| /> | ||
| </div> | ||
| <div className="col col-12"> | ||
| <InputText | ||
| value={changes.start_time} | ||
| name="start_time" | ||
| isValid={!errors.start_time} | ||
| validationMessage={errors.start_time} | ||
| label={gettext('Start Time')} | ||
| description={errors.start_time || gettext('Time you want the video to start if you don\'t want the entire video to play. Not supported in the native mobile app: the full video file will play. Formatted as HH:MM:SS. The maximum value is 23:59:59.')} | ||
| onChange={updateForm('start_time')} | ||
| /> | ||
| </div> | ||
| <div className="col col-12"> | ||
| <InputText | ||
| value={changes.end_time} | ||
| name="end_time" | ||
| label={gettext('End Time')} | ||
| isValid={!errors.end_time} | ||
| validationMessage={errors.end_time} | ||
| description={gettext('Time you want the video to stop if you don\'t want the entire video to play. Not supported in the native mobile app: the full video file will play. Formatted as HH:MM:SS. The maximum value is 23:59:59.')} | ||
| onChange={updateForm('end_time')} | ||
| /> | ||
| </div> | ||
| </div> | ||
| </Tabs> | ||
| <div className="xblock-actions"> | ||
| <ul> | ||
| <li> | ||
| <Button label="Save" onClick={saveSettings} /> | ||
| </li> | ||
| </ul> | ||
| </div> | ||
| </div> | ||
| ) | ||
| }; | ||
|
|
||
|
|
||
| export const VideoBlockEditorContainerBase = ({ | ||
| fetchSettings, submitSettings, settings, xblockElement, runtime, emit, changes, errors, | ||
| }) => { | ||
| if (settings === null) { | ||
| fetchSettings({runtime, xblockElement}).catch((error) => { | ||
| if (runtime.notify) { | ||
| runtime.notify("error", {message: error + ''}) | ||
| } else { | ||
| throw error | ||
| } | ||
| }) | ||
| return '' | ||
| } | ||
| if (changes === null) { | ||
| // Can happen briefly after settings are first set in the store but before copied into the changes field. | ||
| return '' | ||
| } | ||
| const updateForm = (fieldName) => (newValue) => { | ||
| if (errors[fieldName]) { | ||
| const newErrors = {...errors} | ||
| delete newErrors[fieldName] | ||
| emit('setErrors', {errors: newErrors}) | ||
| } | ||
| emit('updateChanges', {changes: {...changes, [fieldName]: newValue}}) | ||
| } | ||
| const saveSettings = () => { | ||
| emit('setErrors', {errors: {}}) | ||
| submitSettings({runtime, xblockElement, changes}).then((revised) => { | ||
| emit('updateSettings', {settings: revised}) | ||
| emit('updateChanges', {changes: revised}) | ||
| }).catch(() => undefined) | ||
| } | ||
| const fieldErrors = {} | ||
| // Usability standards suggest only showing one error message per field at a time. | ||
| // If there are multiple, just show one for now. | ||
| Object.keys(errors).map((key) => fieldErrors[key] = errors[key][0]) | ||
| return <VideoBlockEditor changes={changes} updateForm={updateForm} saveSettings={saveSettings} errors={fieldErrors} /> | ||
| }; | ||
|
|
||
| VideoBlockEditorContainerBase.propDefaults = { | ||
| changes: null, | ||
| settings: null, | ||
| } | ||
|
|
||
| VideoBlockEditorContainerBase.propTypes = { | ||
| fetchSettings: PropTypes.func.isRequired, | ||
| submitSettings: PropTypes.func.isRequired, | ||
| xblockElement: PropTypes.element.isRequired, | ||
| runtime: PropTypes.shape({ | ||
| notify: PropTypes.func, | ||
| }), | ||
| emit: PropTypes.func.isRequired, | ||
| changes: SettingsShape, | ||
| settings: SettingsShape, | ||
| } | ||
|
|
||
| export const VideoBlockEditorContainer = connect( | ||
| (state) => ({settings: state.settings, changes: state.changes, errors: state.errors}), | ||
| { | ||
| fetchSettings, | ||
| submitSettings, | ||
| emit, | ||
| }, | ||
| )(VideoBlockEditorContainerBase) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,91 @@ | ||
| import React, {useState} from "react"; | ||
| import PropTypes from 'prop-types'; | ||
| import {Button, InputText} from "@edx/paragon/src"; | ||
|
|
||
| /** | ||
| * VideoList template. See VideoListContainer below. | ||
| */ | ||
| export const VideoList = ({name, label, value, setExpanded, expanded, updatePosition}) => { | ||
| return ( | ||
| <React.Fragment> | ||
| <InputText | ||
| name={name} | ||
| label={label} | ||
| value={value[0]} | ||
| onChange={updatePosition(0)} | ||
| description={gettext('The URL for your video. This can be a YouTube URL or a link to an .mp4, .ogg, or .webm video file hosted elsewhere on the Internet.')} | ||
| /> | ||
| <div> | ||
| <Button onClick={() => setExpanded(!expanded)} label={gettext('Add backup URLs')} /> | ||
| </div> | ||
| {expanded && ( | ||
| <div> | ||
| <p> | ||
| {gettext('To be sure all students can access the video, we recommend providing both an .mp4 and a .webm version of your video. Click below to add a URL for another version. These URLs cannot be YouTube URLs. The first listed video that\'s compatible with the student\'s computer will play.')} | ||
| </p> | ||
| <div> | ||
| <InputText | ||
| name={`${name}_1`} | ||
| label="" | ||
| value={value[1]} | ||
| onChange={updatePosition(1)} | ||
| /> | ||
| </div> | ||
| <div> | ||
| <InputText | ||
| name={`${name}_2`} | ||
| label="" | ||
| value={value[2]} | ||
| onChange={updatePosition(2)} | ||
| /> | ||
| </div> | ||
| </div> | ||
| )} | ||
| </React.Fragment> | ||
| ) | ||
| } | ||
|
|
||
| VideoList.propTypes = { | ||
| name: PropTypes.string.isRequired, | ||
| label: PropTypes.string.isRequired, | ||
| value: PropTypes.arrayOf(PropTypes.string).isRequired, | ||
| updatePosition: PropTypes.func.isRequired, | ||
| expanded: PropTypes.bool.isRequired, | ||
| setExpanded: PropTypes.func.isRequired, | ||
| } | ||
|
|
||
| /** | ||
| * VideoListContainer | ||
| * This function is a react component that handles the 'video_url' field, which, despite its name, is array of video | ||
| * urls, not just one. It's also dynamically constructed by the backend based on the set YoutubeID and the backup | ||
| * HTML5 sources. | ||
| * | ||
| * This component isn't complete-- it matches the functionality of the 'Basic' tab in studio, but either a different | ||
| * component needs to be made to handle the 'Advanced' tab's functionality or else this component needs to be | ||
| * refactored/split up. | ||
| */ | ||
| export const VideoListContainer = ({name, label, value, update}) => { | ||
| const updatePosition = (position) => (value) => { | ||
| const revised = [...value] | ||
| revised[position] = value; | ||
| update(revised); | ||
| } | ||
| const [expanded, setExpanded] = useState(false) | ||
| return ( | ||
| <VideoList | ||
| expanded={expanded} | ||
| setExpanded={setExpanded} | ||
| updatePosition={updatePosition} | ||
| name={name} | ||
| label={label} | ||
| value={value} | ||
| /> | ||
| ) | ||
| } | ||
|
|
||
| VideoListContainer.propTypes = { | ||
| name: PropTypes.string.isRequired, | ||
| label: PropTypes.string.isRequired, | ||
| value: PropTypes.arrayOf(PropTypes.string).isRequired, | ||
| update: PropTypes.func.isRequired, | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| import Cookies from "js-cookie"; | ||
|
|
||
| const HEADERS = { | ||
| Accept: 'application/json', | ||
| 'Content-Type': 'application/json', | ||
| 'X-CSRFToken': Cookies.get('csrftoken'), | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. XBlock handlers are exempt from CSRF and exempt from CORS ; they use another method for security. As a result, you shouldn't need the Note that if the handler throws an exception, it may return a response that doesn't include the CORS exempt header, so you can sometimes see CORS errors which are really just masking some other error - check the LMS logs to debug. |
||
| }; | ||
|
|
||
| export const getSettings = async ({ runtime, xblockElement }) => ( | ||
| fetch( | ||
| runtime.handlerUrl(xblockElement, 'load_settings'), | ||
| {credentials: 'same-origin', method: 'get', headers: HEADERS}, | ||
| ).then((response) => response.json()) | ||
| ) | ||
|
|
||
| export const postSettings = async ({ runtime, xblockElement, changes }) => ( | ||
| fetch( | ||
| runtime.handlerUrl(xblockElement, 'save_settings'), | ||
| {credentials: 'same-origin', method: 'post', headers: HEADERS, body: JSON.stringify(changes)}, | ||
| ).then( | ||
| async (response) => { | ||
| if ((response.status >= 300) || (response.status < 200)) { | ||
| const err = Error('API error.') | ||
| err.data = ( | ||
| (await response.json()) || {'detail': gettext('We had trouble saving this block. Please try again later.')} | ||
| ) | ||
| throw err | ||
| } | ||
| return await response.json() | ||
| }, | ||
| ) | ||
| ) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| export const updateSettings = (state, {payload}) => { | ||
| return {...state, settings: payload.settings} | ||
| }; | ||
|
|
||
| export const updateChanges = (state, {payload}) => { | ||
| return {...state, changes: payload.changes} | ||
| } | ||
|
|
||
| export const setErrors = (state, {payload}) => { | ||
| return {...state, errors: payload.errors} | ||
| } | ||
|
|
||
| export const genReducer = (reducingFunctions) => (state, action) => { | ||
| if (!action) { | ||
| return state | ||
| } | ||
| if (reducingFunctions[action.type]) { | ||
| state = reducingFunctions[action.type](state, action) | ||
| } | ||
| return state | ||
| }; | ||
|
|
||
| export const rootReducer = genReducer({updateSettings, updateChanges, setErrors}); |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be good to put in an ADR, or inline in the code but the new runtime avoids using VAL and explicitly won't use this
edx_video_idfield: https://github.com/edx/edx-platform/blob/657da20c6e75f2c2e6a8c6c6d3833f5aa45deb0e/common/lib/xmodule/xmodule/video_module/video_module.py#L625-L626See openedx-unsupported/blockstore#62 for context.
Basically: in the new runtime, videos and transcripts should be stored in blockstore as static asset files linked to the video block, rather than in VAL. To simplify transcript handling,
.srtis the only supported format.However, we may eventually need to add some backwards compatible support for this field, so perhaps just
comment it out with this explanation ormake it read-only rather than removing it completely.