Repository navigation
fix(photo): scroll PDF pages instead of changing photo #4898
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
Merged
+272
−12
Merged
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
44 changes: 44 additions & 0 deletions
44
docs/specs/4-architecture/features/099-pdf-photo-view/open-questions.md
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| # Open Questions – Feature 099 | ||
|
|
||
| Open questions for [Feature 099](spec.md). Log every high- and medium-impact question here (table row + Question Details entry) before asking the user; see [open-questions-format.md](../../spec-guidelines/open-questions-format.md). Once answered, fold the outcome into [spec.md](spec.md) (and an ADR when architecturally significant), then mark the entry resolved. | ||
|
|
||
| ## Active Questions | ||
|
|
||
| | Question ID | Feature | Priority | Summary | Status | Opened | Updated | | ||
| |-------------|---------|----------|---------|--------|--------|---------| | ||
| | ~~Q-099-01~~ | 099 | Medium | Wheel over a PDF: A) never navigate photos while a PDF is shown, B) navigate only when the wheel is outside the PDF frame, C) navigate only once the PDF hits its first/last page | Resolved (A, 2026-10-17, FR-099-01) | 2026-10-17 | 2026-10-17 | | ||
| | ~~Q-099-02~~ | 099 | Medium | Which dock actions move to the header for PDFs: A) every dock action that applies to a PDF (highlight, set album header, move, delete), B) move and delete only, C) move and delete in the header, the rest in an overflow menu | Resolved (B, 2026-10-17, FR-099-03) | 2026-10-17 | 2026-10-17 | | ||
| | ~~Q-099-03~~ | 099 | Low | Frontend scope: A) v8 only, B) v8 and v7 | Resolved (A, 2026-10-17, Non-Goals) | 2026-10-17 | 2026-10-17 | | ||
|
|
||
| ## Question Details | ||
|
|
||
| ### ~~Q-099-01~~ · Wheel behaviour while a PDF is displayed | ||
|
|
||
| **Question:** `PhotoPanel.vue` listens to `wheel` on `window` and turns any wheel event into next/previous when "scroll to navigate" is on, so scrolling a PDF changes photo instead of turning pages. What should the wheel do while a PDF is shown? | ||
|
|
||
| **Answer:** A (2026-10-17), folded into FR-099-01. | ||
|
|
||
| - **A (recommended) – Never navigate while a PDF is shown.** Same rule as the 360° sphere (FR-082-10). The wheel only scrolls the PDF; arrows, keyboard and swipe still navigate. Simple, predictable, one guard in `scrollTo`. | ||
| - **B – Navigate only when the wheel is outside the PDF frame** (header strip, margins). Keeps wheel navigation, but the strip is thin, and browsers differ in which wheel events leave the embedded viewer, so it is hard to predict and to test. | ||
| - **C – Navigate once the PDF reaches its first/last page.** Natural "scroll through", but the embedded browser viewer exposes no page/scroll position to the page, so it cannot be implemented with `<embed>`. | ||
|
|
||
| ### ~~Q-099-02~~ · Header actions replacing the dock for PDFs | ||
|
|
||
| **Question:** For PDFs, the dock is hidden and its actions move to the header. Which actions? | ||
|
|
||
| Dock actions today: highlight, set album header, watermark, scan faces, rotate CCW/CW, move, delete. Watermark, scan faces and rotate do not apply to a PDF. | ||
|
|
||
| **Answer:** B (2026-10-17), folded into FR-099-03 and Non-Goals. | ||
|
|
||
| - **A (recommended) – Every action that applies to a PDF:** highlight, set album header (not in the timeline), move (when `can_move_content`), delete. No capability lost; same rights checks as the dock. | ||
| - **B – Move and delete only.** Smallest header, but PDF owners can no longer highlight a PDF or set it as album header from the photo view. | ||
| - **C – Move and delete as buttons, highlight and set album header in a `…` overflow menu.** Compact, but adds a new menu pattern to the photo header. | ||
|
|
||
| ### ~~Q-099-03~~ · v7 frontend | ||
|
|
||
| **Question:** The same wheel listener and dock exist in `resources/js/v7/components/gallery/photoModule/`. Should v7 change too? | ||
|
|
||
| **Answer:** A (2026-10-17), folded into Overview and Non-Goals. | ||
|
|
||
| - **A (recommended) – v8 only.** v8 is the current frontend; v7 stays as is. | ||
| - **B – v8 and v7.** Same behaviour in both, but twice the change and the testing. |
50 changes: 50 additions & 0 deletions
50
docs/specs/4-architecture/features/099-pdf-photo-view/plan.md
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| # Feature Plan 099 – PDF Photo View | ||
|
|
||
| | Field | Value | | ||
| |-------|-------| | ||
| | Status | Complete | | ||
| | Last updated | 2026-10-17 | | ||
| | Linked spec | [spec.md](spec.md) | | ||
| | Linked tasks | [tasks.md](tasks.md) | | ||
|
|
||
| ## Vision & Success Criteria | ||
| Wheel scrolls a displayed PDF instead of changing photo; the dock no longer covers the PDF and its move/delete actions live in the photo header (S-099-01 … 05). | ||
|
|
||
| ## Scope Alignment | ||
| In: v8 `PhotoPanel.vue`, `PhotoHeader.vue`, `Dock.vue`. Out: v7, backend, highlight / set album header for PDFs (Q-099-02 B). | ||
|
|
||
| ## Dependencies & Interfaces | ||
| `photoStore.imageViewMode` (`ImageViewMode.Pdf`), `albumStore.rights.can_move_content`, the `toggleMove` / `toggleDelete` events already handled by the photo view parent of `PhotoPanel`. | ||
|
|
||
| ## Assumptions & Risks | ||
| - Keyboard shortcuts for move/delete are unaffected (they do not go through the dock). | ||
| - Risk: header crowding on phones; two icon-only buttons, same style as the existing ones. | ||
|
|
||
| ## Implementation Drift Gate | ||
| Re-read FR-099-01 … 03 against the diff before commit. | ||
|
|
||
| ## Increment Map | ||
| 1. I1 – Wheel guard for PDFs in `PhotoPanel.scrollTo` (FR-099-01). | ||
| 2. I2 – Hide the dock for PDFs, drop the now-unused `isNarrowMenu` prop (FR-099-02). | ||
| 3. I3 – Move/delete buttons in `PhotoHeader` for PDFs, wired through `PhotoPanel` (FR-099-03). | ||
| 4. I4 – Escape closes the photo edit modal (FR-099-04). | ||
|
|
||
| ## Scenario Tracking | ||
| | Scenario | Increment | | ||
| |----------|-----------| | ||
| | S-099-01, S-099-02 | I1 | | ||
| | S-099-03, S-099-03b, S-099-04 | I2, I3 | | ||
| | S-099-05 | I1–I3 (regression) | | ||
|
|
||
| ## Analysis Gate | ||
| 2026-10-17: spec, plan and tasks agree; Q-099-01 … 03 resolved; no ADR needed (local UI change). | ||
|
|
||
| ## Exit Criteria | ||
| `vp check --fix`, `vp run typecheck` green; tasks checked; roadmap updated. | ||
|
|
||
| ## Follow-ups / Backlog | ||
| None. | ||
|
|
||
| ## Intent Log | ||
| - 2026-10-17: Owner reported the wheel changing photo over PDFs and asked to move dock actions to the header. Questions Q-099-01 … 03 asked in chat, answered "A B A". Owner asked to commit and push once done. | ||
| - 2026-10-17: Owner reported Escape not closing the photo edit modal and navigating up. Reproduced the modal not closing (`:dismissible="false"`, from the Nuxt UI migration #4487); the upward navigation did not reproduce on the scratch instance. Fixed by dropping the flag, verified with the run-lychee driver. |
113 changes: 113 additions & 0 deletions
113
docs/specs/4-architecture/features/099-pdf-photo-view/spec.md
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,113 @@ | ||
| # Feature 099 – PDF Photo View | ||
|
|
||
| | Field | Value | | ||
| |-------|-------| | ||
| | Status | Ready | | ||
| | Last updated | 2026-10-17 | | ||
| | Owners | ildyria | | ||
| | Linked plan | [plan.md](plan.md) | | ||
| | Linked tasks | [tasks.md](tasks.md) | | ||
| | Roadmap entry | #099 | | ||
|
|
||
| > Guardrail: This specification is the single normative source of truth for the feature. Track high- and medium-impact questions in the feature's [open-questions.md](open-questions.md), encode resolved answers directly in the Requirements/NFR/Behaviour/UI/Telemetry sections below (no per-feature `## Clarifications` sections), and use ADRs under `docs/specs/6-decisions/` for architecturally significant clarifications (referencing their IDs from the relevant spec sections). | ||
|
|
||
| ## Overview | ||
| When a PDF is shown in the photo view (`ImageViewMode.Pdf`), the mouse wheel changes photo instead of scrolling the PDF's pages: `PhotoPanel.vue` listens to `wheel` on `window` and maps every event to next/previous when "scroll to navigate" is enabled. The dock also sits on top of the PDF and hides part of its first page. This feature makes the wheel scroll the PDF and moves the dock actions into the photo header for PDFs. It also makes Escape close the photo edit modal, which `:dismissible="false"` blocked (owner report 2026-10-17). Frontend only: v8 (Q-099-03 A); v7 is unchanged. No backend or API change. | ||
|
|
||
| ## Goals | ||
| - Scrolling over a displayed PDF scrolls its pages and never changes photo (Q-099-01 A). | ||
| - The dock is not shown for PDFs; move and delete are in the photo header instead (Q-099-02 B). | ||
| - Escape closes the photo edit modal and leaves the photo open. | ||
|
|
||
| ## Non-Goals | ||
| - Replacing the browser's embedded PDF viewer (pdf.js or similar). | ||
| - Changing wheel, dock or header behaviour for images, videos, RAW placeholders or 360° photos. | ||
| - The v7 frontend (Q-099-03 A). | ||
| - Highlight and set-album-header from the PDF photo view: both dock actions are dropped for PDFs (Q-099-02 B). The header's existing highlight button for users without album edit rights is unchanged. | ||
|
|
||
| ## Functional Requirements | ||
|
|
||
| | ID | Requirement | Success path | Validation path | Failure path | Telemetry & traces | Source | | ||
| |----|-------------|--------------|-----------------|--------------|--------------------|--------| | ||
| | FR-099-01 | The wheel never navigates photos while a PDF is displayed, like the 360° sphere (FR-082-10) (Q-099-01 A). | Wheel scrolls the PDF; arrows, keyboard and swipe still navigate. | Applies only when `photoStore.imageViewMode === ImageViewMode.Pdf`. | n/a | none | Owner report 2026-10-17 | | ||
| | FR-099-02 | The dock is hidden while a PDF is displayed. | No dock over the PDF. | Applies only to `ImageViewMode.Pdf`. | n/a | none | Owner directive 2026-10-17 | | ||
| | FR-099-03 | The photo header shows move (when `albumStore.rights.can_move_content`) and delete for a PDF, with the dock's rights checks (Q-099-02 B). | Buttons emit `toggleMove` / `toggleDelete`, the same events as the dock buttons. | Shown only when the dock would have been shown (`canEditCurrentPhoto`, not editing, not in a slideshow). | n/a | none | Owner directive 2026-10-17 | | ||
| | FR-099-04 | Escape closes the photo edit modal (`PhotoEdit.vue`), like the album edit modal. | First Escape closes the modal, photo stays open; a second Escape leaves the photo. | Applies with focus anywhere in the modal. | n/a | none | Owner report 2026-10-17 | | ||
|
|
||
| ## Non-Functional Requirements | ||
|
|
||
| | ID | Requirement | Driver | Measurement | Dependencies | Source | | ||
| |----|-------------|--------|-------------|--------------|--------| | ||
| | NFR-099-01 | No regression for non-PDF media. | Scope guard | Manual check: image, video, 360° photo keep their wheel and dock behaviour. | `PhotoPanel.vue`, `PhotoHeader.vue` | AGENTS.md | | ||
| | NFR-099-02 | Frontend gates pass. | Quality | `vp check --fix`, `vp run typecheck` | Vite+ (Feature 096) | AGENTS.md | | ||
|
|
||
| ## UI / Interaction Mock-ups | ||
|
|
||
| Before (PDF): | ||
| ``` | ||
| +----------------------------------------------------------------+ | ||
| | [<] [app] [dl] [edit] [info] ... | | ||
| | +----------------------------+ | | ||
| | | [flag][img][folder][trash] | <- dock | | ||
| | +----------------------------------------------------------+ | | ||
| | | PDF page 1 (hidden under the dock) | | | ||
| | | | | | ||
| | | wheel -> next / previous photo (bug) | | | ||
| | +----------------------------------------------------------+ | | ||
| +----------------------------------------------------------------+ | ||
| ``` | ||
|
|
||
| After (PDF): | ||
| ``` | ||
| +----------------------------------------------------------------+ | ||
| | [<] [app] [dl] [edit] [info] [folder] [trash] | | ||
| | +----------------------------------------------------------+ | | ||
| | | PDF page 1 | | | ||
| | | | | | ||
| | | wheel -> scrolls the PDF pages | | | ||
| | +----------------------------------------------------------+ | | ||
| +----------------------------------------------------------------+ | ||
| ``` | ||
|
|
||
| ## Branch & Scenario Matrix | ||
|
|
||
| | Scenario ID | Description / Expected outcome | | ||
| |-------------|--------------------------------| | ||
| | S-099-01 | PDF shown, scroll-to-navigate on, wheel over the PDF: pages scroll, photo stays. | | ||
| | S-099-02 | PDF shown, arrow key / next button: navigates as before. | | ||
| | S-099-03 | PDF shown, user can edit and move: no dock; header shows move and delete. | | ||
| | S-099-03b | PDF shown, user can edit but not move: header shows delete only. | | ||
| | S-099-04 | PDF shown, user cannot edit: no dock, no extra header actions. | | ||
| | S-099-05 | Image shown: wheel navigation and dock unchanged. | | ||
| | S-099-06 | Photo edit modal open, Escape: modal closes, photo stays; second Escape returns to the album. | | ||
|
|
||
| ## Test Strategy | ||
| - **Frontend (v8):** no frontend unit test harness covers `PhotoPanel`/`PhotoHeader`; verified with `vp run typecheck`, `vp check --fix` and a manual run of S-099-01 … 05 on a scratch instance (run-lychee skill). | ||
| - **S-099-06:** checked 2026-10-17 on the run-lychee scratch instance (focus on body, title, description; reopen with `e`; second Escape). | ||
| - **Backend:** none. | ||
|
|
||
| ## Interface & Contract Catalogue | ||
|
|
||
| ### UI States | ||
| | ID | State | Trigger / Expected outcome | | ||
| |----|-------|---------------------------| | ||
| | UI-099-01 | PDF view, editor | Dock hidden, move and delete in header. | | ||
| | UI-099-02 | PDF view, viewer | Dock hidden, header unchanged. | | ||
|
|
||
| ## Telemetry & Observability | ||
| None. | ||
|
|
||
| ## Documentation Deliverables | ||
| Roadmap row 099. | ||
|
|
||
| ## Fixtures & Sample Data | ||
| None. | ||
|
|
||
| ## Spec DSL | ||
| ``` | ||
| ui_states: | ||
| - id: UI-099-01 | ||
| description: PDF view, editor — dock hidden, move and delete in header | ||
| - id: UI-099-02 | ||
| description: PDF view, viewer — dock hidden, header unchanged | ||
| ``` | ||
19 changes: 19 additions & 0 deletions
19
docs/specs/4-architecture/features/099-pdf-photo-view/tasks.md
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| # Feature 099 Tasks – PDF Photo View | ||
|
|
||
| _Status: Complete_ | ||
| _Last updated: 2026-10-17_ | ||
|
|
||
| ## Checklist | ||
| - [x] T-099-01 – Skip wheel navigation while a PDF is shown (FR-099-01, S-099-01, S-099-02, S-099-05). | ||
| _Verification commands:_ `vp run typecheck`, `vp check --fix` | ||
|
|
||
| - [x] T-099-02 – Hide the dock for PDFs, remove `Dock.isNarrowMenu` (FR-099-02, S-099-03, S-099-04). | ||
| _Verification commands:_ `vp run typecheck`, `vp check --fix` | ||
|
|
||
| - [x] T-099-03 – Move and delete buttons in `PhotoHeader` for PDFs (FR-099-03, S-099-03, S-099-03b, S-099-04). | ||
| _Verification commands:_ `vp run typecheck`, `vp check --fix` | ||
| - [x] T-099-04 – Escape closes the photo edit modal: drop `:dismissible="false"` from `PhotoEdit.vue` (FR-099-04, S-099-06). | ||
| _Verification commands:_ `vp check` on the file, driver run on the scratch instance | ||
|
|
||
| ## Notes / TODOs | ||
| No frontend unit harness for these components. Verified 2026-10-17 with `vp check` and `vp run typecheck` on the three changed files. Manual browser check of S-099-01 … 05 left to the owner. Run `vp check --fix` on the changed files only: on the whole tree it rewrites unrelated files and breaks `EmbedCodeDialog.vue` (`<\/script>` unescaped). |
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
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
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
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
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.