Skip to content

Commit

Permalink
fix(component): lit portal not re-rendering in inline links case (#9321)
Browse files Browse the repository at this point in the history
* uses `portal.key` in templates
* updates `portal.id`  for use in queries
  • Loading branch information
fundon authored and pengx17 committed Dec 27, 2024
1 parent f336d68 commit beb61d2
Show file tree
Hide file tree
Showing 7 changed files with 177 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ export function attachmentViewToggleMenu({
button => button.type,
({ type, label, action, disabled }) => html`
<editor-menu-action
aria-label=${label}
data-testid=${`link-to-${type}`}
?data-selected=${type === viewType}
?disabled=${disabled}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@ export {
type ElementOrFactory,
useLitPortal,
useLitPortalFactory,
} from './lite-portal';
} from './lit-portal';
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,9 @@ type PortalEvent = {
type PortalListener = (event: PortalEvent) => void;

export function createLitPortalAnchor(callback: (event: PortalEvent) => void) {
const id = nanoid();
return html`<lit-react-portal
.notify=${callback}
portalId=${id}
portalId=${nanoid()}
></lit-react-portal>`;
}

Expand Down Expand Up @@ -119,24 +118,27 @@ export const useLitPortalFactory = () => {
}

const prevId = event.previousPortalId;
// Ignore first `willUpdate`
if (!prevId) {
return;
}

// No re-rendering allowed
// Used in `pdf embed view` scenario
if (!rerendering) {
// Ignores first `willUpdate`
if (!prevId) {
return;
}

setPortals(portals => {
const portal = portals.find(p => p.id === prevId);
if (!portal) return portals;
if (!portal) return [...portals];

// Updates `ID`
// Used for portal queries in `disconnectedCallback`
portal.id = id;
portal.portal.key = id;
portal.portal.children = element;

// Re-rendering
// true: `inline link`
// false: `pdf embed view`
if (rerendering) {
portal.portal = ReactDOM.createPortal(element, target, id);
}

return [...portals];
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -219,13 +219,15 @@ export function PDFViewerEmbeddedInner({ model }: PDFViewerProps) {
icon={<ArrowUpSmallIcon />}
className={embeddedStyles.pdfControlButton}
onDoubleClick={stopPropagation}
aria-label="Prev"
{...navigator.prev}
/>
<IconButton
size={16}
icon={<ArrowDownSmallIcon />}
className={embeddedStyles.pdfControlButton}
onDoubleClick={stopPropagation}
aria-label="Next"
{...navigator.next}
/>
<IconButton
Expand All @@ -250,8 +252,13 @@ export function PDFViewerEmbeddedInner({ model }: PDFViewerProps) {
embeddedStyles.pdfPageCount,
])}
>
<span>{meta.pageCount > 0 ? cursor + 1 : '-'}</span>/
<span>{meta.pageCount > 0 ? meta.pageCount : '-'}</span>
<span className="page-cursor">
{meta.pageCount > 0 ? cursor + 1 : '-'}
</span>
/
<span className="page-count">
{meta.pageCount > 0 ? meta.pageCount : '-'}
</span>
</div>
</footer>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ export const BiDirectionalLinkPanel = () => {
{
<>
{portals.map(p => (
<Fragment key={p.id}>{p.portal}</Fragment>
<Fragment key={p.portal.key}>{p.portal}</Fragment>
))}
</>
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ const usePatchSpecs = (shared: boolean, mode: DocMode) => {
() => (
<>
{portals.map(p => (
<Fragment key={p.id}>{p.portal}</Fragment>
<Fragment key={p.portal.key}>{p.portal}</Fragment>
))}
</>
),
Expand Down
159 changes: 150 additions & 9 deletions tests/affine-local/e2e/attachment-preview.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
/* eslint-disable unicorn/prefer-dom-node-dataset */
import path from 'node:path';

import { test } from '@affine-test/kit/playwright';
Expand All @@ -7,7 +6,13 @@ import {
clickNewPageButton,
getBlockSuiteEditorTitle,
waitForEditorLoad,
waitForEmptyEditor,
} from '@affine-test/kit/utils/page-logic';
import {
confirmExperimentalPrompt,
openEditorSetting,
openExperimentalFeaturesPanel,
} from '@affine-test/kit/utils/setting';
import type { Page } from '@playwright/test';
import { expect } from '@playwright/test';

Expand Down Expand Up @@ -53,10 +58,10 @@ test('attachment preview should be shown', async ({ page }) => {

await page.waitForTimeout(500);

const pageCount = attachmentViewer.locator('.page-cursor');
expect(await pageCount.textContent()).toBe('1');
const pageTotal = attachmentViewer.locator('.page-count');
expect(await pageTotal.textContent()).toBe('3');
const pageCursor = attachmentViewer.locator('.page-cursor');
expect(await pageCursor.textContent()).toBe('1');
const pageCount = attachmentViewer.locator('.page-count');
expect(await pageCount.textContent()).toBe('3');

const thumbnails = attachmentViewer.locator('.thumbnails');
await thumbnails.locator('button').click();
Expand Down Expand Up @@ -99,10 +104,10 @@ test('attachment preview can be expanded', async ({ page }) => {

await page.waitForTimeout(500);

const pageCount = attachmentViewer.locator('.page-cursor');
expect(await pageCount.textContent()).toBe('1');
const pageTotal = attachmentViewer.locator('.page-count');
expect(await pageTotal.textContent()).toBe('3');
const pageCursor = attachmentViewer.locator('.page-cursor');
expect(await pageCursor.textContent()).toBe('1');
const pageCount = attachmentViewer.locator('.page-count');
expect(await pageCount.textContent()).toBe('3');

const thumbnails = attachmentViewer.locator('.thumbnails');
await thumbnails.locator('button').click();
Expand All @@ -116,3 +121,139 @@ test('attachment preview can be expanded', async ({ page }) => {
.count()
).toBe(3);
});

test('should preview PDF in embed view', async ({ page }) => {
await openHomePage(page);
await clickNewPageButton(page);
await waitForEmptyEditor(page);

const title = getBlockSuiteEditorTitle(page);
await title.click();
await page.keyboard.type('PDF preview');

// Opens settings panel
await openEditorSetting(page);
await openExperimentalFeaturesPanel(page);
await confirmExperimentalPrompt(page);

const settingModal = page.locator('[data-testid=setting-modal-content]');
const item = settingModal.locator('div').getByText('PDF embed preview');
await item.waitFor({ state: 'attached' });
await expect(item).toBeVisible();
const button = item.locator('label');
const isChecked = await button.locator('input').isChecked();
if (!isChecked) {
await button.click();
}

// Closes settings panel
await page.keyboard.press('Escape');

await clickNewPageButton(page);
await waitForEmptyEditor(page);

await title.click();
await page.keyboard.type('PDF page');

await page.keyboard.press('Enter');

await insertAttachment(
page,
path.join(__dirname, '../../fixtures/lorem-ipsum.pdf')
);

const attachment = page.locator('affine-attachment');
await attachment.hover();

const attachmentToolbar = page.locator('.affine-attachment-toolbar');
await expect(attachmentToolbar).toBeVisible();

// Switches to embed view
await attachmentToolbar.getByRole('button', { name: 'Switch view' }).click();
await attachmentToolbar.getByRole('button', { name: 'Embed view' }).click();

await page.waitForTimeout(500);

const portal = attachment.locator('lit-react-portal');
await expect(portal).toBeVisible();

await attachment.click();

await page.waitForTimeout(500);

const pageCursor = portal.locator('.page-cursor');
expect(await pageCursor.textContent()).toBe('1');
const pageCount = portal.locator('.page-count');
expect(await pageCount.textContent()).toBe('3');

const prevButton = portal.getByRole('button', { name: 'Prev' });
const nextButton = portal.getByRole('button', { name: 'Next' });

await nextButton.click();
expect(await pageCursor.textContent()).toBe('2');

await nextButton.click();
expect(await pageCursor.textContent()).toBe('3');

await prevButton.click();
expect(await pageCursor.textContent()).toBe('2');

// Title alias
{
await page.keyboard.press('Enter');

await page.keyboard.press('@');

const doc0 = page.locator('.linked-doc-popover').getByText('PDF preview');
await doc0.click();

Check failure on line 208 in tests/affine-local/e2e/attachment-preview.spec.ts

View workflow job for this annotation

GitHub Actions / E2E Test (1)

attachment-preview.spec.ts:125:5 › should preview PDF in embed view

1) attachment-preview.spec.ts:125:5 › should preview PDF in embed view ─────────────────────────── TimeoutError: locator.click: Timeout 5000ms exceeded. Call log: - waiting for locator('.linked-doc-popover').getByText('PDF preview') 206 | 207 | const doc0 = page.locator('.linked-doc-popover').getByText('PDF preview'); > 208 | await doc0.click(); | ^ 209 | 210 | await page.keyboard.press('@'); 211 | at /home/runner/work/AFFiNE/AFFiNE/tests/affine-local/e2e/attachment-preview.spec.ts:208:16

Check failure on line 208 in tests/affine-local/e2e/attachment-preview.spec.ts

View workflow job for this annotation

GitHub Actions / E2E Test (1)

attachment-preview.spec.ts:125:5 › should preview PDF in embed view

1) attachment-preview.spec.ts:125:5 › should preview PDF in embed view ─────────────────────────── TimeoutError: locator.click: Timeout 5000ms exceeded. Call log: - waiting for locator('.linked-doc-popover').getByText('PDF preview') 206 | 207 | const doc0 = page.locator('.linked-doc-popover').getByText('PDF preview'); > 208 | await doc0.click(); | ^ 209 | 210 | await page.keyboard.press('@'); 211 | at /home/runner/work/AFFiNE/AFFiNE/tests/affine-local/e2e/attachment-preview.spec.ts:208:16

await page.keyboard.press('@');

const doc1 = page.locator('.linked-doc-popover').getByText('PDF page');
await doc1.click();

const inlineLink = page.locator('affine-reference').nth(0);
const inlineToolbar = page.locator('reference-popup');
const inlineTitle = inlineLink.locator('.affine-reference-title');

await expect(inlineTitle).toHaveText('PDF preview');

const bouding = await inlineLink.boundingBox();
expect(bouding).not.toBeNull();

await page.mouse.move(bouding!.x - 50, bouding!.y + bouding!.height / 2);
await page.waitForTimeout(500);
await page.mouse.click(bouding!.x - 50, bouding!.y + bouding!.height / 2);

await inlineLink.hover({ timeout: 500 });

// Edits title
await inlineToolbar.getByRole('button', { name: 'Edit' }).click();

// Title alias
await page.keyboard.type('PDF embed preview');
await page.keyboard.press('Enter');

await expect(inlineTitle).toHaveText('PDF embed preview');
}

// PDF embed view should not be re-rendered
expect(await pageCursor.textContent()).toBe('2');
expect(await pageCount.textContent()).toBe('3');

// Chagnes origin title
{
const inlineLink = page.locator('affine-reference').nth(1);
const inlineTitle = inlineLink.locator('.affine-reference-title');
await expect(inlineTitle).toHaveText('PDF page');

await title.click();
await page.keyboard.type(' preview');

await expect(inlineTitle).toHaveText('PDF page preview');
}

// PDF embed view should not be re-rendered
expect(await pageCursor.textContent()).toBe('2');
expect(await pageCount.textContent()).toBe('3');
});

0 comments on commit beb61d2

Please sign in to comment.