feat: support copy link to block render#360
Open
LucasXu0 wants to merge 3 commits into
Open
Conversation
Reviewer's GuideAdds support for pasting AppFlowy "copy link to block" URLs as page mention references and refactors hover controls behavior for table blocks, along with new parsing utilities and tests. Sequence diagram for pasting AppFlowy block link as page mentionsequenceDiagram
actor User
participant Browser
participant withPasted
participant getSingleURLTextFromClipboardData
participant parseAppFlowyBlockLink
participant handleURLPaste
participant Transforms
User->>Browser: paste (DataTransfer)
Browser->>withPasted: onPaste(data)
withPasted->>getSingleURLTextFromClipboardData: getSingleURLTextFromClipboardData(data)
getSingleURLTextFromClipboardData-->>withPasted: clipboardURL
withPasted->>parseAppFlowyBlockLink: parseAppFlowyBlockLink(clipboardURL)
parseAppFlowyBlockLink-->>withPasted: AppFlowyBlockLink | null
withPasted->>handleURLPaste: handleURLPaste(editor, clipboardURL)
handleURLPaste->>parseAppFlowyBlockLink: parseAppFlowyBlockLink(url)
parseAppFlowyBlockLink-->>handleURLPaste: AppFlowyBlockLink
handleURLPaste->>Transforms: insertNodes(editor, mention)
Transforms-->>handleURLPaste: success
handleURLPaste-->>withPasted: true
File-Level Changes
Assessment against linked issues
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 2 issues, and left some high level feedback:
- In
withPasted,parseAppFlowyBlockLinkis invoked once beforehandleURLPasteand again insidehandleURLPaste; consider changinghandleURLPasteto accept anAppFlowyBlockLink(or returning early when a parsed link is passed) to avoid redundant parsing and branching. shouldShowHoverControlsForBlockduplicates the logic ingetTableHoverControlsRootby callingblockElement.closestin a loop; reusinggetTableHoverControlsRootthere would simplify the code and ensure the hover-control table detection behavior stays consistent in one place.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `withPasted`, `parseAppFlowyBlockLink` is invoked once before `handleURLPaste` and again inside `handleURLPaste`; consider changing `handleURLPaste` to accept an `AppFlowyBlockLink` (or returning early when a parsed link is passed) to avoid redundant parsing and branching.
- `shouldShowHoverControlsForBlock` duplicates the logic in `getTableHoverControlsRoot` by calling `blockElement.closest` in a loop; reusing `getTableHoverControlsRoot` there would simplify the code and ensure the hover-control table detection behavior stays consistent in one place.
## Individual Comments
### Comment 1
<location path="src/components/editor/plugins/appflowy-block-link.ts" line_range="7-8" />
<code_context>
+}
+
+const UUID_PATTERN = '[0-9a-fA-F-]{36}';
+const APPFLOWY_BLOCK_LINK_PATTERN = new RegExp(
+ `^https?://[^/]+/app/(${UUID_PATTERN})/(${UUID_PATTERN})(?:[?#][^\\s]*)?$`
+);
+
</code_context>
<issue_to_address>
**issue (bug_risk):** The new block-link parser no longer enforces same-origin, which may be an unintended behavior change.
Previously, `handleURLPaste` called `isURL` with `host_whitelist: [window.location.hostname]`, so only same-origin URLs were treated as internal block links. The new `APPFLOWY_BLOCK_LINK_PATTERN` accepts any host (`[^/]+`) as long as the path matches `/app/{uuid}/{uuid}` with optional query/hash, so a URL from any domain that mimics this structure will now be handled as an internal block link. If we still intend to restrict this to the current origin, we should add an explicit host (and possibly protocol/port) check via the regex or a `URL` parse compared against `window.location.hostname`.
</issue_to_address>
### Comment 2
<location path="src/components/editor/plugins/appflowy-block-link.ts" line_range="1" />
<code_context>
+export interface AppFlowyBlockLink {
+ pageId: string;
+ blockId: string;
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring the URL parsing and clipboard helpers to centralize HTTP URL validation and line handling, eliminating duplicate parsing logic and regex branching.
You can keep the same behavior with less branching and duplication by:
### 1. Simplifying `parseAppFlowyBlockLink`
You don’t need both a regex and `new URL`. You can derive `pageId` from the path and `blockId` from `searchParams` in a single pass, and avoid hard‑coding UUID shapes:
```ts
export function parseAppFlowyBlockLink(raw: string): AppFlowyBlockLink | null {
const trimmed = raw.trim();
let url: URL;
try {
url = new URL(trimmed);
} catch {
return null;
}
// Expect /app/{pageId}/{something}
const segments = url.pathname.split('/').filter(Boolean);
if (segments.length < 3 || segments[0] !== 'app') return null;
const pageId = segments[1];
const blockId = url.searchParams.get('blockId');
if (!blockId) return null;
return { pageId, blockId };
}
```
This removes `UUID_PATTERN`, `APPFLOWY_BLOCK_LINK_PATTERN`, and the double parse while keeping the same logical checks.
---
### 2. Centralizing URL parsing
Instead of `isHTTPURL` + multiple `new URL` calls, use a single helper:
```ts
function tryParseHttpUrl(value: string | undefined): URL | null {
if (!value) return null;
try {
const url = new URL(value);
return url.protocol === 'http:' || url.protocol === 'https:' ? url : null;
} catch {
return null;
}
}
```
Then your call sites become simpler, e.g.:
```ts
function getSingleURLText(value: string | undefined): string | undefined {
const trimmed = value?.trim();
if (!trimmed) return undefined;
const lines = trimmed.split(/\r\n|\r|\n/).filter(Boolean);
if (lines.length !== 1) return undefined;
return tryParseHttpUrl(lines[0]) ? lines[0] : undefined;
}
```
And:
```ts
function getSingleURLTextFromUriList(value: string | undefined): string | undefined {
const lines = value
?.split(/\r\n|\r|\n/)
.map((line) => line.trim())
.filter((line) => line && !line.startsWith('#'));
if (!lines || lines.length !== 1) return undefined;
return tryParseHttpUrl(lines[0]) ? lines[0] : undefined;
}
```
---
### 3. Flattening the clipboard URL extraction pipeline
You can reuse a “single URL from lines” helper so the three sources differ only in how they supply candidate lines:
```ts
function pickSingleHttpUrl(lines: string[] | undefined): string | undefined {
if (!lines) return undefined;
const normalized = lines.map((l) => l.trim()).filter(Boolean);
if (normalized.length !== 1) return undefined;
return tryParseHttpUrl(normalized[0]) ? normalized[0] : undefined;
}
```
Then:
```ts
function getSingleURLText(value: string | undefined): string | undefined {
return pickSingleHttpUrl(value?.split(/\r\n|\r|\n/));
}
function getSingleURLTextFromUriList(value: string | undefined): string | undefined {
const lines = value
?.split(/\r\n|\r|\n/)
.filter((line) => !line.trim().startsWith('#'));
return pickSingleHttpUrl(lines);
}
```
For HTML, avoid re‑entering `getSingleURLText` and instead feed `pickSingleHttpUrl` directly:
```ts
function getSingleURLTextFromHTML(html: string | undefined): string | undefined {
const trimmed = html?.trim();
if (!trimmed || typeof DOMParser === 'undefined') return undefined;
try {
const doc = new DOMParser().parseFromString(trimmed, 'text/html');
const href = doc.querySelector('a[href]')?.getAttribute('href')?.trim();
if (href && tryParseHttpUrl(href)) return href;
const bodyText = doc.body.textContent ?? '';
return pickSingleHttpUrl(bodyText.split(/\r\n|\r|\n/));
} catch {
return undefined;
}
}
```
This keeps the same behavior (HTML → URL from text or `<a>`), but with a single line-processing + URL-validation core, and no nested reuse of helpers with different semantics.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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.
Description
closes AppFlowy-IO/AppFlowy#8748
Checklist
General
Testing
Feature-Specific
Summary by Sourcery
Add support for interpreting pasted AppFlowy block links as page-reference mentions and improve hover controls behavior for table blocks.
New Features:
Enhancements:
Tests: