feat: enhance face-editor positioning (#26303)

feat: enhance face-editor positioning - less overlap

test: timeline with actual video

e2e/src/ui/mock-network/face-editor-network.ts

@@ -0,0 +1,127 @@
+import { BrowserContext } from '@playwright/test';
+import { randomThumbnail } from 'src/ui/generators/timeline';
+
+// Minimal valid H.264 MP4 (8x8px, 1 frame) that browsers can decode to get videoWidth/videoHeight
+const MINIMAL_MP4_BASE64 =
+  'AAAAIGZ0eXBpc29tAAACAGlzb21pc28yYXZjMW1wNDEAAAAIZnJlZQAAAr9tZGF0AAACoAYF//+c' +
+  '3EXpvebZSLeWLNgg2SPu73gyNjQgLSBjb3JlIDEyNSAtIEguMjY0L01QRUctNCBBVkMgY29kZWMg' +
+  'LSBDb3B5bGVmdCAyMDAzLTIwMTIgLSBodHRwOi8vd3d3LnZpZGVvbGFuLm9yZy94MjY0Lmh0bWwg' +
+  'LSBvcHRpb25zOiBjYWJhYz0xIHJlZj0zIGRlYmxvY2s9MTowOjAgYW5hbHlzZT0weDM6MHgxMTMg' +
+  'bWU9aGV4IHN1Ym1lPTcgcHN5PTEgcHN5X3JkPTEuMDA6MC4wMCBtaXhlZF9yZWY9MSBtZV9yYW5n' +
+  'ZT0xNiBjaHJvbWFfbWU9MSB0cmVsbGlzPTEgOHg4ZGN0PTEgY3FtPTAgZGVhZHpvbmU9MjEsMTEg' +
+  'ZmFzdF9wc2tpcD0xIGNocm9tYV9xcF9vZmZzZXQ9LTIgdGhyZWFkcz02IGxvb2thaGVhZF90aHJl' +
+  'YWRzPTEgc2xpY2VkX3RocmVhZHM9MCBucj0wIGRlY2ltYXRlPTEgaW50ZXJsYWNlZD0wIGJsdXJh' +
+  'eV9jb21wYXQ9MCBjb25zdHJhaW5lZF9pbnRyYT0wIGJmcmFtZXM9MyBiX3B5cmFtaWQ9MiBiX2Fk' +
+  'YXB0PTEgYl9iaWFzPTAgZGlyZWN0PTEgd2VpZ2h0Yj0xIG9wZW5fZ29wPTAgd2VpZ2h0cD0yIGtl' +
+  'eWludD0yNTAga2V5aW50X21pbj0yNCBzY2VuZWN1dD00MCBpbnRyYV9yZWZyZXNoPTAgcmNfbG9v' +
+  'a2FoZWFkPTQwIHJjPWNyZiBtYnRyZWU9MSBjcmY9MjMuMCBxY29tcD0wLjYwIHFwbWluPTAgcXBt' +
+  'YXg9NjkgcXBzdGVwPTQgaXBfcmF0aW89MS40MCBhcT0xOjEuMDAAgAAAAA9liIQAV/0TAAYdeBTX' +
+  'zg8AAALvbW9vdgAAAGxtdmhkAAAAAAAAAAAAAAAAAAAD6AAAACoAAQAAAQAAAAAAAAAAAAAAAAEAAAAA' +
+  'AAAAAAAAAAAAAAABAAAAAAAAAAAAAAAAAABAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAgAA' +
+  'Ahl0cmFrAAAAXHRraGQAAAAPAAAAAAAAAAAAAAABAAAAAAAAACoAAAAAAAAAAAAAAAAAAAAAAAEAAAAA' +
+  'AAAAAAAAAAAAAAABAAAAAAAAAAAAAAAAAABAAAAAAAgAAAAIAAAAAAAkZWR0cwAAABxlbHN0AAAAAAAA' +
+  'AAEAAAAqAAAAAAABAAAAAAGRbWRpYQAAACBtZGhkAAAAAAAAAAAAAAAAAAAwAAAAAgBVxAAAAAAA' +
+  'LWhkbHIAAAAAAAAAAHZpZGUAAAAAAAAAAAAAAABWaWRlb0hhbmRsZXIAAAABPG1pbmYAAAAUdm1oZAAA' +
+  'AAEAAAAAAAAAAAAAACRkaW5mAAAAHGRyZWYAAAAAAAAAAQAAAAx1cmwgAAAAAQAAAPxzdGJsAAAAmHN0' +
+  'c2QAAAAAAAAAAQAAAIhhdmMxAAAAAAAAAAEAAAAAAAAAAAAAAAAAAAAAAAgACABIAAAASAAAAAAAAAAB' +
+  'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAGP//AAAAMmF2Y0MBZAAK/+EAGWdkAAqs' +
+  '2V+WXAWyAAADAAIAAAMAYB4kSywBAAZo6+PLIsAAAAAYc3R0cwAAAAAAAAABAAAAAQAAAgAAAAAcc3Rz' +
+  'YwAAAAAAAAABAAAAAQAAAAEAAAABAAAAFHN0c3oAAAAAAAACtwAAAAEAAAAUc3RjbwAAAAAAAAABAAAA' +
+  'MAAAAGJ1ZHRhAAAAWm1ldGEAAAAAAAAAIWhkbHIAAAAAAAAAAG1kaXJhcHBsAAAAAAAAAAAAAAAALWls' +
+  'c3QAAAAlqXRvbwAAAB1kYXRhAAAAAQAAAABMYXZmNTQuNjMuMTA0';
+
+export const MINIMAL_MP4_BUFFER = Buffer.from(MINIMAL_MP4_BASE64, 'base64');
+
+export type MockPerson = {
+  id: string;
+  name: string;
+  birthDate: string | null;
+  isHidden: boolean;
+  thumbnailPath: string;
+  updatedAt: string;
+};
+
+export const createMockPeople = (count: number): MockPerson[] => {
+  const names = [
+    'Alice Johnson',
+    'Bob Smith',
+    'Charlie Brown',
+    'Diana Prince',
+    'Eve Adams',
+    'Frank Castle',
+    'Grace Lee',
+    'Hank Pym',
+    'Iris West',
+    'Jack Ryan',
+  ];
+  return Array.from({ length: count }, (_, index) => ({
+    id: `person-${index}`,
+    name: names[index % names.length],
+    birthDate: null,
+    isHidden: false,
+    thumbnailPath: `/upload/thumbs/person-${index}.jpeg`,
+    updatedAt: '2025-01-01T00:00:00.000Z',
+  }));
+};
+
+export type FaceCreateCapture = {
+  requests: Array<{
+    assetId: string;
+    personId: string;
+    x: number;
+    y: number;
+    width: number;
+    height: number;
+    imageWidth: number;
+    imageHeight: number;
+  }>;
+};
+
+export const setupFaceEditorMockApiRoutes = async (
+  context: BrowserContext,
+  mockPeople: MockPerson[],
+  faceCreateCapture: FaceCreateCapture,
+) => {
+  await context.route('**/api/people?*', async (route, request) => {
+    if (request.method() !== 'GET') {
+      return route.fallback();
+    }
+
+    return route.fulfill({
+      status: 200,
+      contentType: 'application/json',
+      json: {
+        hasNextPage: false,
+        hidden: 0,
+        people: mockPeople,
+        total: mockPeople.length,
+      },
+    });
+  });
+
+  await context.route('**/api/faces', async (route, request) => {
+    if (request.method() !== 'POST') {
+      return route.fallback();
+    }
+
+    const body = request.postDataJSON();
+    faceCreateCapture.requests.push(body);
+
+    return route.fulfill({
+      status: 201,
+      contentType: 'text/plain',
+      body: 'OK',
+    });
+  });
+
+  await context.route('**/api/people/*/thumbnail', async (route) => {
+    if (!route.request().serviceWorker()) {
+      return route.continue();
+    }
+    return route.fulfill({
+      status: 200,
+      headers: { 'content-type': 'image/jpeg' },
+      body: await randomThumbnail('person-thumb', 1),
+    });
+  });
+};

e2e/src/ui/mock-network/timeline-network.ts

@@ -12,6 +12,7 @@ import {
   TimelineData,
 } from 'src/ui/generators/timeline';
 import { sleep } from 'src/ui/specs/timeline/utils';
+import { MINIMAL_MP4_BUFFER } from './face-editor-network';
 
 export class TimelineTestContext {
   slowBucket = false;
@@ -135,6 +136,14 @@ export const setupTimelineMockApiRoutes = async (
     return route.continue();
   });
 
+  await context.route('**/api/assets/*/video/playback*', async (route) => {
+    return route.fulfill({
+      status: 200,
+      headers: { 'content-type': 'video/mp4' },
+      body: MINIMAL_MP4_BUFFER,
+    });
+  });
+
   await context.route('**/api/albums/**', async (route, request) => {
     const albumsMatch = request.url().match(/\/api\/albums\/(?<albumId>[^/?]+)/);
     if (albumsMatch) {

e2e/src/ui/specs/asset-viewer/face-editor.e2e-spec.ts

@@ -0,0 +1,285 @@
+import { expect, Page, test } from '@playwright/test';
+import { SeededRandom, selectRandom, TimelineAssetConfig } from 'src/ui/generators/timeline';
+import {
+  createMockPeople,
+  FaceCreateCapture,
+  MockPerson,
+  setupFaceEditorMockApiRoutes,
+} from 'src/ui/mock-network/face-editor-network';
+import { assetViewerUtils } from '../timeline/utils';
+import { setupAssetViewerFixture } from './utils';
+
+const waitForSelectorTransition = async (page: Page) => {
+  await page.waitForFunction(
+    () => {
+      const selector = document.querySelector('#face-selector') as HTMLElement | null;
+      if (!selector) {
+        return false;
+      }
+      return selector.getAnimations({ subtree: false }).every((animation) => animation.playState === 'finished');
+    },
+    undefined,
+    { timeout: 1000, polling: 50 },
+  );
+};
+
+const openFaceEditor = async (page: Page, asset: TimelineAssetConfig) => {
+  await page.goto(`/photos/${asset.id}`);
+  await assetViewerUtils.waitForViewerLoad(page, asset);
+  await page.keyboard.press('i');
+  await page.locator('#detail-panel').waitFor({ state: 'visible' });
+  await page.getByLabel('Tag people').click();
+  await page.locator('#face-selector').waitFor({ state: 'visible' });
+  await waitForSelectorTransition(page);
+};
+
+test.describe.configure({ mode: 'parallel' });
+test.describe('face-editor', () => {
+  const fixture = setupAssetViewerFixture(777);
+  const rng = new SeededRandom(777);
+  let mockPeople: MockPerson[];
+  let faceCreateCapture: FaceCreateCapture;
+
+  test.beforeAll(async () => {
+    mockPeople = createMockPeople(8);
+  });
+
+  test.beforeEach(async ({ context }) => {
+    faceCreateCapture = { requests: [] };
+    await setupFaceEditorMockApiRoutes(context, mockPeople, faceCreateCapture);
+  });
+
+  type ScreenRect = { top: number; left: number; width: number; height: number };
+
+  const getFaceBoxRect = async (page: Page): Promise<ScreenRect> => {
+    const dataEl = page.locator('#face-editor-data');
+    await expect(dataEl).toHaveAttribute('data-face-left', /^-?\d+/);
+    await expect(dataEl).toHaveAttribute('data-face-top', /^-?\d+/);
+    await expect(dataEl).toHaveAttribute('data-face-width', /^[1-9]/);
+    await expect(dataEl).toHaveAttribute('data-face-height', /^[1-9]/);
+    const canvasBox = await page.locator('#face-editor').boundingBox();
+    if (!canvasBox) {
+      throw new Error('Canvas element not found');
+    }
+    const left = Number(await dataEl.getAttribute('data-face-left'));
+    const top = Number(await dataEl.getAttribute('data-face-top'));
+    const width = Number(await dataEl.getAttribute('data-face-width'));
+    const height = Number(await dataEl.getAttribute('data-face-height'));
+    return {
+      top: canvasBox.y + top,
+      left: canvasBox.x + left,
+      width,
+      height,
+    };
+  };
+
+  const getSelectorRect = async (page: Page): Promise<ScreenRect> => {
+    const box = await page.locator('#face-selector').boundingBox();
+    if (!box) {
+      throw new Error('Face selector element not found');
+    }
+    return { top: box.y, left: box.x, width: box.width, height: box.height };
+  };
+
+  const computeOverlapArea = (a: ScreenRect, b: ScreenRect): number => {
+    const overlapX = Math.max(0, Math.min(a.left + a.width, b.left + b.width) - Math.max(a.left, b.left));
+    const overlapY = Math.max(0, Math.min(a.top + a.height, b.top + b.height) - Math.max(a.top, b.top));
+    return overlapX * overlapY;
+  };
+
+  const dragFaceBox = async (page: Page, deltaX: number, deltaY: number) => {
+    const faceBox = await getFaceBoxRect(page);
+    const centerX = faceBox.left + faceBox.width / 2;
+    const centerY = faceBox.top + faceBox.height / 2;
+    await page.mouse.move(centerX, centerY);
+    await page.mouse.down();
+    await page.mouse.move(centerX + deltaX, centerY + deltaY, { steps: 5 });
+    await page.mouse.up();
+    await page.waitForTimeout(300);
+  };
+
+  test('Face editor opens with person list', async ({ page }) => {
+    const asset = selectRandom(fixture.assets, rng);
+    await openFaceEditor(page, asset);
+
+    await expect(page.locator('#face-selector')).toBeVisible();
+    await expect(page.locator('#face-editor')).toBeVisible();
+
+    for (const person of mockPeople) {
+      await expect(page.locator('#face-selector').getByText(person.name)).toBeVisible();
+    }
+  });
+
+  test('Search filters people by name', async ({ page }) => {
+    const asset = selectRandom(fixture.assets, rng);
+    await openFaceEditor(page, asset);
+
+    const searchInput = page.locator('#face-selector input');
+    await searchInput.fill('Alice');
+
+    await expect(page.locator('#face-selector').getByText('Alice Johnson')).toBeVisible();
+    await expect(page.locator('#face-selector').getByText('Bob Smith')).toBeHidden();
+
+    await searchInput.clear();
+
+    for (const person of mockPeople) {
+      await expect(page.locator('#face-selector').getByText(person.name)).toBeVisible();
+    }
+  });
+
+  test('Search with no results shows empty message', async ({ page }) => {
+    const asset = selectRandom(fixture.assets, rng);
+    await openFaceEditor(page, asset);
+
+    const searchInput = page.locator('#face-selector input');
+    await searchInput.fill('Nonexistent Person XYZ');
+
+    for (const person of mockPeople) {
+      await expect(page.locator('#face-selector').getByText(person.name)).toBeHidden();
+    }
+  });
+
+  test('Selecting a person shows confirmation dialog', async ({ page }) => {
+    const asset = selectRandom(fixture.assets, rng);
+    await openFaceEditor(page, asset);
+
+    const personToTag = mockPeople[0];
+    await page.locator('#face-selector').getByText(personToTag.name).click();
+
+    await expect(page.getByRole('dialog')).toBeVisible();
+  });
+
+  test('Confirming tag calls createFace API and closes editor', async ({ page }) => {
+    const asset = selectRandom(fixture.assets, rng);
+    await openFaceEditor(page, asset);
+
+    const personToTag = mockPeople[0];
+    await page.locator('#face-selector').getByText(personToTag.name).click();
+
+    await expect(page.getByRole('dialog')).toBeVisible();
+    await page.getByRole('button', { name: /confirm/i }).click();
+
+    await expect(page.locator('#face-selector')).toBeHidden();
+    await expect(page.locator('#face-editor')).toBeHidden();
+
+    expect(faceCreateCapture.requests).toHaveLength(1);
+    expect(faceCreateCapture.requests[0].assetId).toBe(asset.id);
+    expect(faceCreateCapture.requests[0].personId).toBe(personToTag.id);
+  });
+
+  test('Cancel button closes face editor', async ({ page }) => {
+    const asset = selectRandom(fixture.assets, rng);
+    await openFaceEditor(page, asset);
+
+    await expect(page.locator('#face-selector')).toBeVisible();
+    await expect(page.locator('#face-editor')).toBeVisible();
+
+    await page.getByRole('button', { name: /cancel/i }).click();
+
+    await expect(page.locator('#face-selector')).toBeHidden();
+    await expect(page.locator('#face-editor')).toBeHidden();
+  });
+
+  test('Selector does not overlap face box on initial open', async ({ page }) => {
+    const asset = selectRandom(fixture.assets, rng);
+    await openFaceEditor(page, asset);
+
+    const faceBox = await getFaceBoxRect(page);
+    const selectorBox = await getSelectorRect(page);
+    const overlap = computeOverlapArea(faceBox, selectorBox);
+
+    expect(overlap).toBe(0);
+  });
+
+  test('Selector repositions without overlap after dragging face box down', async ({ page }) => {
+    const asset = selectRandom(fixture.assets, rng);
+    await openFaceEditor(page, asset);
+
+    await dragFaceBox(page, 0, 150);
+
+    const faceBox = await getFaceBoxRect(page);
+    const selectorBox = await getSelectorRect(page);
+    const overlap = computeOverlapArea(faceBox, selectorBox);
+
+    expect(overlap).toBe(0);
+  });
+
+  test('Selector repositions without overlap after dragging face box right', async ({ page }) => {
+    const asset = selectRandom(fixture.assets, rng);
+    await openFaceEditor(page, asset);
+
+    await dragFaceBox(page, 200, 0);
+
+    const faceBox = await getFaceBoxRect(page);
+    const selectorBox = await getSelectorRect(page);
+    const overlap = computeOverlapArea(faceBox, selectorBox);
+
+    expect(overlap).toBe(0);
+  });
+
+  test('Selector repositions without overlap after dragging face box to top-left corner', async ({ page }) => {
+    const asset = selectRandom(fixture.assets, rng);
+    await openFaceEditor(page, asset);
+
+    await dragFaceBox(page, -300, -300);
+
+    const faceBox = await getFaceBoxRect(page);
+    const selectorBox = await getSelectorRect(page);
+    const overlap = computeOverlapArea(faceBox, selectorBox);
+
+    expect(overlap).toBe(0);
+  });
+
+  test('Selector repositions without overlap after dragging face box to bottom-right', async ({ page }) => {
+    const asset = selectRandom(fixture.assets, rng);
+    await openFaceEditor(page, asset);
+
+    await dragFaceBox(page, 300, 300);
+
+    const faceBox = await getFaceBoxRect(page);
+    const selectorBox = await getSelectorRect(page);
+    const overlap = computeOverlapArea(faceBox, selectorBox);
+
+    expect(overlap).toBe(0);
+  });
+
+  test('Selector stays within viewport bounds', async ({ page }) => {
+    const asset = selectRandom(fixture.assets, rng);
+    await openFaceEditor(page, asset);
+
+    const viewportSize = page.viewportSize()!;
+    const selectorBox = await getSelectorRect(page);
+
+    expect(selectorBox.top).toBeGreaterThanOrEqual(0);
+    expect(selectorBox.left).toBeGreaterThanOrEqual(0);
+    expect(selectorBox.top + selectorBox.height).toBeLessThanOrEqual(viewportSize.height);
+    expect(selectorBox.left + selectorBox.width).toBeLessThanOrEqual(viewportSize.width);
+  });
+
+  test('Selector stays within viewport after dragging to edge', async ({ page }) => {
+    const asset = selectRandom(fixture.assets, rng);
+    await openFaceEditor(page, asset);
+
+    await dragFaceBox(page, -400, -400);
+
+    const viewportSize = page.viewportSize()!;
+    const selectorBox = await getSelectorRect(page);
+
+    expect(selectorBox.top).toBeGreaterThanOrEqual(0);
+    expect(selectorBox.left).toBeGreaterThanOrEqual(0);
+    expect(selectorBox.top + selectorBox.height).toBeLessThanOrEqual(viewportSize.height);
+    expect(selectorBox.left + selectorBox.width).toBeLessThanOrEqual(viewportSize.width);
+  });
+
+  test('Face box is draggable on the canvas', async ({ page }) => {
+    const asset = selectRandom(fixture.assets, rng);
+    await openFaceEditor(page, asset);
+
+    const beforeDrag = await getFaceBoxRect(page);
+    await dragFaceBox(page, 100, 50);
+    const afterDrag = await getFaceBoxRect(page);
+
+    expect(afterDrag.left).toBeGreaterThan(beforeDrag.left + 50);
+    expect(afterDrag.top).toBeGreaterThan(beforeDrag.top + 20);
+  });
+});