Skip to content
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.

Sign up for GitHub

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

Add Video Tracks Support to VideoPress Block #21578

Merged
merged 18 commits into from
Nov 24, 2021
Merged

Add Video Tracks Support to VideoPress Block #21578

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
7291421
Initial commit of new videopress tracks management UI.
roundhill Oct 28, 2021
66d0e38
Adding changelog.
roundhill Oct 28, 2021
d73e357
Added support for apiFetch calls from wp.com.
roundhill Oct 29, 2021
90b4d43
Remove duplicate apiFetch call
roundhill Oct 29, 2021
57714b6
[not verified] Merge branch 'master' into add/videopress-video-tracks
roundhill Oct 29, 2021
3003771
Removed `getFilename` since we don't need to use the `src` property i…
roundhill Nov 1, 2021
eefd566
Use `isLink` prop instead of variant to support older Gutenbergs.
roundhill Nov 1, 2021
5cc245d
Use `isSecondary` prop for Save button.
roundhill Nov 2, 2021
84ff4f6
Added check for existing track, and show an error if so.
roundhill Nov 3, 2021
fdceff2
[not verified] Merge branch 'master' into add/videopress-video-tracks
roundhill Nov 3, 2021
129e285
Import the base gutenberg styles so we don't have to have duplicated …
roundhill Nov 3, 2021
0e27b4b
Remove autofocus, and moved save button click to an extracted functio…
roundhill Nov 3, 2021
9849bd0
* Added help label to file upload that explains which files are allowed.
roundhill Nov 3, 2021
c0883c5
[not verified] Merge branch 'master' into add/videopress-video-tracks
roundhill Nov 10, 2021
bb30f90
Changed `setTracks` to use a Promise instead of async/await.
roundhill Nov 12, 2021
09bf973
Merge branch 'master' into add/videopress-video-tracks
roundhill Nov 15, 2021
340028f
Merge branch 'master' into add/videopress-video-tracks
roundhill Nov 23, 2021
62149e9
Merge remote-tracking branch 'origin/master' into add/videopress-vide…
jeherve Nov 24, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions projects/plugins/jetpack/changelog/add-videopress-video-tracks
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: minor
Type: enhancement

Added captions/subtitles support to the VideoPress block.
55 changes: 54 additions & 1 deletion projects/plugins/jetpack/extensions/blocks/videopress/edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import Loading from './loading';
import { getVideoPressUrl } from './url';
import { getClassNames } from './utils';
import SeekbarColorSettings from './seekbar-color-settings';
import TracksEditor from './tracks-editor';

const VIDEO_POSTER_ALLOWED_MEDIA_TYPES = [ 'image' ];

Expand Down Expand Up @@ -80,6 +81,8 @@ const VideoPressEdit = CoreVideoEdit =>
const { guid } = this.props.attributes;
if ( ! guid ) {
await this.setGuid();
Copy link
Contributor

@pgk pgk Nov 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a chance that this call will throw? Should we wrap in a try/catch?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From looking at setGuid, I think it doesn't throw, but it does fall back to the core block if something goes wrong. So I think safe to leave as-is.

} else {
this.setTracks( guid );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like setTracks always gets called inside setGuid, is this second call still necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so, as I'd like it to still fetch the tracks even if the guid is in the attributes - in case tracks were added in a separate block for the same video.

}

this.setRating();
Expand Down Expand Up @@ -151,6 +154,7 @@ const VideoPressEdit = CoreVideoEdit =>
const guid = get( media, 'jetpack_videopress.guid' );
if ( guid ) {
setAttributes( { guid } );
this.setTracks( guid );
} else {
this.fallbackToCore();
}
Expand Down Expand Up @@ -192,6 +196,35 @@ const VideoPressEdit = CoreVideoEdit =>
return media;
};

setTracks = async guid => {
const { setAttributes } = this.props;

if ( ! guid ) {
return;
}

await apiFetch( {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this throw if the request fails? In async/await the catch portion becomes an exception we should probably handle.

url: `https://public-api.wordpress.com/rest/v1.1/videos/${ guid }`,
credentials: 'omit',
global: true,
} ).then( videoInfo => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we use async/await then the then() part is not needed, we can instead do const videoInfo = await apiFetch(...);

On the other hand, since this code seems pretty Promise-based, we can just go all in and: 1. get rid of await 2. add catch/finally to the promise.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated in bb30f90! I don't think we need to take any action in a catch or finally in this case. If we don't get the video info back, then we simply won't set the tracks data.

// Convert API object response to an array that works better with the tracks editor component
const tracks = [];
Object.keys( videoInfo.tracks ).forEach( kind => {
for ( const srcLang in videoInfo.tracks[ kind ] ) {
const track = videoInfo.tracks[ kind ][ srcLang ];
tracks.push( {
src: track.src,
kind,
srcLang,
label: track.label,
} );
}
} );
setAttributes( { videoPressTracks: tracks } );
} );
};

switchToEditing = () => {
this.props.setAttributes( {
id: undefined,
Expand Down Expand Up @@ -293,12 +326,32 @@ const VideoPressEdit = CoreVideoEdit =>
setAttributes,
} = this.props;
const { fallback, isFetchingMedia, isUpdatingRating, interactive, rating } = this.state;
const { autoplay, caption, controls, loop, muted, playsinline, poster, preload } = attributes;
const {
autoplay,
caption,
controls,
guid,
loop,
muted,
playsinline,
poster,
preload,
videoPressTracks,
} = attributes;

const videoPosterDescription = `video-block__poster-image-description-${ instanceId }`;

const blockSettings = (
<Fragment>
<BlockControls group="block">
<TracksEditor
tracks={ videoPressTracks }
onChange={ newTracks => {
setAttributes( { videoPressTracks: newTracks } );
} }
guid={ guid }
/>
</BlockControls>
<BlockControls>
<ToolbarGroup>
<ToolbarButton
Expand Down
7 changes: 7 additions & 0 deletions projects/plugins/jetpack/extensions/blocks/videopress/editor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,13 @@ const addVideoPressSupport = ( settings, name ) => {
src: {
type: 'string',
},
videoPressTracks: {
type: 'array',
items: {
type: 'object',
},
default: [],
},
videoPressClassNames: {
type: 'string',
},
Expand Down
95 changes: 95 additions & 0 deletions projects/plugins/jetpack/extensions/blocks/videopress/editor.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
@import 'variables.scss';
jeherve marked this conversation as resolved.
Show resolved Hide resolved

.no-videopress-media-placeholder {
.components-placeholder__fieldset {
flex-direction: row-reverse;
Expand All @@ -22,3 +24,96 @@
.videopress-block-hide {
display: none;
}

.videopress-block-tracks-editor > .components-popover__content {
width: 360px;
}

.videopress-block-tracks-editor__track-list-track {
display: flex;
place-content: space-between;
align-items: center;
padding: $grid-unit-05 0 $grid-unit-05 $grid-unit-15;
min-height: 23px;
}

.videopress-block-tracks-editor__track-list-track-delete {
display: flex;
align-items: center;
}

.videopress-block-tracks-editor__single-track-editor-label-language {
display: flex;
margin-top: $grid-unit-15;
& > .components-base-control {
width: 50%;
}
& > .components-base-control:first-child {
margin-right: $grid-unit-20;
}
}

.videopress-block-tracks-editor__single-track-editor-kind-select {
max-width: 240px;
}

.videopress-block-tracks-editor__single-track-editor-buttons-container {
display: flex;
place-content: space-between;
margin-top: $grid-unit-40;
min-height: 36px;
}

.videopress-block-tracks-editor__single-track-editor-label {
color: #757575;
text-transform: uppercase;
font-size: 11px;
font-weight: 500;
display: block;
margin-top: $grid-unit-05;
margin-bottom: $grid-unit-15;
}

.videopress-block-tracks-editor > .components-popover__content > div {
padding: 0;
}
.videopress-block-tracks-editor__track-list,
.videopress-block-tracks-editor__add-tracks-container {
.components-menu-group__label {
padding: 0;
}
}

.videopress-block-tracks-editor__single-track-editor,
.videopress-block-tracks-editor__track-list,
.videopress-block-tracks-editor__add-tracks-container {
padding: $grid-unit-15;
}

.videopress-block-tracks-editor__single-track-editor .components-base-control {
.components-base-control__label {
margin-bottom: $grid-unit-05;
}
.components-base-control__field {
margin-bottom: $grid-unit-15;
}
.components-text-control__input {
margin-left: 0;
}
.components-input-control__label {
margin-bottom: $grid-unit-05;
}
}

.videopress-block-tracks-editor__single-track-editor-edit-file-label {
display: flex;

.components-form-file-upload, .videopress-block-tracks-editor__single-track-editor-edit-file-label-name {
margin-inline-start: $grid-unit;
}
}

.videopress-block-tracks-editor__single-track-editor-error {
padding: $grid-unit-15 0;
color: #cc1818;
jeherve marked this conversation as resolved.
Show resolved Hide resolved
}
Loading