Support additional embedded blocks#359
Merged
Merged
Conversation
Reviewer's GuideAdds new Audio, Google Drive, Gallery, and Link Preview embedded blocks that can be inserted from the slash menu and configured via block popovers, and introduces a deferred popover anchoring mechanism to handle newly-mounted blocks while hardening Google Drive URL parsing/embedding. Sequence diagram for deferred block popover anchoring for new embedded blockssequenceDiagram
actor User
participant SlashPanel
participant Editor
participant BlockPopoverProvider
participant Element
participant EmbeddedBlock as EmbeddedBlock(AudioBlock/GalleryBlock/LinkPreview/GoogleDriveBlock)
participant BlockPopover
User->>SlashPanel: select slash item (e.g. Audio / Gallery / Bookmark / Google Drive)
SlashPanel->>Editor: turnInto(type, initialData)
Note over Editor: New block with blockId is added to document
SlashPanel->>BlockPopoverProvider: openPopover(newBlockId, type)
BlockPopoverProvider->>BlockPopoverProvider: resolveAnchor(newBlockId)
alt block not yet mounted
BlockPopoverProvider->>BlockPopoverProvider: pendingRef.current = { blockId, type }
else block already mounted
BlockPopoverProvider->>BlockPopoverProvider: setBlockId, setType, setAnchorEl(dom)
BlockPopoverProvider-->>BlockPopover: context updated (open = true)
end
Editor-->>Element: render Element for blockId
Element->>Element: usePopoverMountSignal(blockId)
Element->>BlockPopoverProvider: notifyMount(blockId)
BlockPopoverProvider->>BlockPopoverProvider: const pending = pendingRef.current
alt pending matches mounted blockId
BlockPopoverProvider->>BlockPopoverProvider: resolveAnchor(pending.blockId)
BlockPopoverProvider->>BlockPopoverProvider: setBlockId, setType, setAnchorEl(dom)
BlockPopoverProvider-->>BlockPopover: context updated (open = true)
end
BlockPopover->>BlockPopover: render content by type
BlockPopover-->>EmbeddedBlock: e.g. AudioBlockPopoverContent / GalleryBlockPopoverContent / LinkPreviewPopoverContent / GoogleDriveBlockPopoverContent
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 4 issues, and left some high level feedback:
- In
google-drive-utils.ts,isGoogleDriveUrlaccepts hosts likesheets.google.comandslides.google.com, butbuildGoogleDriveEmbeddedUrlonly has special handling fordocs.google.comanddrive.google.com; consider either normalizing those hosts todocs.google.comor aligning the validation/embedding behavior so accepted URLs are always embedded in a consistent way. - In
AudioBlock.tsx, clicks on the block are always given acursor-pointerwhenhasContentor!readOnly, but in read-only mode with content the click handler is effectively a no-op (stops at the audio tag); consider adjusting the cursor or click behavior to better reflect interactivity in read-only views.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `google-drive-utils.ts`, `isGoogleDriveUrl` accepts hosts like `sheets.google.com` and `slides.google.com`, but `buildGoogleDriveEmbeddedUrl` only has special handling for `docs.google.com` and `drive.google.com`; consider either normalizing those hosts to `docs.google.com` or aligning the validation/embedding behavior so accepted URLs are always embedded in a consistent way.
- In `AudioBlock.tsx`, clicks on the block are always given a `cursor-pointer` when `hasContent` or `!readOnly`, but in read-only mode with content the click handler is effectively a no-op (stops at the audio tag); consider adjusting the cursor or click behavior to better reflect interactivity in read-only views.
## Individual Comments
### Comment 1
<location path="src/components/editor/components/blocks/audio/AudioBlock.tsx" line_range="43-52" />
<code_context>
+export function usePopoverMountSignal(blockId: string | undefined) {
+ const { notifyMount } = usePopoverContext();
+
+ useEffect(() => {
+ if (!blockId) return;
+ notifyMount(blockId);
</code_context>
<issue_to_address>
**issue (bug_risk):** In read-only mode, pending local audio files are not surfaced, so unfinished uploads become invisible to viewers.
Because the effect that restores pending local files exits when `readOnly` is true, `localUrl` and `needRetry` are never set in read-only views. If an upload failed or is pending and only `retry_local_url` exists, viewers get an empty block with no audio or retry state. It would be better to always run this effect so `localUrl`/`needRetry` are derived, and instead gate only the mutating handlers (e.g. `handleRetry`, `handleLoadedMetadata`) on `readOnly`, so read-only views can still show or play cached audio and display failures correctly.
</issue_to_address>
### Comment 2
<location path="src/components/editor/components/blocks/audio/AudioBlock.tsx" line_range="76-85" />
<code_context>
+ const handleRetry = useCallback(
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Retry logic silently bails out when the local cache is missing, leaving the block stuck in a failed state.
In `handleRetry`, when `getStoredFile(retry_local_url)` returns `undefined`, we just show another generic `uploadFailed` toast and leave the block in the same failed state with the retry button. If the cached file was evicted, the user has no path forward. Please either clear `needRetry`/`retry_local_url` and prompt for a fresh upload, or show a specific message that the file is no longer available with an option to re-select it, so the block doesn’t get stuck permanently.
Suggested implementation:
```typescript
const fileData = await fileHandlerRef.current.getStoredFile(retry_local_url);
const file = fileData?.file;
if (!file) {
// Cached file is gone: clear retry state and prompt user to re-upload
setNeedRetry(false);
setRetryLocalUrl(undefined);
toast({
status: 'error',
title: t('audioBlock.retryFileMissingTitle', 'Original file no longer available'),
description: t(
'audioBlock.retryFileMissingDescription',
'The previously uploaded audio file is no longer available. Please select the file again to upload.'
),
});
setLoading(false);
return;
}
```
1. Ensure `setNeedRetry` and `setRetryLocalUrl` are defined state setters in this component and that `retry_local_url` is nullable (e.g. `string | undefined`).
2. If the component uses a different toast/notification mechanism than `toast({ ... })`, adapt the call accordingly and ensure `toast` is imported/initialized (e.g. via `const toast = useToast()`).
3. Make sure `t` is available in scope (from your i18n solution) or replace `t(...)` with hard-coded strings or your existing translation helper.
4. Optionally, if you have a "select file" action (e.g. `openFilePicker()`), you could call it right after clearing retry state to streamline the re-upload flow.
</issue_to_address>
### Comment 3
<location path="src/components/editor/components/blocks/audio/AudioBlock.tsx" line_range="20" />
<code_context>
+import { constructFileUrl } from '@/components/editor/utils/file-url';
+import { FileHandler } from '@/utils/file';
+
+export const AudioBlock = memo(
+ forwardRef<HTMLDivElement, EditorElementProps<AudioBlockNode>>(({ node, children, ...attributes }, ref) => {
+ const { t } = useTranslation();
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting the upload/retry logic, header UI, and optional toolbar hover wiring from `AudioBlock` into separate hooks/components to keep the main block focused on composition and rendering.
You can reduce the complexity of `AudioBlock` by extracting the upload/retry plumbing and header rendering into small, focused units. This keeps behavior intact while making the main component mostly about rendering.
### 1. Extract upload/retry logic into a hook
Move `localUrl`, `needRetry`, `loading`, the `useEffect` for `retry_local_url`, and `handleRetry` into a reusable hook:
```ts
// usePendingCloudUpload.ts
import { useCallback, useEffect, useRef, useState } from 'react';
import { FileHandler } from '@/utils/file';
import { notify } from '@/components/_shared/notify';
import { CustomEditor } from '@/application/slate-yjs/command';
import { AudioBlockData, AudioUrlType } from '@/application/types';
export function usePendingCloudUpload({
readOnly,
dataUrl,
retryLocalUrl,
name,
editor,
blockId,
uploadFileRemote,
t,
}: {
readOnly: boolean;
dataUrl?: string;
retryLocalUrl?: string;
name?: string;
editor: any;
blockId: string;
uploadFileRemote?: (file: File) => Promise<string | undefined>;
t: (key: string, opts?: any) => string;
}) {
const fileHandlerRef = useRef(new FileHandler());
const [localUrl, setLocalUrl] = useState<string | undefined>();
const [needRetry, setNeedRetry] = useState(false);
const [loading, setLoading] = useState(false);
useEffect(() => {
if (readOnly) return;
void (async () => {
if (!retryLocalUrl || dataUrl) {
setLocalUrl(undefined);
setNeedRetry(false);
return;
}
const fileData = await fileHandlerRef.current.getStoredFile(retryLocalUrl);
setLocalUrl(fileData?.url);
setNeedRetry(!!fileData);
})();
}, [dataUrl, readOnly, retryLocalUrl]);
const handleRetry = useCallback(
async (e: React.MouseEvent) => {
e.stopPropagation();
if (!retryLocalUrl) return;
setLoading(true);
try {
const fileData = await fileHandlerRef.current.getStoredFile(retryLocalUrl);
const file = fileData?.file;
if (!file) {
notify.error(t('web.fileBlock.uploadFailed'));
return;
}
const url = await uploadFileRemote?.(file);
if (!url) {
notify.error(t('web.fileBlock.uploadFailed'));
return;
}
await fileHandlerRef.current.cleanup(retryLocalUrl);
CustomEditor.setBlockData(editor, blockId, {
url,
name,
uploaded_at: Date.now(),
url_type: AudioUrlType.Cloud,
retry_local_url: '',
pending_upload_id: '',
} as AudioBlockData);
} finally {
setLoading(false);
}
},
[blockId, editor, name, retryLocalUrl, t, uploadFileRemote]
);
return { localUrl, needRetry, loading, handleRetry };
}
```
Then `AudioBlock` becomes simpler:
```tsx
const { url: dataUrl, name, retry_local_url, duration_in_second } = data || {};
const uploadFileRemote = useCallback(
async (file: File) => {
try {
return await uploadFile?.(file);
} catch {
return undefined;
}
},
[uploadFile]
);
const { localUrl, needRetry, loading, handleRetry } = usePendingCloudUpload({
readOnly,
dataUrl,
retryLocalUrl: retry_local_url,
name,
editor,
blockId,
uploadFileRemote,
t,
});
```
This removes the `FileHandler` ref, `useEffect`, and most retry-related state from the component.
### 2. Extract a small header component
Pull the header (icon + title + error + retry UI) into a presentational component to remove branching from the main JSX:
```tsx
// AudioBlockHeader.tsx
import { CircularProgress, IconButton, Tooltip } from '@mui/material';
import { ReactComponent as AudioIcon } from '@/assets/icons/audio.svg';
import { ReactComponent as ReloadIcon } from '@/assets/icons/regenerate.svg';
export function AudioBlockHeader({
hasContent,
name,
needRetry,
loading,
onRetry,
t,
emptyRef,
}: {
hasContent: boolean;
name?: string;
needRetry: boolean;
loading: boolean;
onRetry: (e: React.MouseEvent) => void;
t: (key: string, opts?: any) => string;
emptyRef: React.RefObject<HTMLDivElement>;
}) {
return (
<div className="flex w-full items-center gap-3">
<AudioIcon className="h-6 w-6 flex-none" />
<div ref={emptyRef} className="min-w-0 flex-1 text-base font-medium">
{hasContent ? (
<div className="truncate">
{name?.trim() || t('document.selectionMenu.audio', { defaultValue: 'Audio' })}
</div>
) : (
<div className="text-text-secondary">
{t('document.plugins.audio.addAudio', { defaultValue: 'Upload or embed audio' })}
</div>
)}
{needRetry && (
<div className="text-sm font-normal text-function-error">
{t('web.fileBlock.uploadFailed')}
</div>
)}
</div>
{needRetry &&
(loading ? (
<CircularProgress size={16} />
) : (
<Tooltip placement="top" title={t('web.fileBlock.retry')}>
<IconButton onClick={onRetry} size="small" color="error">
<ReloadIcon />
</IconButton>
</Tooltip>
))}
</div>
);
}
```
Usage in `AudioBlock`:
```tsx
<AudioBlockHeader
hasContent={hasContent}
name={name}
needRetry={needRetry}
loading={loading}
onRetry={handleRetry}
t={t}
emptyRef={emptyRef}
/>
```
### 3. Optional: reuse toolbar wiring
If `FileToolbar` is used similarly in other blocks, wrap the `showToolbar` hover logic and `remoteUrl` mapping in a tiny helper/hook:
```tsx
// useFileToolbar.ts
import { useState, useCallback } from 'react';
export function useFileToolbar(hasContent: boolean) {
const [showToolbar, setShowToolbar] = useState(false);
const onMouseEnter = useCallback(() => {
if (hasContent) setShowToolbar(true);
}, [hasContent]);
const onMouseLeave = useCallback(() => setShowToolbar(false), []);
return { showToolbar, onMouseEnter, onMouseLeave };
}
```
Then in `AudioBlock`:
```tsx
const { showToolbar, onMouseEnter, onMouseLeave } = useFileToolbar(hasContent);
<div
{...attributes}
onMouseEnter={onMouseEnter}
onMouseLeave={onMouseLeave}
// ...
>
{/* ... */}
{showToolbar && remoteUrl && (
<FileToolbar
node={
{
...node,
data: { ...data, url: remoteUrl },
} as unknown as FileNode
}
/>
)}
</div>
```
These extractions keep all behavior but make `AudioBlock` mostly compose existing pieces, reducing cognitive load and making the upload/retry logic reusable across `FileBlock`/`ImageBlock`.
</issue_to_address>
### Comment 4
<location path="src/components/editor/components/block-popover/AudioBlockPopoverContent.tsx" line_range="37" />
<code_context>
+ return AUDIO_EXTENSION_REGEX.test(url);
+}
+
+function AudioBlockPopoverContent({ blockId, onClose }: { blockId: string; onClose: () => void }) {
+ const editor = useSlateStatic() as YjsEditor;
+ const { uploadFile } = useEditorContext();
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting the multi-file upload orchestration from `AudioBlockPopoverContent` into a reusable hook so the component focuses on audio-specific UI and wiring only.
You can reduce the cognitive load in `AudioBlockPopoverContent` by extracting the multi‑file upload orchestration into a dedicated hook and keeping the popover focused on UI + wiring. That will also make it reusable for image/video, etc.
### 1. Extract upload orchestration into a hook
Move the generic bits (`createPendingAudioData`, `cleanupLocalFile`, `uploadIntoAudioBlock`, `handleChangeUploadFiles`) into a hook that is parameterized by how to build/update block data.
```ts
// useMultiBlockUpload.ts
import { useCallback } from 'react';
import { CustomEditor } from '@/application/slate-yjs/command';
import { BlockType } from '@/application/types';
import { FileHandler } from '@/utils/file';
type PendingData<T> = T & { pending_upload_id?: string; retry_local_url?: string };
interface UseMultiBlockUploadParams<T> {
editor: YjsEditor;
blockId: string;
blockType: BlockType;
uploadFileRemote: (file: File) => Promise<string | undefined>;
createPendingData: (file: File) => Promise<PendingData<T>>;
finalizeData: (file: File, url: string) => T;
}
export function useMultiBlockUpload<T>({
editor,
blockId,
blockType,
uploadFileRemote,
createPendingData,
finalizeData,
}: UseMultiBlockUploadParams<T>) {
const cleanupLocalFile = useCallback(async (retryLocalUrl?: string) => {
if (!retryLocalUrl) return;
const fileHandler = new FileHandler();
await fileHandler.cleanup(retryLocalUrl).catch(() => undefined);
}, []);
const uploadIntoBlock = useCallback(
async (targetBlockId: string, file: File, pendingData: PendingData<T>) => {
const url = await uploadFileRemote(file);
if (!url) return;
await cleanupLocalFile(pendingData.retry_local_url);
let currentData: PendingData<T> | undefined;
try {
const entry = findSlateEntryByBlockId(editor, targetBlockId);
currentData = entry ? (entry[0] as { data?: PendingData<T> }).data ?? undefined : undefined;
} catch {
return;
}
if (!currentData?.pending_upload_id) return;
if (currentData.pending_upload_id !== pendingData.pending_upload_id) return;
if ((currentData as any).url) return;
CustomEditor.setBlockData(editor, targetBlockId, finalizeData(file, url));
},
[cleanupLocalFile, editor, uploadFileRemote, finalizeData],
);
const handleChangeUploadFiles = useCallback(
async (files: File[]) => {
if (!files.length) return;
const [primaryData, ...otherDatas] = await Promise.all(files.map((f) => createPendingData(f)));
const [file, ...otherFiles] = files;
CustomEditor.setBlockData(editor, blockId, primaryData as any);
const pendingUploads: Promise<void>[] = [uploadIntoBlock(blockId, file, primaryData)];
const reversedPairs = otherFiles.map((f, i) => [f, otherDatas[i]] as const).reverse();
for (const [f, data] of reversedPairs) {
const newId = CustomEditor.addBelowBlock(editor, blockId, blockType, data as any);
if (newId) pendingUploads.push(uploadIntoBlock(newId, f, data));
}
await Promise.all(pendingUploads);
},
[blockId, createPendingData, uploadIntoBlock, editor, blockType],
);
return { handleChangeUploadFiles };
}
```
### 2. Keep `AudioBlockPopoverContent` focused on audio specifics + UI
With the hook, the popover becomes mostly wiring + audio‑specific helpers:
```tsx
function AudioBlockPopoverContent({ blockId, onClose }: { blockId: string; onClose: () => void }) {
const editor = useSlateStatic() as YjsEditor;
const { uploadFile } = useEditorContext();
const { t } = useTranslation();
const [tabValue, setTabValue] = React.useState('upload');
const [uploading, setUploading] = React.useState(false);
const uploadFileRemote = useCallback(
async (file: File) => {
try {
return await uploadFile?.(file);
} catch {
return undefined;
}
},
[uploadFile],
);
const createPendingAudioData = useCallback(async (file: File): Promise<AudioBlockData> => {
const data: AudioBlockData = {
url: undefined,
name: file.name,
uploaded_at: Date.now(),
url_type: AudioUrlType.Cloud,
pending_upload_id: createPendingUploadId(),
};
try {
const fileHandler = new FileHandler();
const res = await fileHandler.handleFileUpload(file);
URL.revokeObjectURL(res.url);
data.retry_local_url = res.id;
} catch {
data.retry_local_url = '';
}
return data;
}, []);
const finalizeAudioData = useCallback(
(file: File, url: string): AudioBlockData => ({
url,
name: file.name,
uploaded_at: Date.now(),
url_type: AudioUrlType.Cloud,
retry_local_url: '',
pending_upload_id: '',
}),
[],
);
const { handleChangeUploadFiles } = useMultiBlockUpload<AudioBlockData>({
editor,
blockId,
blockType: BlockType.AudioBlock,
uploadFileRemote,
createPendingData: createPendingAudioData,
finalizeData: finalizeAudioData,
});
const handleTabChange = useCallback((_e: React.SyntheticEvent, newValue: string) => {
setTabValue(newValue);
}, []);
// wrap uploaded handler for UI state & closing
const handleUpload = useCallback(
async (files: File[]) => {
if (!files.length) return;
setUploading(true);
try {
await handleChangeUploadFiles(files);
onClose();
} finally {
setUploading(false);
}
},
[handleChangeUploadFiles, onClose],
);
// ...embed link + UI unchanged, just use handleUpload for FileDropzone.onChange
}
```
This keeps:
- All existing behavior (pending upload id guard, reverse insertion, cleanup).
- Audio specifics (types, `createPendingUploadId`, `AudioUrlType`, `getAudioName`) in the audio component.
- The multi‑file upload orchestration in one reusable, testable unit.
It also flattens the nested async callbacks in `AudioBlockPopoverContent` into one `handleUpload` call, making the popover much easier to read.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+43
to
+52
| useEffect(() => { | ||
| if (readOnly) return; | ||
| void (async () => { | ||
| if (!retry_local_url || dataUrl) { | ||
| setLocalUrl(undefined); | ||
| setNeedRetry(false); | ||
| return; | ||
| } | ||
|
|
||
| const fileData = await fileHandlerRef.current.getStoredFile(retry_local_url); |
There was a problem hiding this comment.
issue (bug_risk): In read-only mode, pending local audio files are not surfaced, so unfinished uploads become invisible to viewers.
Because the effect that restores pending local files exits when readOnly is true, localUrl and needRetry are never set in read-only views. If an upload failed or is pending and only retry_local_url exists, viewers get an empty block with no audio or retry state. It would be better to always run this effect so localUrl/needRetry are derived, and instead gate only the mutating handlers (e.g. handleRetry, handleLoadedMetadata) on readOnly, so read-only views can still show or play cached audio and display failures correctly.
Comment on lines
+76
to
+85
| const handleRetry = useCallback( | ||
| async (e: React.MouseEvent) => { | ||
| e.stopPropagation(); | ||
| if (!retry_local_url) return; | ||
|
|
||
| setLoading(true); | ||
| try { | ||
| const fileData = await fileHandlerRef.current.getStoredFile(retry_local_url); | ||
| const file = fileData?.file; | ||
|
|
There was a problem hiding this comment.
suggestion (bug_risk): Retry logic silently bails out when the local cache is missing, leaving the block stuck in a failed state.
In handleRetry, when getStoredFile(retry_local_url) returns undefined, we just show another generic uploadFailed toast and leave the block in the same failed state with the retry button. If the cached file was evicted, the user has no path forward. Please either clear needRetry/retry_local_url and prompt for a fresh upload, or show a specific message that the file is no longer available with an option to re-select it, so the block doesn’t get stuck permanently.
Suggested implementation:
const fileData = await fileHandlerRef.current.getStoredFile(retry_local_url);
const file = fileData?.file;
if (!file) {
// Cached file is gone: clear retry state and prompt user to re-upload
setNeedRetry(false);
setRetryLocalUrl(undefined);
toast({
status: 'error',
title: t('audioBlock.retryFileMissingTitle', 'Original file no longer available'),
description: t(
'audioBlock.retryFileMissingDescription',
'The previously uploaded audio file is no longer available. Please select the file again to upload.'
),
});
setLoading(false);
return;
}- Ensure
setNeedRetryandsetRetryLocalUrlare defined state setters in this component and thatretry_local_urlis nullable (e.g.string | undefined). - If the component uses a different toast/notification mechanism than
toast({ ... }), adapt the call accordingly and ensuretoastis imported/initialized (e.g. viaconst toast = useToast()). - Make sure
tis available in scope (from your i18n solution) or replacet(...)with hard-coded strings or your existing translation helper. - Optionally, if you have a "select file" action (e.g.
openFilePicker()), you could call it right after clearing retry state to streamline the re-upload flow.
| import { constructFileUrl } from '@/components/editor/utils/file-url'; | ||
| import { FileHandler } from '@/utils/file'; | ||
|
|
||
| export const AudioBlock = memo( |
There was a problem hiding this comment.
issue (complexity): Consider extracting the upload/retry logic, header UI, and optional toolbar hover wiring from AudioBlock into separate hooks/components to keep the main block focused on composition and rendering.
You can reduce the complexity of AudioBlock by extracting the upload/retry plumbing and header rendering into small, focused units. This keeps behavior intact while making the main component mostly about rendering.
1. Extract upload/retry logic into a hook
Move localUrl, needRetry, loading, the useEffect for retry_local_url, and handleRetry into a reusable hook:
// usePendingCloudUpload.ts
import { useCallback, useEffect, useRef, useState } from 'react';
import { FileHandler } from '@/utils/file';
import { notify } from '@/components/_shared/notify';
import { CustomEditor } from '@/application/slate-yjs/command';
import { AudioBlockData, AudioUrlType } from '@/application/types';
export function usePendingCloudUpload({
readOnly,
dataUrl,
retryLocalUrl,
name,
editor,
blockId,
uploadFileRemote,
t,
}: {
readOnly: boolean;
dataUrl?: string;
retryLocalUrl?: string;
name?: string;
editor: any;
blockId: string;
uploadFileRemote?: (file: File) => Promise<string | undefined>;
t: (key: string, opts?: any) => string;
}) {
const fileHandlerRef = useRef(new FileHandler());
const [localUrl, setLocalUrl] = useState<string | undefined>();
const [needRetry, setNeedRetry] = useState(false);
const [loading, setLoading] = useState(false);
useEffect(() => {
if (readOnly) return;
void (async () => {
if (!retryLocalUrl || dataUrl) {
setLocalUrl(undefined);
setNeedRetry(false);
return;
}
const fileData = await fileHandlerRef.current.getStoredFile(retryLocalUrl);
setLocalUrl(fileData?.url);
setNeedRetry(!!fileData);
})();
}, [dataUrl, readOnly, retryLocalUrl]);
const handleRetry = useCallback(
async (e: React.MouseEvent) => {
e.stopPropagation();
if (!retryLocalUrl) return;
setLoading(true);
try {
const fileData = await fileHandlerRef.current.getStoredFile(retryLocalUrl);
const file = fileData?.file;
if (!file) {
notify.error(t('web.fileBlock.uploadFailed'));
return;
}
const url = await uploadFileRemote?.(file);
if (!url) {
notify.error(t('web.fileBlock.uploadFailed'));
return;
}
await fileHandlerRef.current.cleanup(retryLocalUrl);
CustomEditor.setBlockData(editor, blockId, {
url,
name,
uploaded_at: Date.now(),
url_type: AudioUrlType.Cloud,
retry_local_url: '',
pending_upload_id: '',
} as AudioBlockData);
} finally {
setLoading(false);
}
},
[blockId, editor, name, retryLocalUrl, t, uploadFileRemote]
);
return { localUrl, needRetry, loading, handleRetry };
}Then AudioBlock becomes simpler:
const { url: dataUrl, name, retry_local_url, duration_in_second } = data || {};
const uploadFileRemote = useCallback(
async (file: File) => {
try {
return await uploadFile?.(file);
} catch {
return undefined;
}
},
[uploadFile]
);
const { localUrl, needRetry, loading, handleRetry } = usePendingCloudUpload({
readOnly,
dataUrl,
retryLocalUrl: retry_local_url,
name,
editor,
blockId,
uploadFileRemote,
t,
});This removes the FileHandler ref, useEffect, and most retry-related state from the component.
2. Extract a small header component
Pull the header (icon + title + error + retry UI) into a presentational component to remove branching from the main JSX:
// AudioBlockHeader.tsx
import { CircularProgress, IconButton, Tooltip } from '@mui/material';
import { ReactComponent as AudioIcon } from '@/assets/icons/audio.svg';
import { ReactComponent as ReloadIcon } from '@/assets/icons/regenerate.svg';
export function AudioBlockHeader({
hasContent,
name,
needRetry,
loading,
onRetry,
t,
emptyRef,
}: {
hasContent: boolean;
name?: string;
needRetry: boolean;
loading: boolean;
onRetry: (e: React.MouseEvent) => void;
t: (key: string, opts?: any) => string;
emptyRef: React.RefObject<HTMLDivElement>;
}) {
return (
<div className="flex w-full items-center gap-3">
<AudioIcon className="h-6 w-6 flex-none" />
<div ref={emptyRef} className="min-w-0 flex-1 text-base font-medium">
{hasContent ? (
<div className="truncate">
{name?.trim() || t('document.selectionMenu.audio', { defaultValue: 'Audio' })}
</div>
) : (
<div className="text-text-secondary">
{t('document.plugins.audio.addAudio', { defaultValue: 'Upload or embed audio' })}
</div>
)}
{needRetry && (
<div className="text-sm font-normal text-function-error">
{t('web.fileBlock.uploadFailed')}
</div>
)}
</div>
{needRetry &&
(loading ? (
<CircularProgress size={16} />
) : (
<Tooltip placement="top" title={t('web.fileBlock.retry')}>
<IconButton onClick={onRetry} size="small" color="error">
<ReloadIcon />
</IconButton>
</Tooltip>
))}
</div>
);
}Usage in AudioBlock:
<AudioBlockHeader
hasContent={hasContent}
name={name}
needRetry={needRetry}
loading={loading}
onRetry={handleRetry}
t={t}
emptyRef={emptyRef}
/>3. Optional: reuse toolbar wiring
If FileToolbar is used similarly in other blocks, wrap the showToolbar hover logic and remoteUrl mapping in a tiny helper/hook:
// useFileToolbar.ts
import { useState, useCallback } from 'react';
export function useFileToolbar(hasContent: boolean) {
const [showToolbar, setShowToolbar] = useState(false);
const onMouseEnter = useCallback(() => {
if (hasContent) setShowToolbar(true);
}, [hasContent]);
const onMouseLeave = useCallback(() => setShowToolbar(false), []);
return { showToolbar, onMouseEnter, onMouseLeave };
}Then in AudioBlock:
const { showToolbar, onMouseEnter, onMouseLeave } = useFileToolbar(hasContent);
<div
{...attributes}
onMouseEnter={onMouseEnter}
onMouseLeave={onMouseLeave}
// ...
>
{/* ... */}
{showToolbar && remoteUrl && (
<FileToolbar
node={
{
...node,
data: { ...data, url: remoteUrl },
} as unknown as FileNode
}
/>
)}
</div>These extractions keep all behavior but make AudioBlock mostly compose existing pieces, reducing cognitive load and making the upload/retry logic reusable across FileBlock/ImageBlock.
| return AUDIO_EXTENSION_REGEX.test(url); | ||
| } | ||
|
|
||
| function AudioBlockPopoverContent({ blockId, onClose }: { blockId: string; onClose: () => void }) { |
There was a problem hiding this comment.
issue (complexity): Consider extracting the multi-file upload orchestration from AudioBlockPopoverContent into a reusable hook so the component focuses on audio-specific UI and wiring only.
You can reduce the cognitive load in AudioBlockPopoverContent by extracting the multi‑file upload orchestration into a dedicated hook and keeping the popover focused on UI + wiring. That will also make it reusable for image/video, etc.
1. Extract upload orchestration into a hook
Move the generic bits (createPendingAudioData, cleanupLocalFile, uploadIntoAudioBlock, handleChangeUploadFiles) into a hook that is parameterized by how to build/update block data.
// useMultiBlockUpload.ts
import { useCallback } from 'react';
import { CustomEditor } from '@/application/slate-yjs/command';
import { BlockType } from '@/application/types';
import { FileHandler } from '@/utils/file';
type PendingData<T> = T & { pending_upload_id?: string; retry_local_url?: string };
interface UseMultiBlockUploadParams<T> {
editor: YjsEditor;
blockId: string;
blockType: BlockType;
uploadFileRemote: (file: File) => Promise<string | undefined>;
createPendingData: (file: File) => Promise<PendingData<T>>;
finalizeData: (file: File, url: string) => T;
}
export function useMultiBlockUpload<T>({
editor,
blockId,
blockType,
uploadFileRemote,
createPendingData,
finalizeData,
}: UseMultiBlockUploadParams<T>) {
const cleanupLocalFile = useCallback(async (retryLocalUrl?: string) => {
if (!retryLocalUrl) return;
const fileHandler = new FileHandler();
await fileHandler.cleanup(retryLocalUrl).catch(() => undefined);
}, []);
const uploadIntoBlock = useCallback(
async (targetBlockId: string, file: File, pendingData: PendingData<T>) => {
const url = await uploadFileRemote(file);
if (!url) return;
await cleanupLocalFile(pendingData.retry_local_url);
let currentData: PendingData<T> | undefined;
try {
const entry = findSlateEntryByBlockId(editor, targetBlockId);
currentData = entry ? (entry[0] as { data?: PendingData<T> }).data ?? undefined : undefined;
} catch {
return;
}
if (!currentData?.pending_upload_id) return;
if (currentData.pending_upload_id !== pendingData.pending_upload_id) return;
if ((currentData as any).url) return;
CustomEditor.setBlockData(editor, targetBlockId, finalizeData(file, url));
},
[cleanupLocalFile, editor, uploadFileRemote, finalizeData],
);
const handleChangeUploadFiles = useCallback(
async (files: File[]) => {
if (!files.length) return;
const [primaryData, ...otherDatas] = await Promise.all(files.map((f) => createPendingData(f)));
const [file, ...otherFiles] = files;
CustomEditor.setBlockData(editor, blockId, primaryData as any);
const pendingUploads: Promise<void>[] = [uploadIntoBlock(blockId, file, primaryData)];
const reversedPairs = otherFiles.map((f, i) => [f, otherDatas[i]] as const).reverse();
for (const [f, data] of reversedPairs) {
const newId = CustomEditor.addBelowBlock(editor, blockId, blockType, data as any);
if (newId) pendingUploads.push(uploadIntoBlock(newId, f, data));
}
await Promise.all(pendingUploads);
},
[blockId, createPendingData, uploadIntoBlock, editor, blockType],
);
return { handleChangeUploadFiles };
}2. Keep AudioBlockPopoverContent focused on audio specifics + UI
With the hook, the popover becomes mostly wiring + audio‑specific helpers:
function AudioBlockPopoverContent({ blockId, onClose }: { blockId: string; onClose: () => void }) {
const editor = useSlateStatic() as YjsEditor;
const { uploadFile } = useEditorContext();
const { t } = useTranslation();
const [tabValue, setTabValue] = React.useState('upload');
const [uploading, setUploading] = React.useState(false);
const uploadFileRemote = useCallback(
async (file: File) => {
try {
return await uploadFile?.(file);
} catch {
return undefined;
}
},
[uploadFile],
);
const createPendingAudioData = useCallback(async (file: File): Promise<AudioBlockData> => {
const data: AudioBlockData = {
url: undefined,
name: file.name,
uploaded_at: Date.now(),
url_type: AudioUrlType.Cloud,
pending_upload_id: createPendingUploadId(),
};
try {
const fileHandler = new FileHandler();
const res = await fileHandler.handleFileUpload(file);
URL.revokeObjectURL(res.url);
data.retry_local_url = res.id;
} catch {
data.retry_local_url = '';
}
return data;
}, []);
const finalizeAudioData = useCallback(
(file: File, url: string): AudioBlockData => ({
url,
name: file.name,
uploaded_at: Date.now(),
url_type: AudioUrlType.Cloud,
retry_local_url: '',
pending_upload_id: '',
}),
[],
);
const { handleChangeUploadFiles } = useMultiBlockUpload<AudioBlockData>({
editor,
blockId,
blockType: BlockType.AudioBlock,
uploadFileRemote,
createPendingData: createPendingAudioData,
finalizeData: finalizeAudioData,
});
const handleTabChange = useCallback((_e: React.SyntheticEvent, newValue: string) => {
setTabValue(newValue);
}, []);
// wrap uploaded handler for UI state & closing
const handleUpload = useCallback(
async (files: File[]) => {
if (!files.length) return;
setUploading(true);
try {
await handleChangeUploadFiles(files);
onClose();
} finally {
setUploading(false);
}
},
[handleChangeUploadFiles, onClose],
);
// ...embed link + UI unchanged, just use handleUpload for FileDropzone.onChange
}This keeps:
- All existing behavior (pending upload id guard, reverse insertion, cleanup).
- Audio specifics (types,
createPendingUploadId,AudioUrlType,getAudioName) in the audio component. - The multi‑file upload orchestration in one reusable, testable unit.
It also flattens the nested async callbacks in AudioBlockPopoverContent into one handleUpload call, making the popover much easier to read.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Tests
Summary by Sourcery
Add support for new media and embed block types and improve how block popovers are anchored and configured for these embeds.
New Features:
Enhancements:
Tests: