Skip to content

Conversation

@luandro
Copy link
Contributor

@luandro luandro commented Sep 17, 2025

Summary

  • upgrade image zoom plugin config and dependencies to remove forced margins
  • add Root enhancer plus CSS to hide chrome, adjust transforms, and add close button during zoom
  • ensure docs images keep proportions while eliminating top gap on tall assets

Testing

  • bun run dev (manual zoom checks on tall and wide images)

@luandro luandro marked this pull request as draft September 25, 2025 12:19
@luandro luandro closed this Oct 31, 2025
@luandro luandro reopened this Oct 31, 2025
@luandro
Copy link
Contributor Author

luandro commented Oct 31, 2025

Code Review Summary

What This PR Does

This PR implements image zoom functionality with proper viewport sizing to eliminate the top gap issue:

  • Adds docusaurus-plugin-image-zoom plugin with custom configuration
  • Creates Root.tsx theme enhancement to coordinate zoom behavior
  • Implements navbar hiding, scroll lock, and close button during zoom
  • Removes layout constraints during zoom to prevent image distortion
  • Adds comprehensive CSS to control zoom behavior
  • Includes rehype plugin for figure/caption support

How It Solves the Issue

The implementation uses a multi-layered approach:

  1. Plugin Configuration: Configures the zoom plugin with zero margins and scroll offset
  2. Layout Cleanup: Removes navbar height and padding during zoom via CSS variables
  3. State Management: Uses body classes (zooming, zoom-open) to coordinate state
  4. UI Enhancements: Adds a close button and prevents scroll during zoom
  5. Transform Handling: Allows the plugin to handle sizing without interference

Potential Issues & Considerations

Strengths:

  • ✅ Comprehensive implementation addressing the full zoom experience
  • ✅ Navbar hiding before measurement to prevent layout issues
  • ✅ Scroll lock during zoom for better UX
  • ✅ Visual close button for accessibility
  • ✅ Extensive documentation in TASK.md explaining the approach
  • ✅ Adds rehype plugin for figure/caption support (nice bonus)

Areas to Watch:

  • ⚠️ Complexity: This is a large PR (539 lines) with multiple components - ensure all pieces work together correctly
  • ⚠️ Mobile/Responsive: Need to verify behavior on mobile devices, especially with address bar hiding/showing
  • ⚠️ TASK.md: The task file is included in the PR - should be removed before merge as it's development documentation
  • ⚠️ Browser Compatibility: Uses :has() selector which requires modern browsers (Chrome 105+, Safari 15.4+, Firefox 121+)
  • ⚠️ Performance: Multiple event listeners and DOM observers - ensure no memory leaks
  • ⚠️ Plugin Version: Uses docusaurus-plugin-image-zoom v3.0.1 - verify compatibility with current Docusaurus version
  • ⚠️ Touch Devices: Need to verify tap-to-close and touch interactions work correctly

Code Quality:

  • Well-structured with clear separation of concerns
  • Good use of TypeScript
  • Proper React patterns (hooks, effects, cleanup)
  • Extensive CSS with good organization
  • Clear comments and documentation

Testing:

  • Manual testing recommended for:
    • Various image aspect ratios (tall, wide, square)
    • Mobile vs desktop behavior
    • Close button functionality
    • Overlay click-to-close
    • Double-click to close
    • Keyboard interactions (ESC key)

Recommendation: ⚠️ Approve with Caveats - Solid implementation that solves the zoom gap issue. Before merging:

  1. Remove TASK.md file (development documentation)
  2. Add :has() polyfill or fallback for older browsers if needed
  3. Perform thorough manual testing on various devices
  4. Consider adding automated tests for the zoom behavior

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.

2 participants