Browse Source

fix(projects): portal-mounted hover preview for cover thumbnails (#1155)

  @smandon flagged the 40×40 cover thumbnail as too small to recognise
  the print and asked for a click-to-enlarge full preview. Enlarging
  the thumbnail itself would shift the card grid layout, so keep the
  small thumbnail and show a 384×384 hover popover with the full image
  in ``object-contain`` rendering (so tall MakerWorld photos aren't
  cropped to a square).

  Why a portal: ProjectCard carries ``overflow-hidden`` (rounded
  corners + color accent bar), so any in-tree popover gets clipped the
  moment it extends past the card. Rendering via
  ``createPortal(..., document.body)`` escapes every ancestor clipping
  context, and ``position: fixed`` with measurements from
  ``getBoundingClientRect()`` keeps the popover pinned next to the
  thumbnail regardless of grid position. ``pointer-events-none`` on the
  popover so it can't intercept hover and create a flicker loop;
  ``z-[100]`` so it stacks above sibling cards.

  Edge handling: if the thumbnail is near the viewport's right edge the
  popover flips to the LEFT side of the thumbnail; vertical position is
  clamped so the popover never overflows the window top or bottom. The
  thumbnail's own ``onClick`` is ``stopPropagation``'d so hovering the
  popover area never accidentally triggers the parent card's
  "open project" navigation.

  Tests: 2 new ``ProjectsPage.test.tsx`` cases — mouseenter mounts the
  popover at document.body level (not nested in the card subtree, which
  would re-introduce the clipping bug, and the assertion catches that);
  mouseleave unmounts it; the popover img points at the same
  cover-image URL as the small thumbnail with ``object-contain``; cards
  without a cover_image_filename never mount the portal-rendering
  component.
maziggy 3 weeks ago
parent
commit
82a593de95

File diff suppressed because it is too large
+ 0 - 0
CHANGELOG.md


+ 7 - 2
backend/app/api/routes/projects.py

@@ -667,10 +667,15 @@ async def list_project_archives(
     if not result.scalar_one_or_none():
         raise HTTPException(status_code=404, detail="Project not found")
 
-    # Get archives with project relationship eagerly loaded
+    # Get archives with both ``project`` and ``created_by`` eagerly loaded.
+    # ``archive_to_response`` accesses ``archive.created_by.username`` to
+    # surface the creator on the archive card; without selectinload that's
+    # a lazy attribute access on a closed async session, which throws
+    # ``MissingGreenlet`` and produces a 500. ``ArchiveService.list_archives``
+    # already loads both — this route just got out of step.
     query = (
         select(PrintArchive)
-        .options(selectinload(PrintArchive.project))
+        .options(selectinload(PrintArchive.project), selectinload(PrintArchive.created_by))
         .where(PrintArchive.project_id == project_id)
         .order_by(PrintArchive.created_at.desc())
         .limit(limit)

+ 67 - 0
backend/tests/integration/test_projects_api.py

@@ -560,6 +560,73 @@ class TestProjectArchivesAPI:
         data = response.json()
         assert "name" in data
 
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_list_archives_in_project_returns_archives_with_creator(
+        self, async_client: AsyncClient, project_factory, db_session
+    ):
+        """``GET /projects/{id}/archives`` must eagerly load both the project AND
+        the creator User. Without selectinload(created_by) the response
+        converter triggers a lazy attribute load on a closed async session
+        and the request 500s with MissingGreenlet — exactly what was reported
+        the moment a user with auth enabled (so archives carry created_by_id)
+        opened a project view.
+        """
+        from backend.app.models.archive import PrintArchive
+        from backend.app.models.user import User
+
+        # Seed: a user (the eventual creator) and a project owning two archives,
+        # one with created_by_id set, one without.
+        creator = User(
+            username="archive-creator",
+            password_hash="x",
+            role="user",
+            is_active=True,
+        )
+        db_session.add(creator)
+        await db_session.commit()
+        await db_session.refresh(creator)
+
+        project = await project_factory(name="Project Archives Smoke")
+
+        attributed = PrintArchive(
+            filename="attributed.3mf",
+            file_path="x/attributed.3mf",
+            file_size=2048,
+            print_name="Attributed Print",
+            status="completed",
+            quantity=1,
+            project_id=project.id,
+            created_by_id=creator.id,
+        )
+        anonymous = PrintArchive(
+            filename="anon.3mf",
+            file_path="x/anon.3mf",
+            file_size=2048,
+            print_name="Anonymous Print",
+            status="completed",
+            quantity=1,
+            project_id=project.id,
+            created_by_id=None,
+        )
+        db_session.add_all([attributed, anonymous])
+        await db_session.commit()
+
+        response = await async_client.get(f"/api/v1/projects/{project.id}/archives?limit=100&offset=0")
+        assert response.status_code == 200, f"Expected 200, got {response.status_code} body={response.text}"
+
+        rows = response.json()
+        assert len(rows) == 2
+
+        # Both archive shapes serialise — the attributed one surfaces the
+        # creator username (proving the eager-load worked) and the anonymous
+        # one stays None without exploding.
+        by_filename = {r["filename"]: r for r in rows}
+        assert by_filename["attributed.3mf"]["created_by_username"] == "archive-creator"
+        assert by_filename["attributed.3mf"]["created_by_id"] == creator.id
+        assert by_filename["anon.3mf"]["created_by_username"] is None
+        assert by_filename["anon.3mf"]["created_by_id"] is None
+
 
 class TestProjectExportImport:
     """Tests for project export/import functionality."""

+ 74 - 0
frontend/src/__tests__/pages/ProjectsPage.test.tsx

@@ -230,5 +230,79 @@ describe('ProjectsPage', () => {
       // Card thumbnail uses the GET endpoint URL, project.id is 1.
       expect(img.getAttribute('src')).toContain('/projects/1/cover-image');
     });
+
+    it('renders the portal-mounted hover preview only while the thumbnail is hovered (#1155)', async () => {
+      // The portal escapes the project card's ``overflow-hidden``, which
+      // would otherwise clip a 384×384 popover anchored to a 40×40
+      // thumbnail. Pin the contract end-to-end:
+      //   - no preview in the DOM by default (mouseenter not fired yet)
+      //   - mouseenter mounts a popover at document.body level (not
+      //     nested in the card subtree, which would re-introduce the
+      //     clipping bug)
+      //   - mouseleave unmounts it
+      //   - the popover ``<img>`` points at the same cover-image URL as
+      //     the small thumbnail (object-contain so portrait/landscape
+      //     MakerWorld photos aren't cropped)
+      const { fireEvent } = await import('@testing-library/react');
+
+      server.use(
+        http.get('/api/v1/projects/', () =>
+          HttpResponse.json([
+            {
+              ...mockProjects[0],
+              url: null,
+              cover_image_filename: 'cover_abc.png',
+            },
+          ])
+        )
+      );
+
+      render(<ProjectsPage />);
+
+      const thumb = await screen.findByAltText(/Project cover photo/i);
+      // Default: no popover yet.
+      expect(document.querySelectorAll('[aria-hidden="true"] img').length).toBe(0);
+
+      // Hover: walk up to the wrapper div the component attaches its
+      // mouseenter to. The portal mounts under document.body, NOT
+      // nested in the card subtree.
+      const wrapper = thumb.closest('[class*="flex-shrink-0"]') as HTMLElement;
+      expect(wrapper).not.toBeNull();
+      fireEvent.mouseEnter(wrapper);
+
+      const previewImg = document.querySelector('[aria-hidden="true"] img') as HTMLImageElement | null;
+      expect(previewImg).not.toBeNull();
+      expect(previewImg!.getAttribute('src')).toContain('/projects/1/cover-image');
+      expect(previewImg!.className).toContain('object-contain');
+
+      // The portal mounts on document.body (or one of its direct
+      // descendants), not inside the card — that's the whole point of
+      // the portal, so a future refactor that drops the portal would
+      // re-introduce the clipping regression.
+      const popover = previewImg!.closest('[aria-hidden="true"]') as HTMLElement;
+      expect(popover.closest('[class*="rounded-xl"]')).toBeNull();
+
+      // Unmount on leave.
+      fireEvent.mouseLeave(wrapper);
+      expect(document.querySelectorAll('[aria-hidden="true"] img').length).toBe(0);
+    });
+
+    it('does not render a hover preview when there is no cover image', async () => {
+      server.use(
+        http.get('/api/v1/projects/', () =>
+          HttpResponse.json([
+            { ...mockProjects[0], url: null, cover_image_filename: null },
+          ])
+        )
+      );
+
+      render(<ProjectsPage />);
+
+      await screen.findByText(mockProjects[0].name);
+      expect(screen.queryByAltText(/Project cover photo/i)).toBeNull();
+      // No aria-hidden img should ever appear because no thumbnail to
+      // hover means the portal-mounting component never renders.
+      expect(document.querySelectorAll('[aria-hidden="true"] img').length).toBe(0);
+    });
   });
 });

+ 97 - 9
frontend/src/pages/ProjectsPage.tsx

@@ -1,4 +1,5 @@
 import { useState, useRef } from 'react';
+import { createPortal } from 'react-dom';
 import { useTranslation } from 'react-i18next';
 import { useNavigate } from 'react-router-dom';
 import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query';
@@ -394,6 +395,91 @@ export function ProjectModal({ project, onClose, onSave, isLoading, currencySymb
   );
 }
 
+/**
+ * Cover thumbnail with portal-rendered hover preview (#1155 follow-up).
+ *
+ * Why a portal: the parent ``ProjectCard`` carries ``overflow-hidden`` for
+ * its rounded-corner clipping and color accent bar; an in-tree popover
+ * gets clipped by that and only the part that overlaps the card is
+ * visible. Rendering the preview via ``createPortal`` to ``document.body``
+ * escapes every ancestor clipping context, and ``position: fixed`` with
+ * ``getBoundingClientRect()`` keeps it pinned next to the thumbnail
+ * regardless of where the card sits in the grid.
+ */
+function ProjectCoverThumbnail({
+  projectId,
+  altText,
+}: {
+  projectId: number;
+  altText: string;
+}) {
+  const thumbRef = useRef<HTMLDivElement>(null);
+  const [hovered, setHovered] = useState(false);
+  const [pos, setPos] = useState<{ left: number; top: number } | null>(null);
+
+  const handleEnter = () => {
+    if (!thumbRef.current) return;
+    const rect = thumbRef.current.getBoundingClientRect();
+    // Anchor the 384px preview just to the right of the thumbnail (8px gap).
+    // Clamp ``top`` so the preview never overflows the viewport vertically;
+    // similar story for ``left`` if the card is near the right edge — flip
+    // to the LEFT side of the thumbnail in that case.
+    const PREVIEW = 384;
+    const GAP = 8;
+    const vw = window.innerWidth;
+    const vh = window.innerHeight;
+    let left = rect.right + GAP;
+    if (left + PREVIEW > vw - 8) {
+      left = rect.left - PREVIEW - GAP;
+    }
+    let top = rect.top;
+    if (top + PREVIEW > vh - 8) {
+      top = vh - PREVIEW - 8;
+    }
+    if (top < 8) top = 8;
+    setPos({ left, top });
+    setHovered(true);
+  };
+
+  const handleLeave = () => setHovered(false);
+
+  return (
+    <div
+      ref={thumbRef}
+      className="relative flex-shrink-0"
+      onMouseEnter={handleEnter}
+      onMouseLeave={handleLeave}
+      onClick={(e) => e.stopPropagation()}
+    >
+      <div className="w-10 h-10 rounded-lg overflow-hidden bg-bambu-dark border border-bambu-dark-tertiary">
+        <img
+          src={api.getProjectCoverImageUrl(projectId)}
+          alt={altText}
+          className="w-full h-full object-cover"
+          loading="lazy"
+        />
+      </div>
+      {hovered && pos &&
+        createPortal(
+          <div
+            className="fixed z-[100] w-96 h-96 rounded-lg overflow-hidden border border-bambu-dark-tertiary shadow-2xl bg-bambu-dark pointer-events-none"
+            style={{ left: pos.left, top: pos.top }}
+            aria-hidden="true"
+          >
+            <img
+              src={api.getProjectCoverImageUrl(projectId)}
+              alt=""
+              className="w-full h-full object-contain"
+              loading="lazy"
+            />
+          </div>,
+          document.body,
+        )}
+    </div>
+  );
+}
+
+
 interface ProjectCardProps {
   project: ProjectListItem;
   onClick: () => void;
@@ -444,15 +530,17 @@ function ProjectCard({ project, onClick, onEdit, onDelete, hasPermission, t }: P
         <div className="flex items-start justify-between mb-4">
           <div className="flex items-center gap-3 min-w-0 flex-1">
             {project.cover_image_filename ? (
-              // #1155: cover photo replaces the status-icon box
-              <div className="w-10 h-10 rounded-lg overflow-hidden flex-shrink-0 bg-bambu-dark border border-bambu-dark-tertiary">
-                <img
-                  src={api.getProjectCoverImageUrl(project.id)}
-                  alt={t('projects.coverImageAlt')}
-                  className="w-full h-full object-cover"
-                  loading="lazy"
-                />
-              </div>
+              // #1155: cover photo replaces the status-icon box. The thumbnail
+              // itself stays small so the card layout doesn't shift; on hover
+              // a portal-rendered 384×384 preview pops out beside the card
+              // so the user can identify the print without navigating into
+              // the project view. The portal is needed because ProjectCard's
+              // own ``overflow-hidden`` (for rounded corners) clips any
+              // in-tree popover before it can extend outside the card.
+              <ProjectCoverThumbnail
+                projectId={project.id}
+                altText={t('projects.coverImageAlt')}
+              />
             ) : (
               <div className={`p-2 rounded-lg ${statusConfig.bg} flex-shrink-0`}>
                 <statusConfig.icon className={`w-5 h-5 ${statusConfig.color}`} />

File diff suppressed because it is too large
+ 0 - 0
static/assets/index-7GmlJb0k.css


File diff suppressed because it is too large
+ 0 - 0
static/assets/index-BijNyfT8.js


File diff suppressed because it is too large
+ 0 - 0
static/assets/index-Cw7zekS6.css


+ 2 - 2
static/index.html

@@ -26,8 +26,8 @@
 
     <!-- Splash screens for iOS -->
     <link rel="apple-touch-startup-image" href="/img/android-chrome-512x512.png" />
-    <script type="module" crossorigin src="/assets/index-Ct29kBC5.js"></script>
-    <link rel="stylesheet" crossorigin href="/assets/index-7GmlJb0k.css">
+    <script type="module" crossorigin src="/assets/index-BijNyfT8.js"></script>
+    <link rel="stylesheet" crossorigin href="/assets/index-Cw7zekS6.css">
   </head>
   <body>
     <div id="root"></div>

Some files were not shown because too many files changed in this diff