Add a widget for managing thumbnail #1568

roro-lv · 2022-06-02T10:22:39Z

Purpose

Add a widget for uploading a thumbnail for the live video.
It also allows the instructor to replace it and/or delete it to get back to the default thumbnail.
The thumbnail is displayed when the live is starting or paused (on the student side) and when the video is converted to a VOD.

Caution : For now deleting a thumbnail is not working, because there is a bug on python soft delete (see here)

CHANGELOG.md

...nts/DashboardVideoLiveControlPane/customs/DashboardVideoLiveConfirmationModal/index.spec.tsx

...mponents/DashboardVideoLiveControlPane/customs/DashboardVideoLiveConfirmationModal/index.tsx

src/frontend/components/DashboardVideoLiveControlPane/index.spec.tsx

...dVideoLiveControlPane/widgets/DashboardVideoLiveWidgetThumbnail/ThumbnailDisplayer/index.tsx

kernicPanel · 2022-06-07T12:12:04Z

...ardVideoLiveControlPane/widgets/DashboardVideoLiveWidgetThumbnail/ThumbnailManager/index.tsx

+          size="0.875rem"
+          style={{ fontFamily: 'Roboto-Regular' }}


Suggested change

size="0.875rem"

style={{ fontFamily: 'Roboto-Regular' }}

size="xsmall"

Roboto-Regular shouldn't be needed.
Also, maybe xsmall provides a similar size ?

I would like to refute or confirm what you suggest, but i can't fin any correspondence table between pre-defined size and size in rem in grommet...

...veControlPane/widgets/DashboardVideoLiveWidgetThumbnail/ThumbnailRemoveButton/index.spec.tsx

src/frontend/components/StudentLiveAdvertising/index.spec.tsx

kernicPanel · 2022-06-07T12:23:25Z

src/frontend/components/StudentLiveAdvertising/index.tsx

+        {video.thumbnail && video.thumbnail.urls ? (
+          <ThumbnailDisplayer thumbnail={video.thumbnail} />
+        ) : (
+          <Image fit="cover" src={appData.static.img.liveBackground} />
+        )}


Maybe is there a way to use the same components for both static and uploaded thumbnails ?

Yes I put everything in one component

kernicPanel · 2022-06-07T12:26:00Z

src/frontend/data/queries/index.tsx

+  UseDeleteThumbnailError,
+  UseDeleteThumbnailData
+>;
+export const useDeleteThumbnail = (options?: UseDeleteThumbnailOptions) => {


IMO, you should add tests in src/frontend/data/queries/index.spec.tsx

yes i forgot it ty !

...deoLiveControlPane/widgets/DashboardVideoLiveWidgetThumbnail/ThumbnailRemoveButton/index.tsx

lunika · 2022-06-07T14:43:12Z

...mponents/DashboardVideoLiveControlPane/customs/DashboardVideoLiveConfirmationModal/index.tsx

+      onClickOutside={onModalCloseOrCancel}
+      responsive={false}
+      style={{
+        width: size === 'small' ? '95%' : '500px',


That's huge difference. Are you sure about this values ?

For info, this component comes from the PR on the shared live media : https://github.com/openfun/marsha/blob/9f991d654dfb754566af8ca8dd71c93ec49becd7/src/frontend/components/DashboardVideoLiveControlPane/customs/DashboardVideoLiveConfirmationModal/index.tsx

I think the values are correct because this is what I wanted : a almost full page modal when the screen is small, a medium fixed sized modal otherwise

...dVideoLiveControlPane/widgets/DashboardVideoLiveWidgetThumbnail/ThumbnailDisplayer/index.tsx

RVassili · 2022-06-08T09:53:23Z

...ardVideoLiveControlPane/widgets/DashboardVideoLiveWidgetThumbnail/ThumbnailManager/index.tsx

+  uploadManagerState,
+}: ThumbnailManagerProps) => {
+  const intl = useIntl();
+  const thumbnailState = thumbnail ? thumbnail.upload_state : uploadState.READY;


How can thumbnail be undefined or null ?

you're right, ty !

RVassili · 2022-06-08T09:54:56Z

...ardVideoLiveControlPane/widgets/DashboardVideoLiveWidgetThumbnail/ThumbnailManager/index.tsx

+  if (thumbnailState === uploadState.ERROR) {
+    textMessage = intl.formatMessage(messages.noThumbnailUploaded);
+  } else if (


When the state is in error, should we really display this message ?

Yes I agree, i changed the message to be more accurate

RVassili · 2022-06-08T09:56:59Z

...ardVideoLiveControlPane/widgets/DashboardVideoLiveWidgetThumbnail/ThumbnailManager/index.tsx

+    textMessage = intl.formatMessage(messages.noThumbnailUploaded);
+  } else if (
+    (thumbnailState === uploadState.PENDING &&
+      uploadManagerState[thumbnail!.id]?.status ===


Why do you need an assert here ? And you shouldn't use assert (or there is no reason to use typescript in the project)

I forgot to remove it, ty

RVassili · 2022-06-08T09:58:17Z

...ardVideoLiveControlPane/widgets/DashboardVideoLiveWidgetThumbnail/ThumbnailManager/index.tsx

+    (thumbnailState === uploadState.PENDING &&
+      uploadManagerState[thumbnail!.id]?.status ===
+        UploadManagerStatus.UPLOADING) ||
+    uploadManagerState[thumbnail!.id]?.status === UploadManagerStatus.INIT


Same comment here about the assert

I forgot to remove it, ty

RVassili · 2022-06-08T10:03:10Z

...ardVideoLiveControlPane/widgets/DashboardVideoLiveWidgetThumbnail/ThumbnailManager/index.tsx

+
+  let textMessage = null;
+
+  if (thumbnailState === uploadState.ERROR) {


It looks like you can externalize these if else if from this component (like in a dedicated file).

mm the point of the ThumbnailManager was already to externalize the logic you are talking about

...dVideoLiveControlPane/widgets/DashboardVideoLiveWidgetThumbnail/ThumbnailDisplayer/index.tsx

RVassili · 2022-06-08T10:07:06Z

...dVideoLiveControlPane/widgets/DashboardVideoLiveWidgetThumbnail/ThumbnailDisplayer/index.tsx

+
+export const ThumbnailDisplayer = ({
+  rounded,
+  thumbnail,


I don't think you should use a thumbnail here, you are using only the urls.
If you change your component props to receive urls you will be able to use this component to render the default background with it.
CF : https://github.com/openfun/marsha/pull/1568/files#r891150092

I put everything in one component

RVassili · 2022-06-08T10:18:21Z

And it would probably be easier if this is split in 2 PR:

one updating the waiting screen background for the student
one adding the component for the teacher to upload a thumbnail

lunika

The default live background is display with a very high size in the dashboard with firefox

...nts/DashboardVideoLiveControlPane/customs/DashboardVideoLiveConfirmationModal/index.spec.tsx

...veControlPane/widgets/DashboardVideoLiveWidgetThumbnail/ThumbnailRemoveButton/index.spec.tsx

RVassili · 2022-06-08T15:39:28Z

...ardVideoLiveControlPane/widgets/DashboardVideoLiveWidgetThumbnail/ThumbnailManager/index.tsx

+  if (thumbnail.upload_state === uploadState.ERROR) {
+    textMessage = intl.formatMessage(messages.thumbnailError);
+  } else if (
+    (thumbnail.upload_state === uploadState.PENDING &&
+      uploadManagerState[thumbnail.id]?.status ===
+        UploadManagerStatus.UPLOADING) ||
+    uploadManagerState[thumbnail.id]?.status === UploadManagerStatus.INIT
+  ) {
+    textMessage = intl.formatMessage(messages.thumbnailUploaded);
+  } else if (
+    thumbnail.upload_state === uploadState.PROCESSING ||
+    (thumbnail.upload_state === uploadState.PENDING &&
+      uploadManagerState[thumbnail.id]?.status === UploadManagerStatus.SUCCESS)
+  ) {
+    textMessage = intl.formatMessage(messages.thumbnailProcessed);
+  }


I insist, you can externalize this in a dedicated function returning a Nullable, and this component will be cleaner

...deoLiveControlPane/widgets/DashboardVideoLiveWidgetThumbnail/ThumbnailRemoveButton/index.tsx

RVassili · 2022-06-08T15:43:52Z

...components/DashboardVideoLiveControlPane/widgets/DashboardVideoLiveWidgetThumbnail/index.tsx

+    } else {
+      id = thumbnail.id;
+    }
+    addUpload(modelName.THUMBNAILS, id, event.target.files![0]);


Can you remove this assert ?

RVassili · 2022-06-08T15:44:10Z

...components/DashboardVideoLiveControlPane/widgets/DashboardVideoLiveWidgetThumbnail/index.tsx

+    >
+      <Box direction={'column'} justify={'center'} gap="small">
+        {!thumbnail ||
+        (thumbnail && !thumbnail.urls && !uploadManagerState[thumbnail!.id]) ? (


And this one too ?

RVassili · 2022-06-08T15:44:21Z

...components/DashboardVideoLiveControlPane/widgets/DashboardVideoLiveWidgetThumbnail/index.tsx

+          <Stack anchor="top-right">
+            <Box>
+              <ThumbnailManager
+                thumbnail={thumbnail!}


And this one too ?

RVassili · 2022-06-08T15:44:46Z

...components/DashboardVideoLiveControlPane/widgets/DashboardVideoLiveWidgetThumbnail/index.tsx

+              <PictureSVG height="24px" width="24px" iconColor="blue-active" />
+            </Box>
+          }
+          onClick={() => hiddenFileInput.current!.click()}


And this one too ?

...dVideoLiveControlPane/widgets/DashboardVideoLiveWidgetThumbnail/ThumbnailDisplayer/index.tsx

roro-lv · 2022-06-09T13:29:25Z

The default live background is display with a very high size in the dashboard with firefox

Indeed, there was a display problem on the thumbnail (display was different whether you accessed the website from Firefox or Chrome). It's now fixed.

lunika · 2022-06-09T15:15:47Z

CHANGELOG.md

@@ -20,6 +20,7 @@ Versioning](https://semver.org/spec/v2.0.0.html).
 - Add Legacy LTI configuration XML
 - Add an instructor widget to modify join the discussion mode
 - Prevent join API calls in join mode denied
+- Add a widget for the instructor to upload a live video thumbnail


Remember to move it in the Unreleased section

Add a Bin SVG (used for deletion) and a Picture SVG (used for upload image button) to the project.

Update advertising student screen, to display the uploaded thumbnail, when it exists.

Add a button for removing thumbnail (and add its associated queries). Add a component to manage what to display when thumbnail is uploaded. Add a confirmation modal for when an user wants to delete a thumbnail. Co-Authored-By: Manuel Raynaud <[email protected]> Co-Authored-By: Nicolas Clerc <[email protected]>

Add a widget to allow teacher to upload a thumbnail for its live. It also allows him to replace it or delete it and display the default thumbnail.

roro-lv self-assigned this Jun 2, 2022

roro-lv added feature Frontend labels Jun 2, 2022

roro-lv force-pushed the feature/widget/roro-lv/manage-thumbnail branch 13 times, most recently from 4c6f9e2 to 9fae066 Compare June 3, 2022 18:49

roro-lv requested review from RVassili, kernicPanel and lunika June 3, 2022 19:17

roro-lv force-pushed the feature/widget/roro-lv/manage-thumbnail branch from 082b78c to acd3322 Compare June 3, 2022 19:17

kernicPanel reviewed Jun 7, 2022

View reviewed changes

lunika reviewed Jun 7, 2022

View reviewed changes

roro-lv force-pushed the feature/widget/roro-lv/manage-thumbnail branch from acd3322 to 4a94fd6 Compare June 7, 2022 16:33

roro-lv changed the title ~~Feature/widget/roro lv/manage thumbnail~~ Add a widget for managing thumbnail Jun 8, 2022

lunika mentioned this pull request Jun 8, 2022

Join mode forced #1569

Merged

4 tasks

RVassili reviewed Jun 8, 2022

View reviewed changes

roro-lv requested review from kernicPanel, lunika and RVassili June 8, 2022 10:34

kernicPanel approved these changes Jun 8, 2022

View reviewed changes

roro-lv force-pushed the feature/widget/roro-lv/manage-thumbnail branch 3 times, most recently from 121cf48 to 4a49034 Compare June 8, 2022 14:46

lunika reviewed Jun 8, 2022

View reviewed changes

...nts/DashboardVideoLiveControlPane/customs/DashboardVideoLiveConfirmationModal/index.spec.tsx Outdated Show resolved Hide resolved

...veControlPane/widgets/DashboardVideoLiveWidgetThumbnail/ThumbnailRemoveButton/index.spec.tsx Show resolved Hide resolved

RVassili reviewed Jun 8, 2022

View reviewed changes

roro-lv force-pushed the feature/widget/roro-lv/manage-thumbnail branch 2 times, most recently from 248426d to 836b074 Compare June 9, 2022 10:18

roro-lv force-pushed the feature/widget/roro-lv/manage-thumbnail branch from e8fe35e to 5bf4a97 Compare June 9, 2022 13:35

roro-lv requested review from lunika, RVassili and kernicPanel June 9, 2022 14:25

kernicPanel approved these changes Jun 9, 2022

View reviewed changes

lunika approved these changes Jun 9, 2022

View reviewed changes

RVassili approved these changes Jun 13, 2022

View reviewed changes

🍱(front) add SVGs to the project

33701f4

Add a Bin SVG (used for deletion) and a Picture SVG (used for upload image button) to the project.

roro-lv force-pushed the feature/widget/roro-lv/manage-thumbnail branch from 5bf4a97 to 95940a1 Compare June 13, 2022 14:16

✨(front) update advertising to display uploaded thumbnail

7b94e05

Update advertising student screen, to display the uploaded thumbnail, when it exists.

roro-lv force-pushed the feature/widget/roro-lv/manage-thumbnail branch from 95940a1 to a1f9735 Compare June 13, 2022 14:18

roro-lv and others added 2 commits June 13, 2022 16:25

✨(front) add a widget for instructor to upload thumbnail

6a071fb

Add a widget to allow teacher to upload a thumbnail for its live. It also allows him to replace it or delete it and display the default thumbnail.

roro-lv force-pushed the feature/widget/roro-lv/manage-thumbnail branch from a1f9735 to 6a071fb Compare June 13, 2022 14:25

roro-lv merged commit cb59c0b into master Jun 13, 2022

roro-lv deleted the feature/widget/roro-lv/manage-thumbnail branch June 13, 2022 14:36

	size="0.875rem"
	style={{ fontFamily: 'Roboto-Regular' }}
	size="xsmall"


		let textMessage = null;

		if (thumbnailState === uploadState.ERROR) {

Add a widget for managing thumbnail #1568

Add a widget for managing thumbnail #1568

Conversation

roro-lv commented Jun 2, 2022 • edited Loading

Purpose

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

roro-lv Jun 8, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

roro-lv Jun 8, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RVassili commented Jun 8, 2022

lunika left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

roro-lv commented Jun 9, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

roro-lv commented Jun 2, 2022 •

edited

Loading

roro-lv Jun 8, 2022 •

edited

Loading

roro-lv Jun 8, 2022 •

edited

Loading