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

feat(plugin-video): allow video upload via new infra #4368

Merged
merged 18 commits into from
Feb 3, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
4 changes: 1 addition & 3 deletions e2e-tests/tests/430-multimedia-plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,7 @@ Scenario('Multimedia plugin toolbar controls', async ({ I }) => {
I.click('$plugin-multimedia-settings-button')
I.selectOption('$plugin-multimedia-type-select', 'Video')
I.click('$modal-close-button')
I.seeElement(
locate('$plugin-video-placeholder').inside('$plugin-multimedia-wrapper')
)
I.seeElement(locate('$plugin-video-src').inside('$plugin-multimedia-wrapper'))

I.say('Change the type of the multimedia content to GeoGebra')
I.click('$plugin-multimedia-settings-button')
Expand Down
12 changes: 3 additions & 9 deletions e2e-tests/tests/431-multimedia-plugin-image.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,7 @@ Scenario('Multimedia plugin successful image upload', ({ I }) => {
I.click('$plugin-multimedia-settings-button')
I.selectOption('$plugin-multimedia-type-select', 'Video')
I.click('$modal-close-button')
I.seeElement(
locate('$plugin-video-placeholder').inside('$plugin-multimedia-wrapper')
)
I.seeElement(locate('$plugin-video-src').inside('$plugin-multimedia-wrapper'))
I.click('$plugin-multimedia-settings-button')
I.selectOption('$plugin-multimedia-type-select', 'Bild')
I.click('$modal-close-button')
Expand Down Expand Up @@ -111,9 +109,7 @@ Scenario('Multimedia plugin valid image URL', ({ I }) => {
I.click('$plugin-multimedia-settings-button')
I.selectOption('$plugin-multimedia-type-select', 'Video')
I.click('$modal-close-button')
I.seeElement(
locate('$plugin-video-placeholder').inside('$plugin-multimedia-wrapper')
)
I.seeElement(locate('$plugin-video-src').inside('$plugin-multimedia-wrapper'))
I.click('$plugin-multimedia-settings-button')
I.selectOption('$plugin-multimedia-type-select', 'Bild')
I.click('$modal-close-button')
Expand Down Expand Up @@ -148,9 +144,7 @@ Scenario('Multimedia plugin fill in image caption', ({ I }) => {
I.click('$plugin-multimedia-settings-button')
I.selectOption('$plugin-multimedia-type-select', 'Video')
I.click('$modal-close-button')
I.seeElement(
locate('$plugin-video-placeholder').inside('$plugin-multimedia-wrapper')
)
I.seeElement(locate('$plugin-video-src').inside('$plugin-multimedia-wrapper'))
I.click('$plugin-multimedia-settings-button')
I.selectOption('$plugin-multimedia-type-select', 'Bild')
I.click('$modal-close-button')
Expand Down
26 changes: 18 additions & 8 deletions packages/editor/src/i18n/strings/de/edit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,17 +153,10 @@ export const editStrings = {
imageSource: 'Bildquelle',
imageSourceHelpText:
'Füge hier weitere Informationen wie den Urheber dieses Bildes hinzu.',
invalidImageUrl: 'Fehler: Ungültige oder unvollständige URL',
invalidImageUrlMessage:
'Die eingegebene URL ist entweder ungültig oder unvollständig. Bitte stelle sicher, dass du die vollständige URL korrekt kopiert und eingefügt hast. Die URL sollte mit "http://" oder "https://" beginnen.',
search: 'Suche',
searchOnline: 'Online nach lizenzfreien Bildern suchen',
placeholderSource: 'Quelle (optional)',
placeholderEmpty: 'https://example.com/image.png',
placeholderUploading: 'Wird hochgeladen …',
placeholderFailed: 'Hochladen fehlgeschlagen',
retry: 'Erneut versuchen',
failedUpload: 'Hochladen fehlgeschlagen',
captionPlaceholder: 'Bildunterschrift (optional)',
href: 'Link',
hrefPlaceholder: 'Bild verlinken',
Expand Down Expand Up @@ -229,7 +222,6 @@ export const editStrings = {
confirmRemoveAllMarks:
'Bist du sicher, dass du alle Aufgaben löschen willst?',
addOverlayContent: 'Aufgabe an aktueller Stelle einfügen',
addVideo: 'Füge ein Video hinzu (z.B. YouTube)',
changeVideo: 'Video austauschen',
saveInfo: 'Änderungen werden automatisch gespeichert!',
},
Expand Down Expand Up @@ -371,6 +363,10 @@ export const editStrings = {
titlePlaceholder: 'Titel',
url: 'URL',
seoTitle: 'Titel für Suchmaschinen',
upload: 'Video hochladen',
placeholderEmpty: 'YouTube- oder Serlo-URL',
change: 'Video ändern',
settings: 'Einstellungen',
},
audio: {
title: 'Audio',
Expand Down Expand Up @@ -522,6 +518,20 @@ export const editStrings = {
confirmRestore:
'Sicher, dass du deine Änderugen unwiderruflich löschen möchtest?',
},
fileUpload: {
placeholderUploading: 'Wird hochgeladen …',
placeholderFailed: 'Hochladen fehlgeschlagen',
retry: 'Erneut versuchen',
failedUpload: 'Hochladen fehlgeschlagen',
invalidUrl: 'Fehler: Ungültige oder unvollständige URL',
invalidUrlMessage:
'Die eingegebene URL ist entweder ungültig oder unvollständig. Bitte stelle sicher, dass du die vollständige URL korrekt kopiert und eingefügt hast. Die URL sollte mit "http://" oder "https://" beginnen.',
noFileSelected: 'Bitte wähle eine Datei aus',
badExtension:
"Sorry, %ext% ist leider nicht erlaubt. Versuch's mit diesen Typen: %allowed%",
fileTooBig:
"Sorry, diese Datei ist zu groß. Versuch's weniger als %maxsize% MB",
},
settings: 'Einstellungen',
extendedSettings: 'Erweiterte Einstellungen',
close: 'Schließen',
Expand Down
25 changes: 17 additions & 8 deletions packages/editor/src/i18n/strings/en/edit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,17 +149,10 @@ export const editStrings = {
imageUrl: 'Image URL',
imageSource: 'Image Source',
imageSourceHelpText: 'Add the author or source of this image here',
invalidImageUrl: 'Error: Invalid or Incomplete URL',
invalidImageUrlMessage:
'The URL you entered is either invalid or incomplete. Please ensure you have copied and pasted the full URL correctly. The URL should start with "http://" or "https://".',
search: 'Search',
searchOnline: 'Search online for licence-free images',
placeholderSource: 'Source (optional)',
placeholderEmpty: 'https://example.com/image.png',
placeholderUploading: 'Uploading…',
placeholderFailed: 'Upload failed…',
retry: 'Retry',
failedUpload: 'Upload failed',
captionPlaceholder: 'Optional caption',
href: 'Link',
hrefPlaceholder: 'Link the image',
Expand Down Expand Up @@ -218,7 +211,6 @@ export const editStrings = {
removeAllMarks: 'Remove all exercises',
confirmRemoveAllMarks: 'Are you sure you want to remove all exercises?',
addOverlayContent: 'Add exercise',
addVideo: 'Add a video url (e.g. YouTube) to get started',
changeVideo: 'Change video',
saveInfo: 'Changes are continually saved!',
},
Expand Down Expand Up @@ -358,6 +350,10 @@ export const editStrings = {
titlePlaceholder: 'Title',
url: 'URL',
seoTitle: 'Title for search engines',
upload: 'Upload Video',
placeholderEmpty: 'YouTube or Serlo URL',
change: 'Change video',
settings: 'Settings',
},
audio: {
title: 'Audio',
Expand Down Expand Up @@ -505,6 +501,19 @@ export const editStrings = {
restoreInitialButton: 'Delete changes',
confirmRestore: 'Are you sure you want to delete all your changes?',
},
fileUpload: {
placeholderUploading: 'Uploading…',
placeholderFailed: 'Upload failed…',
retry: 'Retry',
failedUpload: 'Upload failed',
invalidUrl: 'Error: Invalid or Incomplete URL',
invalidUrlMessage:
'The URL you entered is either invalid or incomplete. Please ensure you have copied and pasted the full URL correctly. The URL should start with "http://" or "https://".',
noFileSelected: 'Please select a file',
badExtension:
'Sorry, %ext% is not an accepted file type. Try one of: %allowed%',
fileTooBig: 'Sorry, this file is too big. Maximum size is %maxsize% MB',
},
settings: 'Settings',
extendedSettings: 'Extended Settings',
close: 'Close',
Expand Down
3 changes: 2 additions & 1 deletion packages/editor/src/package/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,8 @@ export { ScMcExerciseStaticRenderer } from '@editor/plugins/sc-mc-exercise/stati
// Exported for Solution Serlo static renderer
export { StaticSolutionRenderer } from '@editor/plugins/solution/static'
// Exported for Video Serlo static renderer
export { parseVideoUrl, VideoType } from '@editor/plugins/video/renderer'
export { VideoType } from '@editor/plugins/video/renderer'
export { parseVideoUrl } from '@editor/plugins/video/utils/parse-video-url'
export { VideoStaticRenderer } from '@editor/plugins/video/static'
// Exported for image-with-serlo-config plugin wrapper
export { createImagePlugin } from '@editor/plugins/image'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,19 @@ export function ImageSelectionScreen({
setIsAButtonFocused,
}: ImageSelectionScreenProps) {
const editorStrings = useEditStrings()
const uploadStrings = editorStrings.edtrIo.fileUpload
const imageStrings = editorStrings.plugins.image

const { src, licence } = state
const upload = useUploadFile(config.upload)

const imageStrings = editorStrings.plugins.image
const disableFileUpload = config.disableFileUpload // HACK: Temporary solution to make image plugin available in Moodle & Chancenwerk integration with file upload disabled.

const placeholder = !isTempFile(src.value)
? imageStrings.placeholderEmpty
: !src.value.failed
? imageStrings.placeholderUploading
: imageStrings.placeholderFailed
? uploadStrings.placeholderUploading
: uploadStrings.placeholderFailed

const imageUrl = src.value as string
const showErrorMessage = imageUrl.length > 5 && !isImageUrl(imageUrl)
Expand Down Expand Up @@ -115,9 +117,9 @@ export function ImageSelectionScreen({
className="mt-1 inline-block pl-1 text-sm font-semibold text-red-500"
data-qa="plugin-image-src-error"
>
{imageStrings.invalidImageUrl}
{uploadStrings.invalidUrl}
</span>
<EditorTooltip text={imageStrings.invalidImageUrlMessage} />
<EditorTooltip text={uploadStrings.invalidUrlMessage} />
</>
)}
</span>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { LicenseDropdown } from '../components/licence-dropdown'
export function SettingsModalControls({ state }: Pick<ImageProps, 'state'>) {
const { alt, src, imageSource, licence } = state
const imageStrings = useEditStrings().plugins.image
const uploadStrings = useEditStrings().edtrIo.fileUpload

const isTemp = isTempFile(src.value)
const isFailed = isTempFile(src.value) && src.value.failed
Expand All @@ -29,8 +30,8 @@ export function SettingsModalControls({ state }: Pick<ImageProps, 'state'>) {
!isTemp
? imageStrings.placeholderEmpty
: isFailed
? imageStrings.placeholderFailed
: imageStrings.placeholderUploading
? uploadStrings.placeholderFailed
: uploadStrings.placeholderUploading
}
// eslint-disable-next-line @typescript-eslint/no-base-to-string
value={isTemp ? '' : src.value.toString()}
Expand Down
5 changes: 3 additions & 2 deletions packages/editor/src/plugins/image/controls/upload-button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export function UploadButton({
onFocus,
onBlur,
}: UploadButtonProps) {
const uploadStrings = useEditStrings().edtrIo.fileUpload
const imageStrings = useEditStrings().plugins.image
const isFailed = isTempFile(src.value) && src.value.failed

Expand Down Expand Up @@ -60,7 +61,7 @@ export function UploadButton({
<input
type="file"
multiple={!!config.onMultipleUpload}
accept="image/*"
accept="image/*,.gif,.jpg,.jpeg,.png,.svg,.webp"
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious, why was this necessary? In theory, image/* should cover all of these added types.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good question. Now that I think of it .gif,.jpg,.jpeg,.png,.svg,.webp would be better, since we then exclude unwanted image types like bmp or tiff.

Copy link
Member Author

Choose a reason for hiding this comment

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

(docs)

className="sr-only"
onChange={({ target }) => {
if (target.files && target.files.length) {
Expand All @@ -87,7 +88,7 @@ export function UploadButton({
onClick={() => src.upload((src.value as TempFile).failed!, upload)}
data-qa="plugin-image-retry"
>
<EditorTooltip text={imageStrings.retry} className="top-10" />
<EditorTooltip text={uploadStrings.retry} className="top-10" />
<FaIcon icon={faRedoAlt} />
</button>
) : null}
Expand Down
16 changes: 10 additions & 6 deletions packages/editor/src/plugins/image/utils/upload-file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,22 @@ import {
type EditorMeta,
} from '@editor/core/contexts/editor-meta-context'
import { useIsSerlo } from '@editor/core/hooks/use-is-serlo'
import { useEditStrings } from '@editor/i18n/edit-strings-provider'
import { type UploadHandler } from '@editor/plugin'
import { EditStrings } from '@editor/types/language-data'
import { useContext } from 'react'

import { handleError, validateFile } from './validate-file'

type UploadMeta = Pick<EditorMeta, 'editorVariant' | 'userId'>

export function useUploadFile(oldUploader: UploadHandler<string>) {
export function useUploadFile(oldUploader?: UploadHandler<string>) {
const { editorVariant, userId } = useContext(EditorMetaContext)
const isSerlo = useIsSerlo()

const uploadStrings = useEditStrings().edtrIo.fileUpload
const uploader = (file: File) =>
uploadFile({ file, editorVariant, userId, isSerlo })
return shouldUseNewUpload(isSerlo) ? uploader : oldUploader
uploadFile({ file, editorVariant, userId, isSerlo, uploadStrings })
return shouldUseNewUpload(isSerlo) ? uploader : oldUploader!
}

function shouldUseNewUpload(isSerlo: boolean) {
Expand All @@ -37,15 +39,17 @@ function shouldUseNewUpload(isSerlo: boolean) {
}

async function uploadFile({
isSerlo,
file,
editorVariant,
userId,
isSerlo,
uploadStrings,
}: UploadMeta & {
file: File
uploadStrings: EditStrings['edtrIo']['fileUpload']
isSerlo: boolean
}) {
const validated = validateFile(file)
const validated = validateFile(file, uploadStrings)
if (!validated) return Promise.reject()

const parentHost = getParentHost()
Expand Down
52 changes: 41 additions & 11 deletions packages/editor/src/plugins/image/utils/validate-file.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { showToastNotice } from '@editor/editor-ui/show-toast-notice'
import { EditStrings } from '@editor/types/language-data'

export enum FileErrorCode {
TOO_MANY_FILES,
Expand All @@ -12,34 +13,63 @@ export interface FileError {
errorCode: FileErrorCode
message: string
}
const maxFileSize = 2 * 1024 * 1024
const allowedExtensions = ['gif', 'jpg', 'jpeg', 'png', 'svg', 'webp']
const maxImageFileSize = 2 * 1024 * 1024
const maxVideoFileSize = 16 * 1024 * 1024
const allowedExtensions = [
'gif',
'jpg',
'jpeg',
'png',
'svg',
'webp',
'webm',
'mp4',
]

export function validateFile(file: File) {
// TODO: i18n and make error messages actually helpful
export function validateFile(
file: File,
uploadStrings: EditStrings['edtrIo']['fileUpload']
) {
if (!file) {
handleError('No file selected')
handleError(uploadStrings.noFileSelected)
return false
}
if (!matchesAllowedExtensions(file.name)) {
handleError('Not an accepted file type')

const extension = file.name
.toLowerCase()
.slice(file.name.lastIndexOf('.') + 1)

if (!matchesAllowedExtensions(extension)) {
handleError(
uploadStrings.badExtension
.replace('%ext%', extension)
.replace('%allowed%', allowedExtensions.join(', '))
)
return false
}
const maxFileSize = file.type.startsWith('video/')
? maxVideoFileSize
: maxImageFileSize

if (file.size > maxFileSize) {
handleError('File is too big')
handleError(
uploadStrings.fileTooBig.replace(
'%maxsize%',
String(maxFileSize / 1024 / 1024)
)
)
return false
}

return true
}

function matchesAllowedExtensions(fileName: string) {
const extension = fileName.toLowerCase().slice(fileName.lastIndexOf('.') + 1)
function matchesAllowedExtensions(extension: string) {
return allowedExtensions.includes(extension)
}

export function handleError(message: string) {
// eslint-disable-next-line no-console
console.error(message)
showToastNotice(message, 'warning')
showToastNotice('⚠️ ' + message, 'warning')
}
Loading