feat: support simple ai meeting and pdf block #185
Merged
Conversation
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
Reviewer's GuideAdds new editor block types for AI Meeting and PDF documents, including rendering components, popover-based PDF upload/embed workflows, file-upload retry support, and Storybook stories for visual testing. Sequence diagram for PDF block upload and embed workflowsequenceDiagram
actor User
participant Editor as EditorUI
participant ElementRouter as ElementComponentRouter
participant PDF as PDFBlock
participant Popover as BlockPopover
participant PopoverContent as PDFBlockPopoverContent
participant Ctx as EditorContext
participant FH as FileHandler
participant CE as CustomEditor
participant Remote as RemoteStorage
User->>Editor: Insert PDF block
Editor->>ElementRouter: Render block
ElementRouter->>PDF: render(node)
rect rgb(235,235,255)
User->>PDF: Click empty PDF block
PDF->>Popover: openPopover(blockId, PDFBlock, anchor)
Popover->>PopoverContent: render(blockId, onClose)
end
alt Upload_tab
User->>PopoverContent: Drop PDF files
PopoverContent->>Ctx: uploadFileRemote(file)
Ctx->>Remote: uploadFile(file)
Remote-->>Ctx: url
Ctx-->>PopoverContent: url
PopoverContent->>FH: getData(file, url)
FH-->>PopoverContent: PDFBlockData
PopoverContent->>CE: setBlockData(editor, blockId, PDFBlockData)
loop Additional_files
PopoverContent->>CE: addBelowBlock(editor, blockId, PDFBlock, PDFBlockData)
end
PopoverContent->>Popover: onClose()
else Embed_link_tab
User->>PopoverContent: Paste PDF url
PopoverContent->>CE: setBlockData(editor, blockId, PDFBlockData)
PopoverContent->>Popover: onClose()
end
Editor->>ElementRouter: Rerender block
ElementRouter->>PDF: render(node_with_url)
User->>PDF: Click PDF block
PDF->>User: Open url in new tab
Sequence diagram for PDF upload failure and retrysequenceDiagram
actor User
participant PDF as PDFBlock
participant FH as FileHandler
participant Ctx as EditorContext
participant CE as CustomEditor
participant Remote as RemoteStorage
rect rgb(245,235,235)
note over PDF: Initial upload failed in popover
PDF->>FH: getStoredFile(retry_local_url)
FH-->>PDF: fileData(file, local_url)
PDF->>PDF: setNeedRetry(true)
end
User->>PDF: Click Retry button
PDF->>FH: getStoredFile(retry_local_url)
FH-->>PDF: file
PDF->>Ctx: uploadFileRemote(file)
Ctx->>Remote: uploadFile(file)
Remote-->>Ctx: url
Ctx-->>PDF: url
alt Upload_success
PDF->>FH: cleanup(retry_local_url)
FH-->>PDF: ok
PDF->>CE: setBlockData(editor, blockId, PDFBlockData)
PDF->>PDF: setNeedRetry(false)
else Upload_failed
PDF->>PDF: keepNeedRetry(true)
end
Class diagram for new AI Meeting and PDF editor block typesclassDiagram
class BlockType {
<<enumeration>>
SimpleTableCellBlock
ColumnsBlock
ColumnBlock
AIMeetingBlock
PDFBlock
}
class BlockData {
}
class AIMeetingBlockData {
+string title
}
class PDFBlockData {
+string name
+number uploaded_at
+string url
+FieldURLType url_type
+string retry_local_url
}
BlockData <|-- AIMeetingBlockData
BlockData <|-- PDFBlockData
class BlockNode {
+string blockId
+BlockType type
+BlockData data
+BlockNode children
}
class AIMeetingNode {
+string blockId
+AIMeetingBlockData data
}
class PDFNode {
+string blockId
+PDFBlockData data
}
BlockNode <|-- AIMeetingNode
BlockNode <|-- PDFNode
class AIMeetingBlock {
+render(node, children, attributes)
}
class PDFBlock {
+render(node, children, attributes)
-handleClick()
-handleRetry(event)
-uploadFileRemote(file)
}
class PDFBlockPopoverContent {
+render(blockId, onClose)
+handleTabChange(event, newValue)
+handleInsertEmbedLink(url)
+handleChangeUploadFiles(files)
+uploadFileRemote(file)
+insertPDFBlock(file)
+getData(file, remoteUrl)
}
class EditorElementProps {
+BlockNode node
}
class EditorContext {
+uploadFile(file)
+string workspaceId
+string viewId
}
class FileHandler {
+handleFileUpload(file)
+getStoredFile(id)
+cleanup(id)
}
class CustomEditor {
+setBlockData(editor, blockId, data)
+addBelowBlock(editor, blockId, type, data)
}
class YjsEditor {
+boolean isElementReadOnly(element)
}
class BlockPopoverContext {
+openPopover(blockId, type, anchor)
+close()
}
class ElementComponentRouter {
+Element(props)
}
BlockType ..> AIMeetingNode
BlockType ..> PDFNode
AIMeetingNode --> AIMeetingBlock : rendered_by
PDFNode --> PDFBlock : rendered_by
EditorElementProps <.. AIMeetingBlock
EditorElementProps <.. PDFBlock
PDFBlock --> EditorContext : uses_uploadFile
PDFBlock --> FileHandler : creates
PDFBlock --> CustomEditor : setBlockData
PDFBlock --> YjsEditor : readOnly_state
PDFBlock --> BlockPopoverContext : openPopover
PDFBlockPopoverContent --> EditorContext : uses_uploadFile
PDFBlockPopoverContent --> FileHandler : uses
PDFBlockPopoverContent --> CustomEditor : setBlockData_addBelowBlock
ElementComponentRouter --> AIMeetingBlock : case_AIMeetingBlock
ElementComponentRouter --> PDFBlock : case_PDFBlock
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 there - I've reviewed your changes - here's some feedback:
- In
PDFBlockPopoverContent.getFileName, constructingnew URL(url)will throw on invalid or relative URLs; consider wrapping this in a try/catch or falling back to a simpler string-based parse to avoid breaking the popover on bad input. - In
PDFBlock.handleRetry, errors during retry are swallowed and the user only sees a spinner stop; consider surfacing a notification or explicit error state when retry fails so users understand what happened and whether another attempt is possible.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `PDFBlockPopoverContent.getFileName`, constructing `new URL(url)` will throw on invalid or relative URLs; consider wrapping this in a try/catch or falling back to a simpler string-based parse to avoid breaking the popover on bad input.
- In `PDFBlock.handleRetry`, errors during retry are swallowed and the user only sees a spinner stop; consider surfacing a notification or explicit error state when retry fails so users understand what happened and whether another attempt is possible.
## Individual Comments
### Comment 1
<location> `src/components/editor/components/block-popover/PDFBlockPopoverContent.tsx:16-20` </location>
<code_context>
+
+import EmbedLink from 'src/components/_shared/image-upload/EmbedLink';
+
+export function getFileName(url: string) {
+ const urlObj = new URL(url);
+ const name = urlObj.pathname.split('/').pop();
+
+ return name;
+}
+
</code_context>
<issue_to_address>
**issue:** Harden `getFileName` against invalid URLs and empty path segments.
`new URL(url)` will throw for non-absolute inputs (e.g. missing protocol), which will bubble up from `handleInsertEmbedLink`. It also returns an empty last segment for URLs ending in `/`. Wrap parsing in a `try/catch` and handle falsy `name` by falling back to the original `url` (or a generic label), e.g.:
```ts
export function getFileName(rawUrl: string) {
try {
const urlObj = new URL(rawUrl);
const name = urlObj.pathname.split("/").filter(Boolean).pop();
return name || rawUrl;
} catch {
return rawUrl;
}
}
```
</issue_to_address>
### Comment 2
<location> `src/components/editor/components/blocks/pdf/PDFBlock.tsx:27` </location>
<code_context>
+ const fileHandler = useMemo(() => new FileHandler(), []);
+ const [localUrl, setLocalUrl] = useState<string | undefined>(undefined);
+ const [loading, setLoading] = useState(false);
+ const { url: dataUrl, name, retry_local_url } = useMemo(() => data || {}, [data]);
+ const readOnly = useReadOnly() || editor.isElementReadOnly(node as unknown as Element);
+ const emptyRef = useRef<HTMLDivElement>(null);
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring the PDFBlock component to remove unnecessary hooks, clarify click behavior, and simplify class and handler state to make the code easier to follow.
You can trim a fair bit of complexity here without changing behavior by tightening a few patterns inside this component.
### 1. Drop unnecessary `useMemo` around `data` and `url`
You don’t really need the `useMemo` or the extra indirection via `dataUrl`:
```ts
// before
const { url: dataUrl, name, retry_local_url } = useMemo(() => data || {}, [data]);
...
const url = dataUrl;
// after
const { url, name, retry_local_url } = data ?? {};
```
This removes an extra memo and an extra variable, making the data flow easier to follow.
### 2. Simplify `className` computation and avoid duplicate `cursor-pointer`
You currently push `cursor-pointer` in two different branches. That can be simplified into an explicit “interactive” flag:
```ts
// before
const className = useMemo(() => {
const classList = ['w-full'];
if (url) {
classList.push('cursor-pointer');
} else {
classList.push('text-text-secondary');
}
if (attributes.className) {
classList.push(attributes.className);
}
if (!readOnly) {
classList.push('cursor-pointer');
}
return classList.join(' ');
}, [attributes.className, readOnly, url]);
// after
const className = useMemo(() => {
const isInteractive = !!url || !readOnly;
const classList = ['w-full'];
if (attributes.className) {
classList.push(attributes.className);
}
if (isInteractive) {
classList.push('cursor-pointer');
}
if (!url) {
classList.push('text-text-secondary');
}
return classList.join(' ');
}, [attributes.className, readOnly, url]);
```
This expresses the intent directly: “pointer when interactive, faded when no URL”.
### 3. Use `useRef` instead of `useMemo` for `FileHandler`
Since `FileHandler` is used as an instance, a ref is simpler and avoids the mental model of “memoized value that might someday gain deps”:
```ts
// before
const fileHandler = useMemo(() => new FileHandler(), []);
// after
const fileHandlerRef = useRef(new FileHandler());
const fileHandler = fileHandlerRef.current;
```
This keeps the same lifecycle behavior but makes it clear it’s a stable instance.
### 4. Make the click behavior explicit with small helpers
`handleClick` currently mixes “open popover” and “open PDF” logic inline. Pulling small helpers out makes it easier to read without changing behavior:
```ts
const shouldOpenPopover = !url && !needRetry && !readOnly;
const openConfigPopover = () => {
if (emptyRef.current && !readOnly) {
openPopover(blockId, BlockType.PDFBlock, emptyRef.current);
}
};
const openPdfInNewTab = () => {
const link = url || localUrl;
if (link) {
window.open(link, '_blank');
}
};
const handleClick = useCallback(async () => {
try {
if (shouldOpenPopover) {
openConfigPopover();
} else {
openPdfInNewTab();
}
} catch (e: any) {
notify.error(e.message);
}
}, [shouldOpenPopover, url, localUrl, readOnly, openPopover, blockId]);
```
This doesn’t change functionality but makes the two modes of the click much more obvious.
These small refactors keep all features intact but reduce the cognitive load of the component and address some of the complexity concerns without a larger extraction into custom hooks.
</issue_to_address>
### Comment 3
<location> `src/components/editor/components/block-popover/PDFBlockPopoverContent.tsx:56` </location>
<code_context>
+ [blockId, editor, onClose]
+ );
+
+ const uploadFileRemote = useCallback(
+ async (file: File) => {
+ try {
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring the upload/insert logic into a single helper and simplifying the tab rendering so the data flow and UI conditions are more explicit and easier to follow.
You can reduce the complexity in two focused areas without changing behavior: the upload/insert flow and the tab/panel rendering.
---
### 1. Consolidate upload/insert flow into a single helper
Right now `uploadFileRemote`, `getData`, `insertPDFBlock`, and `handleChangeUploadFiles` are chained and share responsibilities. You can encapsulate “file → data → insert/update blocks” in a single helper, and keep `uploadFileRemote` and data shaping clearly separated.
Example refactor:
```ts
const uploadFileRemote = useCallback(
async (file: File) => {
if (!uploadFile) return undefined;
try {
return await uploadFile(file);
} catch {
return undefined;
}
},
[uploadFile],
);
const buildPdfBlockData = useCallback(
async (file: File) => {
const remoteUrl = await uploadFileRemote(file);
const data: PDFBlockData = {
url: remoteUrl,
name: file.name,
uploaded_at: Date.now(),
url_type: FieldURLType.Upload,
};
if (!remoteUrl) {
const fileHandler = new FileHandler();
const res = await fileHandler.handleFileUpload(file);
data.retry_local_url = res.id;
}
return data;
},
[uploadFileRemote],
);
// single high‑level flow for first + remaining files
const insertOrUpdatePdfBlocks = useCallback(
async (files: File[]) => {
if (!files.length) return;
const [first, ...rest] = files;
// update current block with first file
const firstData = await buildPdfBlockData(first);
CustomEditor.setBlockData(editor, blockId, firstData);
// insert remaining files below
for (const file of rest.reverse()) {
const data = await buildPdfBlockData(file);
CustomEditor.addBelowBlock(editor, blockId, BlockType.PDFBlock, data);
}
},
[blockId, editor, buildPdfBlockData],
);
const handleChangeUploadFiles = useCallback(
async (files: File[]) => {
if (!files.length) return;
setUploading(true);
try {
await insertOrUpdatePdfBlocks(files);
onClose();
} finally {
setUploading(false);
}
},
[insertOrUpdatePdfBlocks, onClose],
);
```
This removes `getData` and `insertPDFBlock` as separate concepts and makes the control flow “files → set first block → insert rest” explicit.
---
### 2. Simplify tab rendering and default link extraction
You don’t need `tabOptions`, `selectedIndex`, or mapping twice for just two tabs. You can:
- Use `tabValue` as the discriminator.
- Render panels conditionally.
- Extract `defaultLink` once with proper null‑safety.
Example:
```ts
const defaultLink = useMemo(() => {
const node = entry?.[0];
return (node?.data as PDFBlockData | undefined)?.url ?? '';
}, [entry]);
```
Then:
```tsx
<ViewTabs
value={tabValue}
onChange={handleTabChange}
className="min-h-[38px] w-[560px] max-w-[964px] border-b border-border-primary px-2"
>
<ViewTab iconPosition="start" color="inherit" label={t('button.upload')} value="upload" />
<ViewTab iconPosition="start" color="inherit" label={t('document.plugins.file.networkTab')} value="embed" />
</ViewTabs>
<div className="appflowy-scroller max-h-[400px] overflow-y-auto p-2">
{tabValue === 'upload' && (
<FileDropzone
accept={{ 'application/pdf': ['.pdf'] }}
multiple
placeholder={
<span>
Click to upload or drag and drop PDF files
<span className="text-text-action">
{' '}
{t('document.plugins.photoGallery.browserLayout')}
</span>
</span>
}
onChange={handleChangeUploadFiles}
loading={uploading}
/>
)}
{tabValue === 'embed' && (
<EmbedLink
onDone={handleInsertEmbedLink}
defaultLink={defaultLink}
placeholder="Embed a PDF link"
/>
)}
</div>
```
This removes the `tabOptions` array, the `selectedIndex` indirection, and the need to cast `entry?.[0].data` inline, making the component easier to read and maintain while preserving the same behavior.
</issue_to_address>
### Comment 4
<location> `src/components/editor/components/block-popover/PDFBlockPopoverContent.tsx:18-20` </location>
<code_context>
const name = urlObj.pathname.split('/').pop();
return name;
</code_context>
<issue_to_address>
**suggestion (code-quality):** Inline variable that is immediately returned ([`inline-immediately-returned-variable`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/inline-immediately-returned-variable))
```suggestion
return urlObj.pathname.split('/').pop();
```
<br/><details><summary>Explanation</summary>Something that we often see in people's code is assigning to a result variable
and then immediately returning it.
Returning the result directly shortens the code and removes an unnecessary
variable, reducing the mental load of reading the function.
Where intermediate variables can be useful is if they then get used as a
parameter or a condition, and the name can act like a comment on what the
variable represents. In the case where you're returning it from a function, the
function name is there to tell you what the result is, so the variable name
is unnecessary.
</details>
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
src/components/editor/components/block-popover/PDFBlockPopoverContent.tsx
Outdated
Show resolved
Hide resolved
src/components/editor/components/block-popover/PDFBlockPopoverContent.tsx
Outdated
Show resolved
Hide resolved
josue693
pushed a commit
to josue693/AppFlowy-Web
that referenced
this pull request
Dec 21, 2025
* feat: support simple ai meeting and pdf block * chore: update placeholder text * chore: code review * chore: lint code * chore: update placeholder text * fix: lint error
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
Checklist
General
Testing
Feature-Specific
Summary by Sourcery
Add new AI Meeting and PDF block types to the editor and integrate them into the web editor UI.
New Features:
Tests: