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.

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

Video: Redux state changes for custom video poster #11633

Merged
merged 23 commits into from
Apr 3, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
79dc087
Add undocumented endpoint for updating video poster
donnapep Feb 27, 2017
90f2538
Add video editor action types
donnapep Feb 27, 2017
0841192
Add video editor actions
donnapep Feb 27, 2017
fd6fdd6
Add video editor reducer
donnapep Feb 28, 2017
514500c
Add video editor selectors
donnapep Feb 28, 2017
27880f0
Add video editor to combined reducers
donnapep Feb 28, 2017
95e6da9
Ensure stats are updated when modal view changes
donnapep Feb 28, 2017
1fdedcb
Add constant for video editor
donnapep Feb 28, 2017
a1bba85
Describe params object in JSDoc comment
donnapep Mar 9, 2017
91cec7e
Switch to using data layer for updating video poster
donnapep Mar 10, 2017
7b7515d
Don't store video loading and script errors in Redux state
donnapep Mar 15, 2017
fde2264
Track upload progress updates
donnapep Mar 16, 2017
121bd16
Remove obsolete action for resetting poster state
donnapep Mar 16, 2017
fffa8c2
Rename Redux update actions
donnapep Mar 17, 2017
5e6d625
Move selectors and tests to state/selectors
donnapep Mar 21, 2017
34d26bb
Update documentation comments in reducer
donnapep Mar 21, 2017
e692e94
Refactor handler for updating the poster
donnapep Mar 22, 2017
c192bf8
Refactor action creators to be more concise
donnapep Mar 22, 2017
e1a0606
Use 'atTime' instead of 'at_time'
donnapep Mar 22, 2017
f69706a
Add action to indicate that the video editor modal should close
donnapep Mar 27, 2017
ef2706f
Minor tweaks to video editor poster handler
donnapep Mar 27, 2017
425af57
Rename `hasPosterUpdateError` selector to `shouldShowVideoEditorError`
donnapep Mar 27, 2017
7e22d20
Eliminate VIDEO_EDITOR_RESET_STATE action
donnapep Mar 28, 2017
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
5 changes: 5 additions & 0 deletions client/state/action-types.js
Original file line number Diff line number Diff line change
Expand Up @@ -640,6 +640,11 @@ export const USER_SUGGESTIONS_RECEIVE = 'USER_SUGGESTIONS_RECEIVE';
export const USER_SUGGESTIONS_REQUEST = 'USER_SUGGESTIONS_REQUEST';
export const USER_SUGGESTIONS_REQUEST_FAILURE = 'USER_SUGGESTIONS_REQUEST_FAILURE';
export const USER_SUGGESTIONS_REQUEST_SUCCESS = 'USER_SUGGESTIONS_REQUEST_SUCCESS';
export const VIDEO_EDITOR_CLOSE_MODAL = 'VIDEO_EDITOR_CLOSE_MODAL';
export const VIDEO_EDITOR_SET_POSTER_URL = 'VIDEO_EDITOR_SET_POSTER_URL';
export const VIDEO_EDITOR_SHOW_ERROR = 'VIDEO_EDITOR_SHOW_ERROR';
export const VIDEO_EDITOR_SHOW_UPLOAD_PROGRESS = 'VIDEO_EDITOR_SHOW_UPLOAD_PROGRESS';
export const VIDEO_EDITOR_UPDATE_POSTER = 'VIDEO_EDITOR_UPDATE_POSTER';
export const WORDADS_SITE_APPROVE_REQUEST = 'WORDADS_SITE_APPROVE_REQUEST';
export const WORDADS_SITE_APPROVE_REQUEST_DISMISS_ERROR = 'WORDADS_SITE_APPROVE_REQUEST_DISMISS_ERROR';
export const WORDADS_SITE_APPROVE_REQUEST_DISMISS_SUCCESS = 'WORDADS_SITE_APPROVE_REQUEST_DISMISS_SUCCESS';
Expand Down
6 changes: 4 additions & 2 deletions client/state/data-layer/wpcom/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,18 @@
import { mergeHandlers } from 'state/data-layer/utils';
import accountRecovery from './account-recovery';
import plans from './plans';
import sites from './sites';
import read from './read';
import sites from './sites';
import timezones from './timezones';
import videos from './videos';

export const handlers = mergeHandlers(
accountRecovery,
plans,
sites,
read,
sites,
timezones,
videos,
);

export default handlers;
9 changes: 9 additions & 0 deletions client/state/data-layer/wpcom/videos/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/**
* Internal dependencies
*/
import { mergeHandlers } from 'state/data-layer/utils';
import poster from './poster';

export default mergeHandlers(
poster,
);
67 changes: 67 additions & 0 deletions client/state/data-layer/wpcom/videos/poster/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
/**
* Internal dependencies
*/
import { http } from 'state/data-layer/wpcom-http/actions';
import { dispatchRequest } from 'state/data-layer/wpcom-http/utils';
import { VIDEO_EDITOR_UPDATE_POSTER } from 'state/action-types';
import {
closeModal,
setPosterUrl,
showError,
showUploadProgress,
} from 'state/ui/editor/video-editor/actions';

/**
* Updates the poster for a video.
*
* @param {Object} store Redux store
* @param {Object} action Action object
* @param {Function} next Dispatches to next middleware in chain
* @returns {Object} original action
*/
export const updatePoster = ( { dispatch }, action, next ) => {
if ( ! ( 'file' in action.params || 'atTime' in action.params ) ) {
return next( action );
}

const { atTime, file } = action.params;
const params = Object.assign(
{
apiVersion: '1.1',
method: 'POST',
path: `/videos/${ action.videoId }/poster`,
},
file && { formData: [ [ 'poster', file ] ] },
atTime !== undefined && { body: { at_time: atTime } },
);

dispatch( http( params, action ) );

return next( action );
};

export const receivePosterUrl = ( { dispatch }, action, next, { poster: posterUrl } ) => {
dispatch( setPosterUrl( posterUrl ) );
dispatch( closeModal() );
};

export const receivePosterError = ( { dispatch } ) => {
dispatch( showError() );
};

export const receiveUploadProgress = ( { dispatch }, action, next, progress ) => {
let percentage = 0;

if ( 'loaded' in progress && 'total' in progress ) {
percentage = Math.min( Math.round( progress.loaded / progress.total * 100 ), 100 );
}

dispatch( showUploadProgress( percentage ) );
};

export const dispatchPosterRequest =
dispatchRequest( updatePoster, receivePosterUrl, receivePosterError, receiveUploadProgress );

export default {
[ VIDEO_EDITOR_UPDATE_POSTER ]: [ dispatchPosterRequest ],
};
9 changes: 9 additions & 0 deletions client/state/selectors/get-poster-upload-progress.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/**
* Returns the poster upload progress.
*
* @param {Object} state Global state tree
* @return {Number} Poster upload progress percentage.
*/
export default function getPosterUploadProgress( state ) {
return state.ui.editor.videoEditor.uploadProgress;
}
9 changes: 9 additions & 0 deletions client/state/selectors/get-poster-url.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/**
* Returns the poster URL of the video.
*
* @param {Object} state Global state tree
* @return {String} URL of the poster.
*/
export default function getPosterUrl( state ) {
return state.ui.editor.videoEditor.url;
}
4 changes: 4 additions & 0 deletions client/state/selectors/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ export getMediaUrl from './get-media-url';
export getMenuItemTypes from './get-menu-item-types';
export getPastBillingTransaction from './get-past-billing-transaction';
export getPastBillingTransactions from './get-past-billing-transactions';
export getPosterUploadProgress from './get-poster-upload-progress';
export getPosterUrl from './get-poster-url';
export getPostLikes from './get-post-likes';
export getRawOffsets from './get-raw-offsets';
export getReaderTeams from './get-reader-teams';
Expand Down Expand Up @@ -101,3 +103,5 @@ export isSiteSupportingImageEditor from './is-site-supporting-image-editor';
export isTransientMedia from './is-transient-media';
export isUpdatingJetpackSettings from './is-updating-jetpack-settings';
export isUserRegistrationDaysWithinRange from './is-user-registration-days-within-range';
export shouldCloseVideoEditorModal from './should-close-video-editor-modal';
export shouldShowVideoEditorError from './should-show-video-editor-error';
10 changes: 10 additions & 0 deletions client/state/selectors/should-close-video-editor-modal.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/**
* Returns true if the video editor modal should be closed.
*
* @param {Object} state Global state tree
* @return {Boolean} true if the modal should be closed.
*
*/
export default function shouldCloseVideoEditorModal( state ) {
return state.ui.editor.videoEditor.closeModal;
}
10 changes: 10 additions & 0 deletions client/state/selectors/should-show-video-editor-error.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/**
* Returns true if an error should be shown in the video editor.
*
* @param {Object} state Global state tree
* @return {Boolean} true if an error should be shown.
*
*/
export default function shouldShowVideoEditorError( state ) {
return state.ui.editor.videoEditor.showError;
}
26 changes: 26 additions & 0 deletions client/state/selectors/test/get-poster-upload-progress.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/**
* External dependencies
*/
import { expect } from 'chai';

/**
* Internal dependencies
*/
import { getPosterUploadProgress } from '../';

describe( 'getPosterUploadProgress()', () => {
it( 'should return the upload progress', () => {
const percentage = 50;
const uploadProgress = getPosterUploadProgress( {
ui: {
editor: {
videoEditor: {
uploadProgress: percentage
}
}
}
} );

expect( uploadProgress ).to.eql( percentage );
} );
} );
26 changes: 26 additions & 0 deletions client/state/selectors/test/get-poster-url.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/**
* External dependencies
*/
import { expect } from 'chai';

/**
* Internal dependencies
*/
import { getPosterUrl } from '../';

describe( 'getPosterUrl()', () => {
it( 'should return the current video editor poster', () => {
const url = 'https://i1.wp.com/videos.files.wordpress.com/dummy-guid/thumbnail.jpg?ssl=1';
const poster = getPosterUrl( {
ui: {
editor: {
videoEditor: {
url
}
}
}
} );

expect( poster ).to.eql( url );
} );
} );
25 changes: 25 additions & 0 deletions client/state/selectors/test/should-close-video-editor-modal.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/**
* External dependencies
*/
import { expect } from 'chai';

/**
* Internal dependencies
*/
import { shouldCloseVideoEditorModal } from '../';

describe( 'shouldCloseVideoEditorModal()', () => {
it( 'should return whether or not the video editor modal should be closed', () => {
const shouldClose = shouldCloseVideoEditorModal( {
ui: {
editor: {
videoEditor: {
closeModal: false
}
}
}
} );

expect( shouldClose ).to.be.false;
} );
} );
25 changes: 25 additions & 0 deletions client/state/selectors/test/should-show-video-editor-error.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/**
* External dependencies
*/
import { expect } from 'chai';

/**
* Internal dependencies
*/
import { shouldShowVideoEditorError } from '../';

describe( 'shouldShowVideoEditorError()', () => {
it( 'should return the poster error state', () => {
const showError = shouldShowVideoEditorError( {
ui: {
editor: {
videoEditor: {
showError: true
}
}
}
} );

expect( showError ).to.be.true;
} );
} );
3 changes: 2 additions & 1 deletion client/state/ui/editor/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ export const MODAL_VIEW_STATS = {
[ ModalViews.LIST ]: 'view_list',
[ ModalViews.DETAIL ]: 'view_detail',
[ ModalViews.GALLERY ]: 'view_gallery',
[ ModalViews.IMAGE_EDITOR ]: 'view_edit'
[ ModalViews.IMAGE_EDITOR ]: 'view_edit',
[ ModalViews.VIDEO_EDITOR ]: 'view_edit',
Copy link
Contributor

Choose a reason for hiding this comment

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

comma?

};

/**
Expand Down
2 changes: 2 additions & 0 deletions client/state/ui/editor/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { combineReducers } from 'redux';
*/
import { EDITOR_START, POST_SAVE_SUCCESS } from 'state/action-types';
import imageEditor from './image-editor/reducer';
import videoEditor from './video-editor/reducer';
import lastDraft from './last-draft/reducer';
import contactForm from './contact-form/reducer';

Expand All @@ -33,6 +34,7 @@ export function postId( state = null, action ) {
export default combineReducers( {
postId,
imageEditor,
videoEditor,
Copy link
Member

Choose a reason for hiding this comment

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

it appears like the video editor state has no reference to any particular media id or site or anything. it's "whatever is out front in the editor" but what happens if we change sites or change videos which we are working on? won't we have race-conditions with network requests on the wire?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The video editor is a modal that is opened from the media library modal. Media library modal handles passing the particular video and everything else the video editor needs to know. And because it's a modal, site switching should be a non-issue. Hope that helps clarify and makes sense.

lastDraft,
contactForm
} );
3 changes: 2 additions & 1 deletion client/state/ui/editor/test/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ describe( 'reducer', () => {
'postId',
'lastDraft',
'contactForm',
'imageEditor'
'imageEditor',
'videoEditor',
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it proper to have a final comma in a JavaScript array?

Copy link
Member

Choose a reason for hiding this comment

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

in Calypso it is yeah. we encourage it for the effects on diffs

] );
} );

Expand Down
53 changes: 53 additions & 0 deletions client/state/ui/editor/video-editor/actions.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/**
* Internal dependencies
*/
import {
VIDEO_EDITOR_CLOSE_MODAL,
VIDEO_EDITOR_SET_POSTER_URL,
VIDEO_EDITOR_SHOW_ERROR,
VIDEO_EDITOR_SHOW_UPLOAD_PROGRESS,
VIDEO_EDITOR_UPDATE_POSTER,
} from 'state/action-types';

/**
* Returns an action object to indicate that a request has been made to update the video poster.
*
* @param {String} videoId ID of the video
* @param {Object} params Poster data
* @param {Number} [params.atTime] Number of seconds into the video at which to get the poster
* @param {Object} [params.file] An image to attach to the video
* @return {Object} Action object
*/
export const updatePoster = ( videoId, params ) =>
( { type: VIDEO_EDITOR_UPDATE_POSTER, videoId, params } );

/**
* Returns an action object to indicate that the poster for the video has been updated successfully.
*
* @param {String} posterUrl Poster URL
* @return {Object} Action object
*/
export const setPosterUrl = posterUrl => ( { type: VIDEO_EDITOR_SET_POSTER_URL, posterUrl } );

/**
* Returns an action object to indicate that the video editor modal should close.
*
* @return {Object} Action object
*/
export const closeModal = () => ( { type: VIDEO_EDITOR_CLOSE_MODAL } );

/**
* Returns an action object to indicate that the poster for the video failed to update.
*
* @return {Object} Action object
*/
export const showError = () => ( { type: VIDEO_EDITOR_SHOW_ERROR } );

/**
* Returns an action object to indicate the poster upload progress.
*
* @param {String} percentage Upload progress percentage
* @return {Object} Action object
*/
export const showUploadProgress = percentage =>
( { type: VIDEO_EDITOR_SHOW_UPLOAD_PROGRESS, percentage } );
Loading