Skip to content

mod(v2): preview — restore pdf.js, add comments tab, breathe spacing#149

Merged
zjean merged 1 commit intomainfrom
mod/v2-preview-pdfjs-comments-spacing
May 4, 2026
Merged

mod(v2): preview — restore pdf.js, add comments tab, breathe spacing#149
zjean merged 1 commit intomainfrom
mod/v2-preview-pdfjs-comments-spacing

Conversation

@zjean
Copy link
Copy Markdown
Owner

@zjean zjean commented May 4, 2026

Three small fixes to the v2 preview overlay/standalone, all in `custom-v2/preview/`. Single PR per request.

1. PDFs now render in pdf.js by default

`ONLY_OFFICE_EXTENSIONS` (`only-office.constants.ts:83`) includes `pdf` because OnlyOffice can edit PDFs. That made `isOffice()` true for any `.pdf` file, which short-circuited `showOfficeEmbed()` ahead of the `isPdf()` branch in the template:

```html
@if (showOfficeEmbed() && file(); as f) { <app-v2-preview-office-view ... }
@else if (isPdf() && pdfSafeUrl(); as url) { <iframe ... [src]="url"> }
```

The pdf.js iframe URL was already being built correctly by `pdfSafeUrl()` — it just never rendered.

Fix: `isOffice()` now excludes PDFs explicitly. The toolbar's pencil button (`canToggleToOffice` + `pdfStage`) remains the explicit opt-in to switch into the OnlyOffice editor.

2. Comments tab

The info aside used to show only file metadata. Now it has Info / Comments tabs and mounts the existing `` — same component the file-detail screen and dock-panel use, same `/api/comments/spaces/` endpoints as classic. Folders skip the Comments tab entirely (no empty state needed in the preview UI).

`infoTab` resets to 'info' on every navigation so a previous file's tab choice doesn't leak.

3. Spacing pass

Bumped to match the rest of v2's chrome:

before after
Header height 32 44
Header font 12 13
Toolbar height 40 52
Info aside width 280 320
Info kv padding 14 18
Info kv row pad 5 7
Info kv font 12 / 11.5 13 / 12.5

Plus a proper tab strip with active border-bottom + hover + focus-visible.

Test plan

  • `npx ng lint` clean
  • `npx ng build --configuration development` clean
  • Manual smoke: open a `.pdf` from the v2 file list — should land in pdf.js with the toolbar / page nav (not OnlyOffice). Pencil icon switches to OnlyOffice.
  • Manual smoke: open the info aside on a non-folder file — Comments tab should appear, posting/editing/deleting should work.
  • Visual check on the spacing.

🤖 Generated with Claude Code

… spacing

Three small fixes to the v2 preview overlay/standalone, all in
custom-v2/preview/:

1. PDFs now render in pdf.js by default. ONLY_OFFICE_EXTENSIONS includes
   `pdf` (because OnlyOffice can edit PDFs), which made `isOffice()` true
   for any `.pdf` and short-circuited `showOfficeEmbed()` ahead of the
   `isPdf()` branch in the template. The pdf.js iframe URL was already
   built correctly by `pdfSafeUrl()` — it just never rendered. Now
   `isOffice()` excludes PDFs explicitly. The toolbar's pencil button
   (canToggleToOffice + pdfStage) remains the explicit opt-in to switch
   into OnlyOffice for editing.

2. Comments tab in the info aside. Mounts the existing
   <app-v2-comments-panel> (file-detail screen + dock-panel use the same
   component) — same /api/comments/spaces/<path> endpoints as classic.
   Tab strip switches between Info and Comments; Comments tab is hidden
   for folders. infoTab resets to 'info' on every navigation so a
   previous file's tab choice doesn't leak in.

3. Spacing pass to match the v2 chrome elsewhere:
   - header 32 → 44 px, padding 10 → 14, font 12 → 13
   - toolbar 40 → 52 px, padding 10 → 14
   - info aside 280 → 320 px wide, kv padding 14 → 18, dt/dd 5 → 7,
     fonts 12/11.5 → 13/12.5
   - new __tabs strip styled like the rest of v2 (border-bottom under
     active tab, hover bg, focus-visible outline)
   - __comments-host wrapper makes the comments panel scroll inside the
     aside without pushing the tab strip out of view

No backend changes. Frontend lint + dev build clean.
@zjean zjean enabled auto-merge (squash) May 4, 2026 16:44
@zjean zjean merged commit 2092e40 into main May 4, 2026
1 check passed
@zjean zjean deleted the mod/v2-preview-pdfjs-comments-spacing branch May 4, 2026 16:46
zjean added a commit that referenced this pull request May 4, 2026
…detail Prev/Next history stacking (#150)

## Summary

- **Fix #1 — comments pill now opens the preview overlay** (personal +
space-files screens): `openComments` was routing to the heavier
`/v2/file` full-screen route. Since the preview overlay gained its own
Comments tab in #149, the pill now calls
`PreviewOverlayService.open(path, file, { initialTab: 'comments' })`.
`PreviewOverlayService` gets a new `PreviewOpenOptions` param + a plain
`initialTab` getter; `PreviewComponent` reads that getter on each path
change and auto-opens the info aside when `comments` is requested.

- **Fix #2 — Prev/Next in file-detail no longer stacks history
entries**: `goTo()` was missing `replaceUrl: true`, so each sibling
navigation pushed a new browser history entry. `close()` only walked
back one entry, forcing the user to click Close once per Prev/Next
pressed. Adding `replaceUrl: true` makes Prev/Next mutate the URL in
place; Close exits in a single click.

## Files changed

| File | Change |
|---|---|
| `preview-overlay.service.ts` | Add `PreviewOpenOptions` interface,
`initialTab` getter, extend `open()` signature |
| `preview.component.ts` | Apply `overlay.initialTab` on each path
change; auto-open info aside for comments |
| `personal.component.ts` | `openComments` → overlay with `initialTab:
'comments'` |
| `space-files.component.ts` | Same |
| `file-detail.component.ts` | `goTo()` + `replaceUrl: true` |

## Test plan

- [ ] Click the comments pill on a file row in `/v2/personal` → preview
overlay opens with Comments tab active and info aside expanded
- [ ] Click anywhere else on the same row → preview overlay opens with
Info tab (existing behavior preserved)
- [ ] Same two checks in `/v2/spaces/<alias>`
- [ ] In `/v2/file?path=…` (direct deep-link), click Next/Prev several
times → URL mutates, Close exits in a single click
- [ ] `ng lint` and `tsc --noEmit` pass (verified locally)

🤖 Generated with [Claude Code](https://claude.ai/claude-code)

Co-authored-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant