From b1ca6fe3e85f242fbf98886dab9714484af427e8 Mon Sep 17 00:00:00 2001 From: Artur Gaspar Date: Tue, 17 Oct 2023 11:38:09 -0300 Subject: [PATCH 1/5] fix: video upload page layout fixes --- .../VideoUploadEditor/VideoUploader.jsx | 48 +++--- .../__snapshots__/VideoUploader.test.jsx.snap | 148 ++++++++---------- .../__snapshots__/index.test.jsx.snap | 148 ++++++++---------- .../containers/VideoUploadEditor/index.scss | 21 --- 4 files changed, 161 insertions(+), 204 deletions(-) diff --git a/src/editors/containers/VideoUploadEditor/VideoUploader.jsx b/src/editors/containers/VideoUploadEditor/VideoUploader.jsx index c2783fe64..08103c195 100644 --- a/src/editors/containers/VideoUploadEditor/VideoUploader.jsx +++ b/src/editors/containers/VideoUploadEditor/VideoUploader.jsx @@ -17,17 +17,20 @@ const URLUploader = () => { const intl = useIntl(); return (
-
- +
+
-
- {intl.formatMessage(messages.dropVideoFileHere)} - {intl.formatMessage(messages.info)} +
+ {intl.formatMessage(messages.dropVideoFileHere)} + {intl.formatMessage(messages.info)}
-
OR
-
- +
+ OR +
+
+ { onClick={(event) => { event.stopPropagation(); }} onChange={(event) => { setTextInputValue(event.target.value); }} /> -
- { - event.stopPropagation(); - if (textInputValue.trim() !== '') { - onURLUpload(textInputValue); - } - }} - /> -
+ { + event.stopPropagation(); + if (textInputValue.trim() !== '') { + onURLUpload(textInputValue); + } + }} + />
diff --git a/src/editors/containers/VideoUploadEditor/__snapshots__/VideoUploader.test.jsx.snap b/src/editors/containers/VideoUploadEditor/__snapshots__/VideoUploader.test.jsx.snap index c4307f49a..736654a5c 100644 --- a/src/editors/containers/VideoUploadEditor/__snapshots__/VideoUploader.test.jsx.snap +++ b/src/editors/containers/VideoUploadEditor/__snapshots__/VideoUploader.test.jsx.snap @@ -60,11 +60,11 @@ Object { class="d-flex flex-column" >
- + Drag and drop video here or click to upload Upload MP4 or MOV files (5 GB max)
OR
-
- -
+ +
@@ -218,11 +212,11 @@ Object { class="d-flex flex-column" >
- + Drag and drop video here or click to upload Upload MP4 or MOV files (5 GB max)
OR
-
- -
+ +
diff --git a/src/editors/containers/VideoUploadEditor/__snapshots__/index.test.jsx.snap b/src/editors/containers/VideoUploadEditor/__snapshots__/index.test.jsx.snap index e85627dc6..a125e89bb 100644 --- a/src/editors/containers/VideoUploadEditor/__snapshots__/index.test.jsx.snap +++ b/src/editors/containers/VideoUploadEditor/__snapshots__/index.test.jsx.snap @@ -64,11 +64,11 @@ Object { class="d-flex flex-column" >
- + Drag and drop video here or click to upload Upload MP4 or MOV files (5 GB max)
OR
-
- -
+ +
@@ -228,11 +222,11 @@ Object { class="d-flex flex-column" >
- + Drag and drop video here or click to upload Upload MP4 or MOV files (5 GB max)
OR
-
- -
+ +
diff --git a/src/editors/containers/VideoUploadEditor/index.scss b/src/editors/containers/VideoUploadEditor/index.scss index c1bd2f3be..445e35257 100644 --- a/src/editors/containers/VideoUploadEditor/index.scss +++ b/src/editors/containers/VideoUploadEditor/index.scss @@ -11,12 +11,6 @@ width: 100%; } -.url-submit-button { - position: absolute; - margin-left: 17rem; - font-size: 0.75rem; -} - .video-id-prompt { input::placeholder { @@ -24,7 +18,6 @@ // color: #5E35B1; font-weight: '500'; word-wrap: 'break-word'; - font-size: 0.875rem !important; } button { @@ -36,17 +29,3 @@ background: rgba(239, 234, 247, 0.70); } } - -.video-upload-input-group{ - .form-control { - font-size: 0.875rem !important; - width: 308px !important; - height: 44px !important; - } - - .pgn__icon.pgn__icon__lg { - width: 3.625rem !important; - height: 3.625rem !important; - } -} - From 212578cfac4a0552275a55101597978d9f8770c2 Mon Sep 17 00:00:00 2001 From: Artur Gaspar Date: Tue, 17 Oct 2023 13:01:57 -0300 Subject: [PATCH 2/5] fix: video editor thumbnail fallback icon colour and size --- .../VideoSettingsModal/components/VideoPreviewWidget/index.jsx | 2 +- src/editors/data/images/videoThumbnail.svg | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/VideoPreviewWidget/index.jsx b/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/VideoPreviewWidget/index.jsx index 9cff6456e..6b8e54036 100644 --- a/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/VideoPreviewWidget/index.jsx +++ b/src/editors/containers/VideoEditor/components/VideoSettingsModal/components/VideoPreviewWidget/index.jsx @@ -32,7 +32,7 @@ export const VideoPreviewWidget = ({
{intl.formatMessage(thumbnailMessages.thumbnailAltText)} - + From 85cc5bebe3bdc4663f58e0a38cbbbfdc0ad5f4e2 Mon Sep 17 00:00:00 2001 From: Artur Gaspar Date: Wed, 18 Oct 2023 17:23:25 -0300 Subject: [PATCH 3/5] fix: video uploader close button go back instead of closing app Change the video uploader close button to always go back in navigation history, and change the gallery to replace its location with the video uploader's when automatically loading it due to an empty video list, so that when the uploader goes back in the history, it goes to what was before the gallery. --- src/editors/containers/VideoGallery/hooks.js | 10 +++++++--- src/editors/containers/VideoGallery/index.jsx | 2 +- .../containers/VideoGallery/index.test.jsx | 6 +++--- .../VideoUploadEditor/VideoUploader.jsx | 8 +++----- .../containers/VideoUploadEditor/hooks.js | 3 +++ .../containers/VideoUploadEditor/index.jsx | 13 ++----------- .../VideoUploadEditor/index.test.jsx | 19 ++++++++++++------- 7 files changed, 31 insertions(+), 30 deletions(-) diff --git a/src/editors/containers/VideoGallery/hooks.js b/src/editors/containers/VideoGallery/hooks.js index 3c8c3ec24..4912a8aac 100644 --- a/src/editors/containers/VideoGallery/hooks.js +++ b/src/editors/containers/VideoGallery/hooks.js @@ -145,10 +145,14 @@ export const useVideoListProps = ({ }; }; -export const useVideoUploadHandler = () => { +export const useVideoUploadHandler = ({ replace }) => { const learningContextId = useSelector(selectors.app.learningContextId); const blockId = useSelector(selectors.app.blockId); - return () => navigateTo(`/course/${learningContextId}/editor/video_upload/${blockId}`); + const path = `/course/${learningContextId}/editor/video_upload/${blockId}`; + if (replace) { + return () => window.location.replace(path); + } + return () => navigateTo(path); }; export const useCancelHandler = () => ( @@ -203,7 +207,7 @@ export const useVideoProps = ({ videos }) => { inputError, selectBtnProps, } = videoList; - const fileInput = { click: useVideoUploadHandler() }; + const fileInput = { click: useVideoUploadHandler({ replace: false }) }; return { galleryError, diff --git a/src/editors/containers/VideoGallery/index.jsx b/src/editors/containers/VideoGallery/index.jsx index 0600c1334..164f174ae 100644 --- a/src/editors/containers/VideoGallery/index.jsx +++ b/src/editors/containers/VideoGallery/index.jsx @@ -19,7 +19,7 @@ export const VideoGallery = () => { (state) => selectors.requests.isFailed(state, { requestKey: RequestKeys.uploadVideo }), ); const videos = hooks.buildVideos({ rawVideos }); - const handleVideoUpload = hooks.useVideoUploadHandler(); + const handleVideoUpload = hooks.useVideoUploadHandler({ replace: true }); useEffect(() => { // If no videos exists redirects to the video upload screen diff --git a/src/editors/containers/VideoGallery/index.test.jsx b/src/editors/containers/VideoGallery/index.test.jsx index 1ac21014d..9ae86f403 100644 --- a/src/editors/containers/VideoGallery/index.test.jsx +++ b/src/editors/containers/VideoGallery/index.test.jsx @@ -80,7 +80,7 @@ describe('VideoGallery', () => { beforeAll(() => { oldLocation = window.location; delete window.location; - window.location = { assign: jest.fn() }; + window.location = { replace: jest.fn() }; }); afterAll(() => { window.location = oldLocation; @@ -118,10 +118,10 @@ describe('VideoGallery', () => { )); }); it('navigates to video upload page when there are no videos', async () => { - expect(window.location.assign).not.toHaveBeenCalled(); + expect(window.location.replace).not.toHaveBeenCalled(); updateState({ videos: [] }); await renderComponent(); - expect(window.location.assign).toHaveBeenCalled(); + expect(window.location.replace).toHaveBeenCalled(); }); it.each([ [/by date added \(newest\)/i, [2, 1, 3]], diff --git a/src/editors/containers/VideoUploadEditor/VideoUploader.jsx b/src/editors/containers/VideoUploadEditor/VideoUploader.jsx index 08103c195..2b6c98efa 100644 --- a/src/editors/containers/VideoUploadEditor/VideoUploader.jsx +++ b/src/editors/containers/VideoUploadEditor/VideoUploader.jsx @@ -8,7 +8,6 @@ import { ArrowForward, FileUpload, Close } from '@edx/paragon/icons'; import { useDispatch } from 'react-redux'; import { thunkActions } from '../../data/redux'; import * as hooks from './hooks'; -import * as editorHooks from '../EditorContainer/hooks'; import messages from './messages'; const URLUploader = () => { @@ -58,10 +57,10 @@ const URLUploader = () => { ); }; -export const VideoUploader = ({ setLoading, onClose }) => { +export const VideoUploader = ({ setLoading }) => { const dispatch = useDispatch(); const intl = useIntl(); - const handleCancel = editorHooks.handleCancel({ onClose }); + const goBack = hooks.useHistoryGoBack(); const handleProcessUpload = ({ fileData }) => { dispatch(thunkActions.video.uploadVideo({ @@ -79,7 +78,7 @@ export const VideoUploader = ({ setLoading, onClose }) => { alt={intl.formatMessage(messages.closeButtonAltText)} src={Close} iconAs={Icon} - onClick={handleCancel} + onClick={goBack} />
{ VideoUploader.propTypes = { setLoading: PropTypes.func.isRequired, - onClose: PropTypes.func.isRequired, }; export default VideoUploader; diff --git a/src/editors/containers/VideoUploadEditor/hooks.js b/src/editors/containers/VideoUploadEditor/hooks.js index 7771f2d48..d96996d81 100644 --- a/src/editors/containers/VideoUploadEditor/hooks.js +++ b/src/editors/containers/VideoUploadEditor/hooks.js @@ -31,8 +31,11 @@ export const useUploadVideo = async ({ })); }; +export const useHistoryGoBack = () => (() => window.history.back()); + export default { postUploadRedirect, onVideoUpload, useUploadVideo, + useHistoryGoBack, }; diff --git a/src/editors/containers/VideoUploadEditor/index.jsx b/src/editors/containers/VideoUploadEditor/index.jsx index ba4bf12d6..9dc31ab35 100644 --- a/src/editors/containers/VideoUploadEditor/index.jsx +++ b/src/editors/containers/VideoUploadEditor/index.jsx @@ -1,16 +1,11 @@ import React from 'react'; -import PropTypes from 'prop-types'; import { useIntl } from '@edx/frontend-platform/i18n'; import { Spinner } from '@edx/paragon'; import './index.scss'; import messages from './messages'; import { VideoUploader } from './VideoUploader'; -export const VideoUploadEditor = ( - { - onClose, - }, -) => { +export const VideoUploadEditor = () => { const [loading, setLoading] = React.useState(false); const intl = useIntl(); @@ -18,7 +13,7 @@ export const VideoUploadEditor = (
{(!loading) ? (
- +
) : (
@@ -33,8 +28,4 @@ export const VideoUploadEditor = ( ); }; -VideoUploadEditor.propTypes = { - onClose: PropTypes.func.isRequired, -}; - export default VideoUploadEditor; diff --git a/src/editors/containers/VideoUploadEditor/index.test.jsx b/src/editors/containers/VideoUploadEditor/index.test.jsx index bf032cd70..e200e9f3a 100644 --- a/src/editors/containers/VideoUploadEditor/index.test.jsx +++ b/src/editors/containers/VideoUploadEditor/index.test.jsx @@ -13,13 +13,12 @@ jest.unmock('@edx/paragon'); jest.unmock('@edx/paragon/icons'); describe('VideoUploadEditor', () => { - const onCloseMock = jest.fn(); let store; - const renderComponent = async (storeParam, onCloseMockParam) => render( + const renderComponent = async (storeParam) => render( - + , , ); @@ -45,14 +44,20 @@ describe('VideoUploadEditor', () => { }); it('renders as expected with default behavior', async () => { - expect(await renderComponent(store, onCloseMock)).toMatchSnapshot(); + expect(await renderComponent(store)).toMatchSnapshot(); }); - it('calls onClose when close button is clicked', async () => { - const container = await renderComponent(store, onCloseMock); + it('calls window.history.back when close button is clicked', async () => { + const container = await renderComponent(store); const closeButton = container.getAllByRole('button', { name: /close/i }); + const oldHistoryBack = window.history.back; + window.history.back = jest.fn(); + expect(closeButton).toHaveLength(1); + expect(window.history.back).not.toHaveBeenCalled(); closeButton.forEach((button) => fireEvent.click(button)); - expect(onCloseMock).toHaveBeenCalled(); + expect(window.history.back).toHaveBeenCalled(); + + window.history.back = oldHistoryBack; }); }); From 204a70f2a6bc65a9ec9dfc2b13de5c364dc39503 Mon Sep 17 00:00:00 2001 From: Artur Gaspar Date: Thu, 19 Oct 2023 14:07:29 -0300 Subject: [PATCH 4/5] fix: video editor spinners vertical alignment The Editor component uses the pgn__modal-fullscreen class to be fullscreen, but it is not structured like a Paragon FullscreenModal and the fullscreen positioning style is not applied correctly, particularly in the case where the content is smaller than the body - a vertically centred component will be centred to the content size, not to the screen. Here we directly apply the style that would have applied to the relevant elements of a Paragon FullscreenModal. --- src/editors/Editor.jsx | 14 +- .../__snapshots__/Editor.test.jsx.snap | 22 +- .../__snapshots__/index.test.jsx.snap | 18 +- .../containers/EditorContainer/index.jsx | 5 +- .../__snapshots__/index.test.jsx.snap | 314 +++++++++--------- .../containers/VideoUploadEditor/index.jsx | 32 +- 6 files changed, 221 insertions(+), 184 deletions(-) diff --git a/src/editors/Editor.jsx b/src/editors/Editor.jsx index d1b14cb38..9236e0474 100644 --- a/src/editors/Editor.jsx +++ b/src/editors/Editor.jsx @@ -31,9 +31,19 @@ export const Editor = ({ const EditorComponent = supportedEditors[blockType]; return ( -
+
diff --git a/src/editors/__snapshots__/Editor.test.jsx.snap b/src/editors/__snapshots__/Editor.test.jsx.snap index 8c929ec63..8a314a651 100644 --- a/src/editors/__snapshots__/Editor.test.jsx.snap +++ b/src/editors/__snapshots__/Editor.test.jsx.snap @@ -3,10 +3,19 @@ exports[`Editor render presents error message if no relevant editor found and ref ready 1`] = `
TextEditor) 1`] = `

My test content @@ -78,7 +83,12 @@ exports[`EditorContainer component render snapshot: initialized. enable save and exports[`EditorContainer component render snapshot: not initialized. disable save and pass to header 1`] = `
- + {isInitialized && children} -
-
-
-
-
- -
- -
-
-
- , -
- , - "container":
+ , +
+ , + "container":
+
+
+
+ +
+ +
,
, diff --git a/src/editors/containers/VideoUploadEditor/index.jsx b/src/editors/containers/VideoUploadEditor/index.jsx index 9dc31ab35..9c91260be 100644 --- a/src/editors/containers/VideoUploadEditor/index.jsx +++ b/src/editors/containers/VideoUploadEditor/index.jsx @@ -9,21 +9,23 @@ export const VideoUploadEditor = () => { const [loading, setLoading] = React.useState(false); const intl = useIntl(); - return ( -
- {(!loading) ? ( -
- -
- ) : ( -
- -
- )} + return (!loading) ? ( +
+ +
+ ) : ( +
+
); }; From 67a53d0a8fd761e10465f24d9ebb21f6952eaad6 Mon Sep 17 00:00:00 2001 From: Artur Gaspar Date: Mon, 4 Dec 2023 09:29:01 -0300 Subject: [PATCH 5/5] fix: use trailingElement for video uploader input button Also styles the button so it looks the same on hover/active. --- .../VideoUploadEditor/VideoUploader.jsx | 29 +++-- .../__snapshots__/VideoUploader.test.jsx.snap | 122 +++++++++--------- .../__snapshots__/index.test.jsx.snap | 122 +++++++++--------- .../containers/VideoUploadEditor/index.scss | 10 ++ 4 files changed, 153 insertions(+), 130 deletions(-) diff --git a/src/editors/containers/VideoUploadEditor/VideoUploader.jsx b/src/editors/containers/VideoUploadEditor/VideoUploader.jsx index 2b6c98efa..8b7a73f94 100644 --- a/src/editors/containers/VideoUploadEditor/VideoUploader.jsx +++ b/src/editors/containers/VideoUploadEditor/VideoUploader.jsx @@ -36,20 +36,21 @@ const URLUploader = () => { borderless onClick={(event) => { event.stopPropagation(); }} onChange={(event) => { setTextInputValue(event.target.value); }} - /> - { - event.stopPropagation(); - if (textInputValue.trim() !== '') { - onURLUpload(textInputValue); - } - }} + trailingElement={( + { + event.stopPropagation(); + if (textInputValue.trim() !== '') { + onURLUpload(textInputValue); + } + }} + /> + )} />
diff --git a/src/editors/containers/VideoUploadEditor/__snapshots__/VideoUploader.test.jsx.snap b/src/editors/containers/VideoUploadEditor/__snapshots__/VideoUploader.test.jsx.snap index 736654a5c..5a4f1efbc 100644 --- a/src/editors/containers/VideoUploadEditor/__snapshots__/VideoUploader.test.jsx.snap +++ b/src/editors/containers/VideoUploadEditor/__snapshots__/VideoUploader.test.jsx.snap @@ -107,7 +107,7 @@ Object { class="input-group" >
-
- + + + + + +

+
@@ -259,7 +262,7 @@ Object { class="input-group" >
-
- + + + + + +
+
diff --git a/src/editors/containers/VideoUploadEditor/__snapshots__/index.test.jsx.snap b/src/editors/containers/VideoUploadEditor/__snapshots__/index.test.jsx.snap index dcbdde965..62df48ed6 100644 --- a/src/editors/containers/VideoUploadEditor/__snapshots__/index.test.jsx.snap +++ b/src/editors/containers/VideoUploadEditor/__snapshots__/index.test.jsx.snap @@ -110,7 +110,7 @@ Object { class="input-group" >
-
- + + + + + + + @@ -266,7 +269,7 @@ Object { class="input-group" >
-
- + + + + + + + diff --git a/src/editors/containers/VideoUploadEditor/index.scss b/src/editors/containers/VideoUploadEditor/index.scss index 445e35257..0c678fb8e 100644 --- a/src/editors/containers/VideoUploadEditor/index.scss +++ b/src/editors/containers/VideoUploadEditor/index.scss @@ -1,3 +1,5 @@ +@import "@edx/paragon/scss/core/core"; + .dropzone-middle { border: 2px dashed #ccc; @@ -25,6 +27,14 @@ background-color: #FFFFFF; } + .btn-icon.url-submit-button { + &, &:active, &:hover { + background-color: transparent !important; + border: none !important; + color: $gray-500 !important; + } + } + .prompt-button { background: rgba(239, 234, 247, 0.70); }