Build unsupport block #183
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 GuideImplements a reusable UnSupportedBlock UI for unknown editor block types, wires BlockNotFound to use it, exposes editor/Yjs handles for E2E testing, and adds Storybook stories plus an E2E test scaffold for unsupported blocks. Sequence diagram for CollaborativeEditor test handle exposuresequenceDiagram
participant Browser
participant CollaborativeEditor
participant YjsEditor as YjsEditor
participant YDoc as Y_Doc
participant Win as Window
Browser->>CollaborativeEditor: mount component
activate CollaborativeEditor
CollaborativeEditor->>YjsEditor: editor.connect(doc)
activate YjsEditor
YjsEditor-->>CollaborativeEditor: connected
deactivate YjsEditor
Note over CollaborativeEditor: useEffect on editor mount
CollaborativeEditor->>Win: set __TEST_EDITOR__ = editor
CollaborativeEditor->>Win: set __TEST_DOC__ = doc
CollaborativeEditor->>Win: set Y = Y_Module
Browser-->>CollaborativeEditor: unmount component
CollaborativeEditor->>YjsEditor: editor.disconnect()
CollaborativeEditor->>Win: delete __TEST_EDITOR__
CollaborativeEditor->>Win: delete __TEST_DOC__
deactivate CollaborativeEditor
Class diagram for updated unsupported block componentsclassDiagram
class BlockNotFound {
+props EditorElementProps
+ref HTMLDivElement
+render(node, children, attributes, ref)
}
class UnSupportedBlock {
+props EditorElementProps
+className string
+children ReactNode
+ref HTMLDivElement
+isDev boolean
+render(node, children, className, attributes, ref)
}
class UnSupportedBlockDev {
+props EditorElementProps
+children ReactNode
+ref HTMLDivElement
+render(node, children, ref)
}
class CollaborativeEditor {
+editor YjsEditor
+doc Y_Doc
+onEditorConnected function
+useEffect_connectAndExpose()
}
class WindowTestHandles {
<<interface>>
+__TEST_EDITOR__ YjsEditor
+__TEST_DOC__ Y_Doc
+Y Y_Module
}
BlockNotFound --> UnSupportedBlock : uses
UnSupportedBlock ..> WindowTestHandles : referenced via attributes
CollaborativeEditor ..> WindowTestHandles : exposes test handles
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:
- The
BlockNotFoundcomponent now always rendersUnSupportedBlockin production instead of a zero-height placeholder; double-check that this UX change is intentional and won’t visually regress existing documents with unknown block types. UnSupportedBlockDevappears to be unused after the refactor; consider removing it or wiring it up somewhere to avoid dead code.- In
UnSupportedBlock, the hardcodeddata-testid="unsupported-block"will overwrite anydata-testidpassed via props; if you need both behaviors, consider composing the test id or only applying the default when one isn’t provided.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `BlockNotFound` component now always renders `UnSupportedBlock` in production instead of a zero-height placeholder; double-check that this UX change is intentional and won’t visually regress existing documents with unknown block types.
- `UnSupportedBlockDev` appears to be unused after the refactor; consider removing it or wiring it up somewhere to avoid dead code.
- In `UnSupportedBlock`, the hardcoded `data-testid="unsupported-block"` will overwrite any `data-testid` passed via props; if you need both behaviors, consider composing the test id or only applying the default when one isn’t provided.
## Individual Comments
### Comment 1
<location> `src/components/editor/components/element/UnSupportedBlock.tsx:39` </location>
<code_context>
+ );
+});
+
+export const UnSupportedBlockDev = forwardRef<HTMLDivElement, EditorElementProps>(({ node, children }, ref) => {
return (
<div className={'w-full select-none'} ref={ref} contentEditable={false}>
</code_context>
<issue_to_address>
**issue (complexity):** Consider consolidating the dev-only and legacy unsupported block behavior into a single `UnSupportedBlock` component with a layout variant instead of maintaining two separate components.
The extra `UnSupportedBlockDev` component does add avoidable complexity and duplicate responsibility. You can keep all current behavior while simplifying by consolidating into a single component and making the “legacy” layout a variant.
For example, instead of exporting two components:
```ts
export const UnSupportedBlock = forwardRef<HTMLDivElement, EditorElementProps>(
({ node, children, className, ...attributes }, ref) => {
const isDev = import.meta.env.DEV;
return (
<div
ref={ref}
{...attributes}
data-testid="unsupported-block"
className={cn(
className,
'my-1 flex w-full select-none items-center gap-2 rounded-lg border border-line-divider bg-fill-list-hover px-3 py-2 text-text-caption'
)}
contentEditable={false}
>
<WarningIcon className="h-5 w-5 flex-shrink-0 text-function-warning" />
<span className="text-sm">
This block type <span className="font-medium text-text-title">“{node.type}”</span> is not supported yet!
</span>
{isDev && (
<details className="ml-auto text-xs">
<summary className="cursor-pointer text-text-caption hover:text-text-title">Debug</summary>
<pre className="mt-2 max-h-40 overflow-auto rounded bg-bg-body p-2">
<code>{JSON.stringify(node, null, 2)}</code>
</pre>
</details>
)}
{children}
</div>
);
}
);
export const UnSupportedBlockDev = forwardRef<HTMLDivElement, EditorElementProps>(
({ node, children }, ref) => {
// legacy Alert-based UI
}
);
```
You can have a single component with a variant flag (or an env-based switch) that preserves the old behavior when needed:
```ts
type UnSupportedBlockProps = EditorElementProps & {
legacyLayout?: boolean;
};
export const UnSupportedBlock = forwardRef<HTMLDivElement, UnSupportedBlockProps>(
({ node, children, className, legacyLayout = false, ...attributes }, ref) => {
const isDev = import.meta.env.DEV;
if (legacyLayout) {
return (
<div className="w-full select-none" ref={ref} contentEditable={false} {...attributes}>
<Alert className="h-fit w-full" severity="warning">
<div className="text-base font-semibold">{`Unsupported Block: ${node.type}`}</div>
<div className="my-4 whitespace-pre font-medium">
{`We're sorry for inconvenience \n`}
Submit an issue on our{' '}
<a className="text-text-action underline" /* ... */>
GitHub
</a>
</div>
{children}
</Alert>
</div>
);
}
return (
<div
ref={ref}
{...attributes}
data-testid="unsupported-block"
className={cn(
className,
'my-1 flex w-full select-none items-center gap-2 rounded-lg border border-line-divider bg-fill-list-hover px-3 py-2 text-text-caption'
)}
contentEditable={false}
>
<WarningIcon className="h-5 w-5 flex-shrink-0 text-function-warning" />
<span className="text-sm">
This block type <span className="font-medium text-text-title">“{node.type}”</span> is not supported yet!
</span>
{isDev && (
<details className="ml-auto text-xs">
<summary className="cursor-pointer text-text-caption hover:text-text-title">Debug</summary>
<pre className="mt-2 max-h-40 overflow-auto rounded bg-bg-body p-2">
<code>{JSON.stringify(node, null, 2)}</code>
</pre>
</details>
)}
{children}
</div>
);
}
);
```
Actionable steps:
1. Remove `UnSupportedBlockDev` if it’s not used anywhere, **or** move its JSX into a `legacyLayout` (or `variant`) branch on `UnSupportedBlock`.
2. If you want it dev-only, gate `legacyLayout` behind an env flag where it’s consumed (or derive it from `import.meta.env.DEV` inside the component).
3. Keep all dev-specific behavior (`Debug` details + any legacy layout) scoped to this single component so future readers don’t have to reason about two overlapping exports.
</issue_to_address>
### Comment 2
<location> `cypress/e2e/editor/blocks/unsupported_block.cy.ts:80` </location>
<code_context>
const Y = testWindow.Y;
</code_context>
<issue_to_address>
**suggestion (code-quality):** Prefer object destructuring when accessing and using properties. ([`use-object-destructuring`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/use-object-destructuring))
```suggestion
const {Y} = testWindow;
```
<br/><details><summary>Explanation</summary>Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.
From the [Airbnb Javascript Style Guide](https://airbnb.io/javascript/#destructuring--object)
</details>
</issue_to_address>
### Comment 3
<location> `cypress/e2e/editor/blocks/unsupported_block.cy.ts:162` </location>
<code_context>
const Y = testWindow.Y;
</code_context>
<issue_to_address>
**suggestion (code-quality):** Prefer object destructuring when accessing and using properties. ([`use-object-destructuring`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/use-object-destructuring))
```suggestion
const {Y} = testWindow;
```
<br/><details><summary>Explanation</summary>Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.
From the [Airbnb Javascript Style Guide](https://airbnb.io/javascript/#destructuring--object)
</details>
</issue_to_address>
### Comment 4
<location> `cypress/e2e/editor/blocks/unsupported_block.cy.ts:235` </location>
<code_context>
const Y = testWindow.Y;
</code_context>
<issue_to_address>
**suggestion (code-quality):** Prefer object destructuring when accessing and using properties. ([`use-object-destructuring`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/use-object-destructuring))
```suggestion
const {Y} = testWindow;
```
<br/><details><summary>Explanation</summary>Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.
From the [Airbnb Javascript Style Guide](https://airbnb.io/javascript/#destructuring--object)
</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.
josue693
pushed a commit
to josue693/AppFlowy-Web
that referenced
this pull request
Dec 21, 2025
* chore: build unsupport block * chore: clippy
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
Handle unknown editor block types with a dedicated unsupported-block UI and improve testability of the collaborative editor.
New Features:
Enhancements:
Tests: