[web] Improve the file viewer options in the embed app

[mnvr]

No description provided.

[mnvr]

@claude review

[claude]

Claude finished @mnvr's task —— View job

---

Code Review

• Read repository guidelines
• Examine changed files
• Analyze git diff
• Provide review feedback

---

Summary

This PR improves the file viewer UI in the embed app by making the fullscreen button more prominent and refining the more menu visibility logic. The changes are well-structured and follow good practices.

Detailed Feedback

✅ Strengths

1. Well-scoped changes - The PR focuses on a specific UI improvement without introducing unnecessary complexity.

2. Good separation of concerns - New props are properly threaded through the component hierarchy:

   • showFullscreenButton added to FileViewer and FileViewerPhotoSwipeOptions interfaces
   • Icon support cleanly extended in icons.tsx

3. Conditional visibility logic - The more menu hiding logic is thoughtful:

   const shouldShowMoreButton = !showFullscreenButton || haveUser;

   This prevents an empty/minimal more menu in public album contexts.

4. Icon implementation - The dual-state fullscreen icon (fullscreen/fullscreen-exit) follows the existing pattern used for the favorite button.

🔍 Observations & Suggestions

1. Fullscreen state management (web/packages/gallery/components/viewer/FileViewer.tsx:366)

The fullscreen state is tracked but could be more robust:

const [isFullscreen, setIsFullscreen] = useState(false);

Consideration: The fullscreen state updates rely on updateFullscreenStatus being called on fullscreenchange events and initialization. If the user enters/exits fullscreen via browser shortcuts (F11, Esc), the icon state should still update correctly since you're listening to the fullscreenchange event. ✅

2. Button ordering (web/packages/gallery/components/viewer/photoswipe.ts:1258-1301)

The fullscreen button is registered with order: 18, placing it after the more button (order: 16). The logic comment states:

// Add fullscreen button as a primary action for embed app

Question: Is this the intended visual order? Typically "primary actions" appear earlier in the sequence. Consider if order: 14 or 15 might be more appropriate to position it before the info button (order: 13) or between info and more.

3. More menu content (web/packages/gallery/components/viewer/photoswipe.ts:1236)

When showFullscreenButton=true and haveUser=false, the more menu is hidden entirely. However, the more menu contains:

• Download (if showDownload == "menu")
• Fullscreen toggle
• Shortcuts

Potential issue: In public albums with downloads disabled (disableDownload=true) and showFullscreenButton=true, users lose access to the "Shortcuts" help dialog since the more menu is hidden. Consider whether shortcuts should always remain accessible, perhaps as a separate button or keyboard-only feature.

4. Embed app integration (web/apps/embed/src/components/EmbedFileListWithViewer.tsx:132)

Clean integration:

showFullscreenButton={true}

Suggestion: Since this is always true in the embed app, consider documenting why in a comment, especially if there are plans to make this configurable in the future.

5. Icon visual alignment (web/packages/gallery/components/viewer/icons.tsx:40-45)

The fullscreen icons use the same transform as other icons:

'<path d="..." transform="translate(0, 4.5)"',

Note: This is consistent with existing icons. If the fullscreen icon appears misaligned with neighbors, the transform may need adjustment, but this should be verified visually.

🐛 Potential Issues

Minor: Console logs left in embed component

web/apps/embed/src/components/EmbedFileListWithViewer.tsx:45-46, 54-55

console.log("Thumbnail clicked, index:", index);
console.log("Files available:", files.length);
// ...
console.log("Closing file viewer");
// ...
console.log("Visual feedback requested");

Recommendation: Remove these console.log statements before merging, or convert them to use the project's logging infrastructure if they're needed for debugging.

📋 Testing Checklist

Please verify:

• Fullscreen button appears in embed app viewer
• Fullscreen button toggles between enter/exit states correctly
• Fullscreen works via button click
• Fullscreen state updates correctly when using F key shortcut
• Fullscreen state updates correctly when using browser F11/Esc
• More menu is hidden in embed app when expected
• More menu still shows in photos app contexts
• Download button visibility unchanged in public albums (when not using fullscreen button)
• Mobile/touch device behavior is correct

🎯 Verdict

Approval recommended with one minor cleanup (remove console.logs). The changes are solid and improve the UX for embed app users. The potential issue with shortcuts access is worth discussing but doesn't block this PR.

---

Job run | Branch: fs