-
Notifications
You must be signed in to change notification settings - Fork 590
✨(frontend) add the presenter mode feature #2321
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
bd7960e
c29ef83
9dfb220
15e9e12
38d10bd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,265 @@ | ||
| import { Page, expect, test } from '@playwright/test'; | ||
|
|
||
| import { createDoc, goToGridDoc, mockedDocument } from './utils-common'; | ||
| import { openSuggestionMenu, writeInEditor } from './utils-editor'; | ||
|
|
||
| const openPresenter = async (page: Page) => { | ||
| await page.getByLabel('Open the document options').click(); | ||
| await page.getByRole('menuitem', { name: 'Present' }).click(); | ||
|
|
||
| const overlay = page.getByRole('dialog', { name: 'Presenter mode' }); | ||
| await expect(overlay).toBeVisible(); | ||
| return overlay; | ||
| }; | ||
|
|
||
| const insertDivider = async (page: Page) => { | ||
| const { suggestionMenu } = await openSuggestionMenu({ page }); | ||
| await suggestionMenu.getByText('Divider', { exact: true }).click(); | ||
| }; | ||
|
|
||
| const writeMultiSlideDoc = async (page: Page) => { | ||
| const editor = await writeInEditor({ page, text: 'Slide one' }); | ||
| await editor.press('Enter'); | ||
| await insertDivider(page); | ||
| await editor.press('Enter'); | ||
| await writeInEditor({ page, text: 'Slide two' }); | ||
| await editor.press('Enter'); | ||
| await insertDivider(page); | ||
| await editor.press('Enter'); | ||
| await writeInEditor({ page, text: 'Slide three' }); | ||
| }; | ||
|
|
||
| test.beforeEach(async ({ page }) => { | ||
| await page.goto('/'); | ||
| }); | ||
|
|
||
| test.describe('Presenter Mode', () => { | ||
| test('opens the presenter overlay from the doc options menu and closes with Escape', async ({ | ||
| page, | ||
| browserName, | ||
| }) => { | ||
| await createDoc(page, 'presenter-open', browserName, 1); | ||
| await writeInEditor({ page, text: 'Hello presenter' }); | ||
|
|
||
| const overlay = await openPresenter(page); | ||
|
|
||
| await expect( | ||
| overlay.getByRole('toolbar', { name: 'Presenter controls' }), | ||
| ).toBeVisible(); | ||
| await expect(overlay.getByText('Hello presenter')).toBeVisible(); | ||
|
|
||
| // The presenter calls requestFullscreen on open. usePresenterShortcuts | ||
| // deliberately ignores Escape while in fullscreen (the browser owns Esc | ||
| // there to exit). Playwright grants requestFullscreen but, unlike a real | ||
| // browser, does NOT dispatch a fullscreen exit on Escape — so we must | ||
| // exit fullscreen ourselves before pressing Escape to test the close | ||
| // path. We also wait for the React state to settle after the exit so | ||
| // the keydown listener is re-bound with isFullscreen=false. | ||
| await page.evaluate(async () => { | ||
| if (document.fullscreenElement) { | ||
| await document.exitFullscreen(); | ||
| } | ||
| }); | ||
| await page.waitForTimeout(200); | ||
| await page.keyboard.press('Escape'); | ||
| await expect(overlay).toBeHidden(); | ||
| }); | ||
|
|
||
| test('renders a single-slide doc with counter 1/1 and disabled nav buttons', async ({ | ||
| page, | ||
| browserName, | ||
| }) => { | ||
| await createDoc(page, 'presenter-single', browserName, 1); | ||
| await writeInEditor({ page, text: 'Slide A' }); | ||
|
|
||
| const overlay = await openPresenter(page); | ||
|
|
||
| await expect(overlay.getByText('1 / 1')).toBeVisible(); | ||
| await expect( | ||
| overlay.getByRole('button', { name: 'Previous slide' }), | ||
| ).toBeDisabled(); | ||
| await expect( | ||
| overlay.getByRole('button', { name: 'Next slide' }), | ||
| ).toBeDisabled(); | ||
| await expect(overlay.getByText('Slide A')).toBeVisible(); | ||
|
|
||
| await overlay.getByRole('button', { name: 'Close presenter' }).click(); | ||
| await expect(overlay).toBeHidden(); | ||
| }); | ||
|
|
||
| test('navigates between slides via the floating bar buttons', async ({ | ||
| page, | ||
| browserName, | ||
| }) => { | ||
| await createDoc(page, 'presenter-nav-bar', browserName, 1); | ||
| await writeMultiSlideDoc(page); | ||
|
|
||
| const overlay = await openPresenter(page); | ||
|
|
||
| const prev = overlay.getByRole('button', { name: 'Previous slide' }); | ||
| const next = overlay.getByRole('button', { name: 'Next slide' }); | ||
|
|
||
| await expect(overlay.getByText('1 / 3')).toBeVisible(); | ||
| await expect(overlay.getByText('Slide one')).toBeVisible(); | ||
| await expect(prev).toBeDisabled(); | ||
| await expect(next).toBeEnabled(); | ||
|
|
||
| await next.click(); | ||
| await expect(overlay.getByText('2 / 3')).toBeVisible(); | ||
| await expect(overlay.getByText('Slide two')).toBeVisible(); | ||
|
|
||
| await next.click(); | ||
| await expect(overlay.getByText('3 / 3')).toBeVisible(); | ||
| await expect(overlay.getByText('Slide three')).toBeVisible(); | ||
| await expect(next).toBeDisabled(); | ||
| await expect(prev).toBeEnabled(); | ||
|
|
||
| await prev.click(); | ||
| await expect(overlay.getByText('2 / 3')).toBeVisible(); | ||
| await expect(overlay.getByText('Slide two')).toBeVisible(); | ||
| }); | ||
|
|
||
| test('navigates between slides via keyboard shortcuts', async ({ | ||
| page, | ||
| browserName, | ||
| }) => { | ||
| await createDoc(page, 'presenter-nav-keyboard', browserName, 1); | ||
| await writeMultiSlideDoc(page); | ||
|
|
||
| const overlay = await openPresenter(page); | ||
|
|
||
| await expect(overlay.getByText('1 / 3')).toBeVisible(); | ||
|
|
||
| await page.keyboard.press('ArrowRight'); | ||
| await expect(overlay.getByText('2 / 3')).toBeVisible(); | ||
|
|
||
| await page.keyboard.press('End'); | ||
| await expect(overlay.getByText('3 / 3')).toBeVisible(); | ||
|
|
||
| await page.keyboard.press('Home'); | ||
| await expect(overlay.getByText('1 / 3')).toBeVisible(); | ||
|
|
||
| // ArrowLeft on the first slide is clamped — counter stays at 1 / 3. | ||
| await page.keyboard.press('ArrowLeft'); | ||
| await expect(overlay.getByText('1 / 3')).toBeVisible(); | ||
| }); | ||
|
|
||
| test('scales each slide to fit the viewport (outer width = 900 × scale)', async ({ | ||
| page, | ||
| browserName, | ||
| }) => { | ||
| await createDoc(page, 'presenter-scaling', browserName, 1); | ||
| await writeInEditor({ page, text: 'A short slide' }); | ||
|
|
||
| const overlay = await openPresenter(page); | ||
| const currentSlide = overlay.getByRole('group').filter({ hasNotText: '' }); | ||
| await expect(currentSlide.first()).toBeVisible(); | ||
|
|
||
| const dims = await currentSlide.first().evaluate((el) => { | ||
| // DOM: <outer role="group"><stage><inner (scaled)>... | ||
| const stage = el.firstElementChild as HTMLElement | null; | ||
| const inner = stage?.firstElementChild as HTMLElement | null; | ||
| const innerStyle = inner ? getComputedStyle(inner) : null; | ||
| return { | ||
| outerWidth: el.getBoundingClientRect().width, | ||
| innerTransform: innerStyle?.transform ?? 'none', | ||
| }; | ||
| }); | ||
|
|
||
| // Inner has `transform: scale(<n>)`; match the first scale matrix value. | ||
| const m = /matrix\(([-\d.]+)/.exec(dims.innerTransform); | ||
| expect( | ||
| m, | ||
| `expected a scale matrix, got: ${dims.innerTransform}`, | ||
| ).not.toBeNull(); | ||
|
|
||
| const scale = m ? parseFloat(m[1]) : NaN; | ||
|
|
||
| // Scale is always clamped to [MIN_SCALE, MAX_SCALE] = [0.7, 1.5]. | ||
| // The exact value depends on viewport: sparse content typically saturates | ||
| // the height-based scale (→ MAX), but at default Playwright viewport | ||
| // (1280) the width path can constrain to scaleW = (1280 - 2*paddingX)/900. | ||
| // We assert the bounds, not a specific value. | ||
| expect(scale).toBeGreaterThanOrEqual(0.7); | ||
| expect(scale).toBeLessThanOrEqual(1.5); | ||
|
|
||
| // The core invariant: outer width = DESIGN_WIDTH (900) × scale, | ||
| // within a 5px tolerance for sub-pixel rounding. | ||
| expect(Math.abs(dims.outerWidth - 900 * scale)).toBeLessThan(5); | ||
|
Comment on lines
+186
to
+188
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Verify the design-width constant value used by the scaler.
fd -t f 'constants.ts' --full-path doc-presenter --exec rg -nP 'DESIGN_WIDTH|WINDOW_RADIUS|MIN_SCALE|MAX_SCALE|PADDING' {}
rg -nP 'DESIGN_WIDTH' src/frontend/apps/impress/src/features/docs/doc-presenterRepository: suitenumerique/docs Length of output: 834 🏁 Script executed: #!/bin/bash
# Inspect the assertion in the spec and look for 800/900 and constant usage.
sed -n '160,210p' src/frontend/apps/e2e/__tests__/app-impress/presenter-mode.spec.ts
rg -n "800|900|PRESENTER_SLIDE_DESIGN_WIDTH|DESIGN_WIDTH" src/frontend/apps/e2e/__tests__/app-impress/presenter-mode.spec.ts
# Show the constant definition to confirm the source of truth.
sed -n '1,80p' src/frontend/apps/impress/src/features/docs/doc-presenter/constants.tsRepository: suitenumerique/docs Length of output: 3931 Hard-coded 🤖 Prompt for AI Agents |
||
| }); | ||
|
|
||
| test('tall slide produces a vertical scrollbar on the outer wrapper with the top visible', async ({ | ||
| page, | ||
| browserName, | ||
| }) => { | ||
| await createDoc(page, 'presenter-tall', browserName, 1); | ||
|
|
||
| // Build a tall single-slide doc: many headings + paragraphs so the | ||
| // natural content height blows past viewport height at MIN_SCALE. | ||
| const editor = await writeInEditor({ page, text: 'TOP MARKER' }); | ||
| for (let i = 0; i < 40; i += 1) { | ||
| await editor.press('Enter'); | ||
| await editor.pressSequentially(`Filler line ${i} to make the slide tall`); | ||
| } | ||
|
|
||
| const overlay = await openPresenter(page); | ||
| const slide = overlay.getByRole('group').filter({ hasNotText: '' }).first(); | ||
| await expect(slide).toBeVisible(); | ||
|
|
||
| // The first block ('TOP MARKER') must be at y=0 of the slide wrapper | ||
| // (i.e. visible at the top, not clipped). This is the regression we fix. | ||
| const topVisible = await slide.evaluate((el) => { | ||
| el.scrollTop = 0; | ||
| const first = el.querySelector('.bn-block-content'); | ||
| if (!first) { | ||
| return { ok: false, reason: 'no first block' }; | ||
| } | ||
| const slideRect = el.getBoundingClientRect(); | ||
| const blockRect = first.getBoundingClientRect(); | ||
| return { | ||
| ok: blockRect.top >= slideRect.top - 1, | ||
| slideTop: slideRect.top, | ||
| blockTop: blockRect.top, | ||
| scrollHeight: el.scrollHeight, | ||
| clientHeight: el.clientHeight, | ||
| }; | ||
| }); | ||
|
|
||
| expect(topVisible.ok, JSON.stringify(topVisible)).toBe(true); | ||
| expect(topVisible.scrollHeight ?? 0).toBeGreaterThan( | ||
| topVisible.clientHeight ?? 0, | ||
| ); | ||
| }); | ||
| }); | ||
|
|
||
| test.describe('Presenter Mode mobile', () => { | ||
| test.use({ viewport: { width: 500, height: 1200 } }); | ||
|
|
||
| test.beforeEach(async ({ page }) => { | ||
| await page.goto('/'); | ||
| }); | ||
|
|
||
| test('hides the Present option on small mobile viewports', async ({ | ||
| page, | ||
| }) => { | ||
| await mockedDocument(page, { | ||
| abilities: { | ||
| destroy: true, | ||
| link_configuration: true, | ||
| versions_destroy: true, | ||
| versions_list: true, | ||
| versions_retrieve: true, | ||
| accesses_manage: true, | ||
| accesses_view: true, | ||
| update: true, | ||
| partial_update: true, | ||
| retrieve: true, | ||
| }, | ||
| }); | ||
|
|
||
| await goToGridDoc(page); | ||
|
|
||
| await page.getByLabel('Open the document options').click(); | ||
| await expect(page.getByRole('menuitem', { name: 'Present' })).toBeHidden(); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| import { Button, useModal } from '@gouvfr-lasuite/cunningham-react'; | ||
| import { useTreeContext } from '@gouvfr-lasuite/ui-kit'; | ||
| import { Present } from '@gouvfr-lasuite/ui-kit/icons'; | ||
| import dynamic from 'next/dynamic'; | ||
| import { useRouter } from 'next/router'; | ||
| import { useState } from 'react'; | ||
|
|
@@ -79,6 +80,14 @@ const ModalExport = | |
| ) | ||
| : null; | ||
|
|
||
| const PresenterOverlay = dynamic( | ||
| () => | ||
| import('@/docs/doc-presenter').then((mod) => ({ | ||
| default: mod.PresenterOverlay, | ||
| })), | ||
| { ssr: false }, | ||
| ); | ||
|
|
||
| interface DocToolBoxProps { | ||
| doc: Doc; | ||
| } | ||
|
|
@@ -93,6 +102,7 @@ export const DocToolBox = ({ doc }: DocToolBoxProps) => { | |
|
|
||
| const [isModalRemoveOpen, setIsModalRemoveOpen] = useState(false); | ||
| const [isModalExportOpen, setIsModalExportOpen] = useState(false); | ||
| const [isPresenterOpen, setIsPresenterOpen] = useState(false); | ||
| const selectHistoryModal = useModal(); | ||
| const modalShare = useModal(); | ||
|
|
||
|
|
@@ -176,6 +186,15 @@ export const DocToolBox = ({ doc }: DocToolBoxProps) => { | |
| showSeparator: true, | ||
| show: !emoji && doc.abilities.partial_update && !isTopRoot, | ||
| }, | ||
| { | ||
| label: t('Present'), | ||
| icon: <Present />, | ||
| callback: () => { | ||
| setIsPresenterOpen(true); | ||
| }, | ||
| show: !doc.deleted_at && !isSmallMobile, | ||
| testId: `docs-actions-present-${doc.id}`, | ||
| }, | ||
|
Comment on lines
+189
to
+197
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🌐 Web query:
💡 Result: I couldn’t find any authoritative, component-level documentation for a dedicated “Icon” component inside Citations:
🏁 Script executed: #!/bin/bash
set -euo pipefail
# 1) Inspect the exact surrounding code in DocToolBox.tsx
FILE="src/frontend/apps/impress/src/features/docs/doc-header/components/DocToolBox.tsx"
if [ -f "$FILE" ]; then
echo "=== $FILE (lines around 175-215) ==="
sed -n '165,215p' "$FILE" | nl -ba | sed 's/^/ /'
else
echo "Missing file: $FILE"
fi
# 2) Find the import source for Present and other icons in this file
echo "=== Imports in $FILE (top part) ==="
sed -n '1,120p' "$FILE" | nl -ba | sed 's/^/ /'
# 3) Locate where Present is used/imported elsewhere in the repo
echo "=== Repo-wide search for Present icon usage ==="
rg -n "Present\s*[>,)]|<Present\b|from ['\"]`@gouvfr-lasuite/ui-kit/icons`['\"][^;]*|`@gouvfr-lasuite/ui-kit/icons`" -S .
# 4) If node_modules is present in the sandbox, inspect ui-kit types/implementation for Present
if [ -d "node_modules/@gouvfr-lasuite/ui-kit" ]; then
echo "=== node_modules/@gouvfr-lasuite/ui-kit present types/implementation ==="
# Try to find Present export
rg -n "export (const|function|class) Present|Present\s*:" node_modules/@gouvfr-lasuite/ui-kit -S || true
# Try to find icon component prop types for width/height/aria-hidden
rg -n "aria-hidden|width\s*\?|height\s*\?" node_modules/@gouvfr-lasuite/ui-kit -S || true
else
echo "node_modules/@gouvfr-lasuite/ui-kit not present in this sandbox."
fiRepository: suitenumerique/docs Length of output: 236 🏁 Script executed: #!/bin/bash
set -euo pipefail
FILE="src/frontend/apps/impress/src/features/docs/doc-header/components/DocToolBox.tsx"
echo "=== Verify file exists ==="
ls -la "$FILE" 2>/dev/null || { echo "Missing file: $FILE"; exit 1; }
echo "=== $FILE (lines 175-215) ==="
sed -n '175,215p' "$FILE" | cat -n
echo "=== $FILE imports (lines 1-120) ==="
sed -n '1,120p' "$FILE" | cat -n
echo "=== Repo-wide search: where Present is imported/used ==="
rg -n --hidden --glob '!.git/*' --glob '!**/dist/**' --glob '!**/build/**' \
"from ['\"]`@gouvfr-lasuite/ui-kit/icons`['\"][^;]*Present|<Present\b|Present\s*\(" .
echo "=== Repo-wide search: icon size/accessibility patterns used elsewhere ==="
rg -n --hidden --glob '!.git/*' --glob '!**/dist/**' --glob '!**/build/**' \
"width=\{24\}\s*height=\{24\}|aria-hidden=\{?\"true\"\\}?" \
src/frontend/apps/impress/src/features/docs/doc-header/components/DocToolBox.tsx
# Inspect node_modules types/impl if available (best-effort)
if [ -d "node_modules/@gouvfr-lasuite/ui-kit" ]; then
echo "=== node_modules/@gouvfr-lasuite/ui-kit exists; searching for Present ==="
rg -n "Present" node_modules/@gouvfr-lasuite/ui-kit -S | head -n 50 || true
rg -n "export .*Present|Present\s*:" node_modules/@gouvfr-lasuite/ui-kit -S | head -n 50 || true
else
echo "=== node_modules/@gouvfr-lasuite/ui-kit not present ==="
fiRepository: suitenumerique/docs Length of output: 7833 🌐 Web query:
💡 Result: I couldn’t find any authoritative, source-backed documentation stating specific default width/height values for the icons provided by Citations: 🌐 Web query:
💡 Result: There is no public npm package named Citations:
🌐 Web query:
💡 Result: I couldn’t find any public GitHub repository/package named What is public under the La Suite numérique GitHub org is
So, in the public codebase I can access, there isn’t a “Present” icon component with If you share the exact GitHub repo URL (or the file path for the icon component), I can extract the precise props from the source. Citations: 🌐 Web query:
💡 Result: I can’t find (in the sources I retrieved) any documentation or code that matches the exact query phrase “export Present Citations:
Make the In 🤖 Prompt for AI Agents |
||
| { | ||
| label: t('Copy link'), | ||
| icon: <AddLinkSVG width={24} height={24} aria-hidden="true" />, | ||
|
|
@@ -320,6 +339,15 @@ export const DocToolBox = ({ doc }: DocToolBoxProps) => { | |
| doc={doc} | ||
| /> | ||
| )} | ||
| {isPresenterOpen && ( | ||
| <PresenterOverlay | ||
| doc={doc} | ||
| onClose={() => { | ||
| setIsPresenterOpen(false); | ||
| restoreFocus(); | ||
| }} | ||
| /> | ||
| )} | ||
| </Box> | ||
| ); | ||
| }; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace fixed 200ms sleep with a deterministic fullscreen/state wait.
waitForTimeout(200)is flaky in CI and can race with React state updates. Wait for fullscreen exit/state observable condition instead.💡 Suggested fix
await page.evaluate(async () => { if (document.fullscreenElement) { await document.exitFullscreen(); } }); - await page.waitForTimeout(200); + await page.waitForFunction(() => document.fullscreenElement === null); + await expect( + overlay.getByRole('button', { name: 'Enter fullscreen' }), + ).toBeVisible(); await page.keyboard.press('Escape');📝 Committable suggestion
🤖 Prompt for AI Agents