From 8456b8f9764edc16ec19d6f89efc776a701209f2 Mon Sep 17 00:00:00 2001 From: cnrd Date: Tue, 25 Aug 2026 15:51:04 +0200 Subject: [PATCH] fix(web): stop the profile card re-anchoring to its own avatar (#39) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Avatar opened the profile popout whenever it received a user prop. Since user is how every avatar gets its gradient, colour and status dot, all 22 call sites became profile triggers by accident — including the picture inside the profile card itself, which re-anchored the card to that picture on every click and walked it across the screen (120px right, 36px down, until it pinned at the viewport clamp). Avatar is now presentational. A new ProfileAvatar carries the open-the-profile behaviour at the five call sites that actually want it. The card's own picture escalates to the full profile modal instead of reopening the card. The card also places itself off its measured size via the shared computeFloatingPosition engine, replacing six call sites that each hand-computed coordinates against a guessed 460px card height. Closes #37 --- docs/systems/design-system.md | 33 +++++ .../web/src/components/chat/MentionBadge.tsx | 6 +- packages/web/src/components/chat/Message.tsx | 9 +- .../web/src/components/chat/MessageList.tsx | 9 +- .../src/components/layout/ActivityPanel.tsx | 7 +- .../web/src/components/layout/AppLayout.tsx | 5 +- .../src/components/layout/ChannelSidebar.tsx | 4 +- .../components/layout/DmMemberRow.test.tsx | 36 +---- .../web/src/components/layout/DmMemberRow.tsx | 14 +- .../src/components/layout/DmRosterPanel.tsx | 3 +- .../src/components/layout/MemberSidebar.tsx | 6 +- .../src/components/modals/GroupDmSettings.tsx | 3 +- .../web/src/components/ui/Avatar.test.tsx | 63 +++++++++ packages/web/src/components/ui/Avatar.tsx | 19 +-- .../src/components/ui/ProfileAvatar.test.tsx | 92 +++++++++++++ .../web/src/components/ui/ProfileAvatar.tsx | 40 ++++++ .../components/ui/UserProfilePopout.test.tsx | 129 ++++++++++++++++++ .../src/components/ui/UserProfilePopout.tsx | 70 ++++++++-- .../web/src/components/voice/VoiceUser.tsx | 4 +- .../web/src/hooks/useFloatingPosition.test.ts | 44 ++++++ packages/web/src/hooks/useFloatingPosition.ts | 50 +++++-- packages/web/src/stores/uiStore.ts | 18 ++- packages/web/src/test/setup.ts | 17 +++ 23 files changed, 563 insertions(+), 118 deletions(-) create mode 100644 packages/web/src/components/ui/Avatar.test.tsx create mode 100644 packages/web/src/components/ui/ProfileAvatar.test.tsx create mode 100644 packages/web/src/components/ui/ProfileAvatar.tsx create mode 100644 packages/web/src/components/ui/UserProfilePopout.test.tsx create mode 100644 packages/web/src/hooks/useFloatingPosition.test.ts diff --git a/docs/systems/design-system.md b/docs/systems/design-system.md index ba0268ac..d2eef15b 100644 --- a/docs/systems/design-system.md +++ b/docs/systems/design-system.md @@ -199,6 +199,39 @@ interface AvatarStackProps { **Hooks-in-loop safety:** each rendered slot is its own `` component so `useCanonicalUserView` is called exactly once per slot, never inside a variable-length `.map()`. +### Avatar vs ProfileAvatar + +Two components, one deliberate split: + +| Component | Role | +|---|---| +| `Avatar` (`ui/Avatar.tsx`) | Purely presentational. Takes `user` for the gradient, avatar colour, `homeUserId` and status dot. Clicking it does nothing unless the caller passes `onClick`. | +| `ProfileAvatar` (`ui/ProfileAvatar.tsx`) | `Avatar` plus the profile card. Opens `UserProfilePopout` anchored to its own box, stops propagation so it wins over an enclosing row handler, and stays inert while `user` is undefined. | + +**Rule:** an avatar is only a profile trigger when it is a `ProfileAvatar`. Never re-add an implicit "open the profile if a `user` prop is present" branch to `Avatar` — passing `user` is how *every* avatar gets its colour, so that branch silently turns the picture inside the profile card, the settings preview, the avatar-upload button and every row in a modal into a trigger. It also made the card re-anchor to its own picture and walk across the screen on repeated clicks (issue #37). + +Use `ProfileAvatar` when the avatar is the primary way to reach that person's profile and nothing else owns the click. Use `Avatar` when an enclosing row, button or list item already handles clicks, or when the avatar depicts the surface it already sits on. + +**Escalation chain.** Clicking a face always moves one step deeper, never sideways and never nowhere: + +| Surface | Picture click | +|---|---| +| Member tile / row / message author | Opens the preview card (`UserProfilePopout`) | +| Preview card | Opens the full profile modal (`UserProfileModal`) and closes the card | +| Full profile modal | Nothing — this is the terminus | + +The middle step matters: an inert picture on the preview card is a dead end that forces the user down to the *View Full Profile* link. What it must never do is reopen the card itself — that is the drift bug from issue #37. + +### Floating placement + +Every floating surface places itself with `computeFloatingPosition` (`hooks/useFloatingPosition.ts`): preferred side → flip when it would overflow → clamp into the viewport, with an 8px viewport padding. + +- Components with a live anchor element use the `useFloatingPosition` hook (tooltips, mention/search popovers, voice popovers). +- Components opened from a store keep the anchor's **rect** instead of an element — `uiStore.openUserProfile(user, anchor, placement)` stores `AnchorRect` + `Placement`, and `UserProfilePopout` measures itself and places off that. `pointAnchor(x, y)` builds a zero-size rect for the rare caller with no anchor element. +- `align: 'start'` lines the surface's leading edge up with the anchor; the default centres it on the anchor. + +**Callers never compute coordinates.** A surface that is handed a finished `{ top, left }` cannot account for its own measured size, and any caller-side constant (an assumed card height, a hardcoded sidebar width) drifts the moment the content or the layout changes. + **Tile geometry contract.** Each `AvatarTile` renders at `size × size` with a 2px border (`box-sizing: border-box` from Tailwind preflight), so its content area is `(size − 4) × (size − 4)`. The inner `Avatar` is sized to that content area (`size − 2 · TILE_BORDER_WIDTH`) and centered geometrically on the tile via `flex items-center justify-center`, **not** by inline-flow placement. Both corrections are required: sizing the Avatar to the outer dimensions overflows the padding box and gets clipped off-center (visible disc remains centered, but the avatar's contents — image crop, initials gradient + letter — anchor at the padding-edge top-left and visibly drift toward the lower-right of the visible disc); relying on `Avatar`'s `inline-flex` placement makes the Avatar drift vertically by whatever the inherited `line-height` adds, independent of border. `TILE_BORDER_WIDTH` is exported from `AvatarStack.tsx` as the single source of truth for the `border-2` width and must be updated in lockstep with any future change to that class. **Border tiers:** the surface tier the stack sits on determines the tile border color (so the tiles cleanly separate from the panel they overlap). `channel` → `border-surface-channel` (sidebar); `chat` → `border-surface-chat` (chat area / welcome header / chat header); `modal` → `border-surface-elevated` (modal hero, mobile info-screen hero — there is no `surface-modal` token in `tailwind.config.js`). diff --git a/packages/web/src/components/chat/MentionBadge.tsx b/packages/web/src/components/chat/MentionBadge.tsx index 91895cc1..70ec259f 100644 --- a/packages/web/src/components/chat/MentionBadge.tsx +++ b/packages/web/src/components/chat/MentionBadge.tsx @@ -43,11 +43,7 @@ export const MentionBadge = React.memo(function MentionBadge({ userId }: Mention const handleClick = (e: React.MouseEvent) => { if (!member || !memberUser) return; e.stopPropagation(); - const rect = e.currentTarget.getBoundingClientRect(); - openUserProfile(memberUser, { - top: Math.min(rect.top, window.innerHeight - 450), - left: rect.right + 8, - }); + openUserProfile(memberUser, e.currentTarget.getBoundingClientRect()); }; // Build inline styles: role-colored text with tinted background diff --git a/packages/web/src/components/chat/Message.tsx b/packages/web/src/components/chat/Message.tsx index 1ba136bf..2b0c6e40 100644 --- a/packages/web/src/components/chat/Message.tsx +++ b/packages/web/src/components/chat/Message.tsx @@ -4,6 +4,7 @@ import type { MessageWithUser, Embed, User } from '@backspace/shared'; import { MarkdownRenderer } from './MarkdownRenderer'; import { MentionBadge } from './MentionBadge'; import { Avatar } from '../ui/Avatar'; +import { ProfileAvatar } from '../ui/ProfileAvatar'; import { useContextMenuStore } from '../../stores/contextMenuStore'; import { buildMessageMenuItems } from './messageMenuItems'; import { useAuthStore } from '../../stores/authStore'; @@ -266,11 +267,7 @@ export function Message({ message, isCompact, isFirstInGroup, previousMessageId const handleUsernameClick = (e: React.MouseEvent) => { if (!message.user) return; e.stopPropagation(); - const rect = e.currentTarget.getBoundingClientRect(); - openUserProfile(message.user, { - top: Math.min(rect.top, window.innerHeight - 450), - left: rect.right + 16, - }); + openUserProfile(message.user, e.currentTarget.getBoundingClientRect()); }; const handleContextMenu = (e: React.MouseEvent) => { @@ -417,7 +414,7 @@ export function Message({ message, isCompact, isFirstInGroup, previousMessageId
{isFirstInGroup || message.replyTo ? (
- ) => { if (!ownerMember) return; - const rect = e.currentTarget.getBoundingClientRect(); - openUserProfile(ownerMember, { - top: Math.min(rect.bottom + 8, window.innerHeight - 450), - left: rect.left, - }); + openUserProfile(ownerMember, e.currentTarget.getBoundingClientRect(), 'bottom'); }; return ( @@ -896,7 +893,7 @@ function WelcomeHeader({ channelId }: { channelId: string }) { return (
- +

{displayName}

diff --git a/packages/web/src/components/layout/ActivityPanel.tsx b/packages/web/src/components/layout/ActivityPanel.tsx index f2c67d48..11e09a86 100644 --- a/packages/web/src/components/layout/ActivityPanel.tsx +++ b/packages/web/src/components/layout/ActivityPanel.tsx @@ -104,7 +104,6 @@ export function ActivityPanel() { const handleFriendClick = (e: React.MouseEvent, friend: Friend) => { e.stopPropagation(); - const rect = e.currentTarget.getBoundingClientRect(); openUserProfile( { id: friend.id, @@ -123,10 +122,8 @@ export function ActivityPanel() { isAdmin: false, replicatedInstances: [], }, - { - top: Math.min(rect.top, window.innerHeight - 450), - left: rect.left - 316, - } + e.currentTarget.getBoundingClientRect(), + 'left', ); }; diff --git a/packages/web/src/components/layout/AppLayout.tsx b/packages/web/src/components/layout/AppLayout.tsx index 00fbc089..d3e02ff5 100644 --- a/packages/web/src/components/layout/AppLayout.tsx +++ b/packages/web/src/components/layout/AppLayout.tsx @@ -463,7 +463,7 @@ export function AppLayout() { {/* User Profile Popout */} - {userProfilePopout.user && userProfilePopout.position && ( + {userProfilePopout.user && userProfilePopout.anchor && ( <>

)} diff --git a/packages/web/src/components/layout/ChannelSidebar.tsx b/packages/web/src/components/layout/ChannelSidebar.tsx index c878d189..3145da31 100644 --- a/packages/web/src/components/layout/ChannelSidebar.tsx +++ b/packages/web/src/components/layout/ChannelSidebar.tsx @@ -9,7 +9,7 @@ import { useInstanceStore } from '../../stores/instanceStore'; import { VoiceChannel } from '../voice/VoiceChannel'; import { VoiceControls } from '../voice/VoiceControls'; import { useVoiceStore } from '../../stores/voiceStore'; -import { Avatar } from '../ui/Avatar'; +import { ProfileAvatar } from '../ui/ProfileAvatar'; import { Mascot } from '../ui/Mascot'; import { wsSend } from '../../hooks/useWebSocket'; import { AudioManager } from '../../audio/AudioManager'; @@ -1135,7 +1135,7 @@ function UserAreaPanel({
{/* Avatar + name */}
- +
{user.displayName ?? user.username}
@{user.username}
diff --git a/packages/web/src/components/layout/DmMemberRow.test.tsx b/packages/web/src/components/layout/DmMemberRow.test.tsx index 9ab90a94..69113647 100644 --- a/packages/web/src/components/layout/DmMemberRow.test.tsx +++ b/packages/web/src/components/layout/DmMemberRow.test.tsx @@ -285,45 +285,17 @@ describe('DmMemberRow — profile popout anchoring', () => { await user.click(profileBtn); expect(openUserProfileMock).toHaveBeenCalledTimes(1); + // The row hands over its rect and the side it wants; the card works out its + // own coordinates once it knows how tall it is (see UserProfilePopout). expect(openUserProfileMock).toHaveBeenCalledWith( expect.objectContaining({ id: member.id }), - // Math.min(200, 800 - 450) = 200; left = 1500 - 316 = 1184. - { top: 200, left: 1184 }, + rect, + 'left', ); // The row no longer routes 'profile' through onMenuAction. expect(onMenuAction).not.toHaveBeenCalled(); }); - it('clamps top to (innerHeight - 450) when the row sits near the bottom of the viewport', async () => { - const user = userEvent.setup(); - const { container } = renderRow(); - const row = container.querySelector('[data-dm-member-row]') as HTMLElement; - - const rect: DOMRect = { - top: 700, - left: 1500, - right: 1740, - bottom: 740, - width: 240, - height: 40, - x: 1500, - y: 700, - toJSON: () => ({}), - } as DOMRect; - row.getBoundingClientRect = () => rect; - - Object.defineProperty(window, 'innerHeight', { value: 800, configurable: true }); - - openMenuByContextMenu(row); - await user.click(await screen.findByText('View Profile')); - - // Math.min(700, 800 - 450 = 350) → top clamped to 350. - expect(openUserProfileMock).toHaveBeenCalledWith( - expect.anything(), - { top: 350, left: 1184 }, - ); - }); - it('falls back to onMenuAction("profile", ...) when the row has no bounding rect', async () => { const user = userEvent.setup(); const { container, onMenuAction, member } = renderRow(); diff --git a/packages/web/src/components/layout/DmMemberRow.tsx b/packages/web/src/components/layout/DmMemberRow.tsx index a20b6335..6af53d7b 100644 --- a/packages/web/src/components/layout/DmMemberRow.tsx +++ b/packages/web/src/components/layout/DmMemberRow.tsx @@ -1,6 +1,6 @@ import React, { useRef } from 'react'; import type { User } from '@backspace/shared'; -import { Avatar } from '../ui/Avatar'; +import { ProfileAvatar } from '../ui/ProfileAvatar'; import { Username } from '../ui/Username'; import { Tooltip } from '../ui/Tooltip'; import { parseFederatedUsername, isFederationGlobeApplicable } from '../../utils/identity'; @@ -104,14 +104,11 @@ export function DmMemberRow({ label: 'View Profile', onClick: () => { // Anchor the popout to this row's bounding rect — matches the - // MemberSidebar pattern (see MemberSidebar.tsx:158-165). On mobile - // the position arg is ignored by the store (full-screen push). + // MemberSidebar pattern (see MemberSidebar.tsx). On mobile the anchor + // is ignored by the store (full-screen push). const rect = rowRef.current?.getBoundingClientRect(); if (rect) { - useUIStore.getState().openUserProfile(canonical, { - top: Math.min(rect.top, window.innerHeight - 450), - left: rect.left - 316, - }); + useUIStore.getState().openUserProfile(canonical, rect, 'left'); } else { // Fallback: defer to the consumer if we can't compute a rect // (shouldn't happen in practice, but keeps the contract intact). @@ -183,12 +180,13 @@ export function DmMemberRow({ className="group flex items-center gap-2.5 px-2 py-1.5 rounded-[6px] hover:bg-interactive-hover transition-colors select-none" >
-
diff --git a/packages/web/src/components/layout/DmRosterPanel.tsx b/packages/web/src/components/layout/DmRosterPanel.tsx index 93ddf0da..be7ceecb 100644 --- a/packages/web/src/components/layout/DmRosterPanel.tsx +++ b/packages/web/src/components/layout/DmRosterPanel.tsx @@ -9,6 +9,7 @@ import { isSelf, parseFederatedUsername } from '../../utils/identity'; import { api } from '../../api/client'; import { ConfirmDialog } from '../ui/ConfirmDialog'; import { DmMemberRow, type DmMemberRowAction } from './DmMemberRow'; +import { pointAnchor } from '../../hooks/useFloatingPosition'; /** * Right-side roster for group DMs. Mirrors `MemberSidebar`'s layout language @@ -93,7 +94,7 @@ export function DmRosterPanel() { // MemberSidebar pattern). This branch only fires on the unlikely // fallback path where the row couldn't compute its bounding rect — // in that case, anchor to the top-left of the roster column. - openUserProfile(member, { top: 100, left: 100 }); + openUserProfile(member, pointAnchor(100, 100)); return; } if (action === 'kick') { diff --git a/packages/web/src/components/layout/MemberSidebar.tsx b/packages/web/src/components/layout/MemberSidebar.tsx index 869a0c5f..4c84db43 100644 --- a/packages/web/src/components/layout/MemberSidebar.tsx +++ b/packages/web/src/components/layout/MemberSidebar.tsx @@ -157,11 +157,7 @@ export function MemberSidebar() { const handleMemberClick = (e: React.MouseEvent, user: MemberWithUser['user']) => { e.stopPropagation(); - const rect = e.currentTarget.getBoundingClientRect(); - openUserProfile(user, { - top: Math.min(rect.top, window.innerHeight - 450), - left: rect.left - 316, - }); + openUserProfile(user, e.currentTarget.getBoundingClientRect(), 'left'); }; const renderMember = (member: MemberWithUser, isOffline = false) => { diff --git a/packages/web/src/components/modals/GroupDmSettings.tsx b/packages/web/src/components/modals/GroupDmSettings.tsx index 0da81688..5e37c7f3 100644 --- a/packages/web/src/components/modals/GroupDmSettings.tsx +++ b/packages/web/src/components/modals/GroupDmSettings.tsx @@ -13,6 +13,7 @@ import { api } from '../../api/client'; import { isSelf, parseFederatedUsername } from '../../utils/identity'; import { AvatarStack } from '../ui/AvatarStack'; import { DmMemberRow, type DmMemberRowAction } from '../layout/DmMemberRow'; +import { pointAnchor } from '../../hooks/useFloatingPosition'; const MAX_NAME_LENGTH = 50; const MAX_GROUP_MEMBERS = 10; @@ -263,7 +264,7 @@ export function GroupDmSettings() { // Fallback path — DmMemberRow normally opens the profile itself via // its own bounding rect. If we reach this branch, just route to a // top-left anchor (matches DmRosterPanel's fallback). - useUIStore.getState().openUserProfile(member, { top: 100, left: 100 }); + useUIStore.getState().openUserProfile(member, pointAnchor(100, 100)); return; } if (action === 'kick') { diff --git a/packages/web/src/components/ui/Avatar.test.tsx b/packages/web/src/components/ui/Avatar.test.tsx new file mode 100644 index 00000000..00ca64b2 --- /dev/null +++ b/packages/web/src/components/ui/Avatar.test.tsx @@ -0,0 +1,63 @@ +import { describe, it, expect, beforeEach } from 'vitest'; +import { render } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import type { User } from '@backspace/shared'; + +import { Avatar } from './Avatar'; +import { useUIStore } from '../../stores/uiStore'; + +function makeUser(): User { + return { + id: 'u-1', + username: 'ada', + displayName: 'Ada', + avatar: null, + banner: null, + accentColor: null, + avatarColor: null, + bio: null, + status: 'online', + customStatus: null, + isAdmin: false, + createdAt: 0, + homeInstance: null, + homeUserId: null, + replicatedInstances: [], + }; +} + +describe('Avatar', () => { + beforeEach(() => { + useUIStore.setState({ + isMobile: false, + userProfilePopout: { user: null, anchor: null, placement: 'right' }, + }); + }); + + it('is presentational: a `user` prop alone does not make it a profile trigger', async () => { + // `user` carries identity for the gradient, colour and status dot. Passing it + // must not silently turn the avatar into a popout trigger — otherwise every + // avatar inside a modal, settings preview or the profile card itself opens a + // second profile card on top of the surface it lives in (issue #37). + const { container } = render(); + + await userEvent.click(container.querySelector('[data-avatar]')!); + + expect(useUIStore.getState().userProfilePopout.user).toBeNull(); + }); + + it('is not focusable or clickable-looking without a handler', () => { + const { container } = render(); + + expect(container.querySelector('[data-avatar]')!.className).not.toContain('cursor-pointer'); + }); + + it('runs an explicit onClick handler', async () => { + let clicks = 0; + const { container } = render( { clicks++; }} />); + + await userEvent.click(container.querySelector('[data-avatar]')!); + + expect(clicks).toBe(1); + }); +}); diff --git a/packages/web/src/components/ui/Avatar.tsx b/packages/web/src/components/ui/Avatar.tsx index 8c1f8560..56976863 100644 --- a/packages/web/src/components/ui/Avatar.tsx +++ b/packages/web/src/components/ui/Avatar.tsx @@ -1,6 +1,5 @@ import React from 'react'; import type { User } from '@backspace/shared'; -import { useUIStore } from '../../stores/uiStore'; import { getAvatarGradient } from '../../utils/gradients'; interface AvatarProps { @@ -56,7 +55,6 @@ function getDotMetrics(avatarSize: number, ringWidth: number = 0) { } export function Avatar({ src, name, size = 40, status, className = '', onClick, user, userId, ring, avatarColor }: AvatarProps) { - const openUserProfile = useUIStore((s) => s.openUserProfile); const initials = name.charAt(0).toUpperCase(); const fontPx = Math.round(size * 0.4); const gradient = getAvatarGradient(userId ?? user?.homeUserId ?? user?.id, name, avatarColor ?? user?.avatarColor); @@ -64,19 +62,6 @@ export function Avatar({ src, name, size = 40, status, className = '', onClick, const ringWidth = ring?.width ?? 0; const outerSize = size + ringWidth * 2; - const handleClick = (e: React.MouseEvent) => { - if (onClick) { - onClick(e); - } else if (user) { - e.stopPropagation(); - const rect = e.currentTarget.getBoundingClientRect(); - openUserProfile(user, { - top: Math.min(rect.top, window.innerHeight - 450), - left: rect.right + 16, - }); - } - }; - // Only compute mask when status dot is visible const cutoutMask = status ? buildCutoutMask(size, ringWidth) : undefined; const maskStyle: React.CSSProperties | undefined = cutoutMask @@ -88,9 +73,9 @@ export function Avatar({ src, name, size = 40, status, className = '', onClick, return (
{/* Inner masked circle — ring background + avatar content */}
) { + vi.spyOn(el, 'getBoundingClientRect').mockReturnValue({ + top: 0, left: 0, right: 0, bottom: 0, width: 0, height: 0, x: 0, y: 0, + toJSON: () => ({}), ...rect, + } as DOMRect); +} + +describe('ProfileAvatar', () => { + beforeEach(() => { + useUIStore.setState({ + isMobile: false, + userProfilePopout: { user: null, anchor: null, placement: 'right' }, + }); + }); + + it('opens the profile popout anchored to its own box', async () => { + const { container } = render(); + const el = container.querySelector('[data-avatar]')!; + stubRect(el, { top: 200, left: 100, right: 140, bottom: 240, width: 40, height: 40 }); + + await userEvent.click(el); + + const popout = useUIStore.getState().userProfilePopout; + expect(popout.user).toMatchObject({ id: 'u-1' }); + expect(popout.anchor).toMatchObject({ top: 200, left: 100, right: 140, bottom: 240 }); + expect(popout.placement).toBe('right'); + }); + + it('honours an explicit placement so callers do not hand-roll offsets', async () => { + const { container } = render(); + const el = container.querySelector('[data-avatar]')!; + stubRect(el, { top: 10, left: 900, right: 940, bottom: 50, width: 40, height: 40 }); + + await userEvent.click(el); + + expect(useUIStore.getState().userProfilePopout.placement).toBe('left'); + }); + + it('degrades to a plain avatar when the user behind it is unknown', async () => { + // Voice tiles and DM intros render before the user record has resolved. + const { container } = render(); + const el = container.querySelector('[data-avatar]')!; + + await userEvent.click(el); + + expect(useUIStore.getState().userProfilePopout.user).toBeNull(); + expect(el.className).not.toContain('cursor-pointer'); + }); + + it('stops the click from reaching an enclosing row handler', async () => { + let rowClicks = 0; + const { container } = render( +
{ rowClicks++; }}> + +
, + ); + const el = container.querySelector('[data-avatar]')!; + stubRect(el, { top: 0, left: 0, right: 40, bottom: 40, width: 40, height: 40 }); + + await userEvent.click(el); + + expect(rowClicks).toBe(0); + }); +}); diff --git a/packages/web/src/components/ui/ProfileAvatar.tsx b/packages/web/src/components/ui/ProfileAvatar.tsx new file mode 100644 index 00000000..df16d5fe --- /dev/null +++ b/packages/web/src/components/ui/ProfileAvatar.tsx @@ -0,0 +1,40 @@ +import React from 'react'; +import type { User } from '@backspace/shared'; +import { Avatar } from './Avatar'; +import { useUIStore } from '../../stores/uiStore'; +import type { Placement } from '../../hooks/useFloatingPosition'; + +type AvatarProps = React.ComponentProps; + +interface ProfileAvatarProps extends Omit { + /** Undefined while the user record is still resolving — the avatar then stays + * presentational rather than offering a click that opens nothing. */ + user?: User; + /** Preferred side for the card; it flips automatically when there's no room. */ + placement?: Placement; +} + +/** + * An avatar that opens the profile card for the user it depicts. + * + * This is deliberately a separate component from `Avatar`: `Avatar` takes a + * `user` for the gradient, colour and status dot, and plenty of avatars carry + * one without being a profile trigger — the picture inside the profile card + * itself, the settings preview, rows inside modals. Folding the behaviour into + * `Avatar` made every one of those a trigger by accident, which is what let the + * profile card re-anchor to its own picture and walk across the screen. + */ +export function ProfileAvatar({ user, placement = 'right', ...avatarProps }: ProfileAvatarProps) { + const openUserProfile = useUIStore((s) => s.openUserProfile); + + const handleClick = user + ? (e: React.MouseEvent) => { + // Rows that hold an avatar usually have their own click target (open the + // DM, select the member). Opening the profile is the more specific intent. + e.stopPropagation(); + openUserProfile(user, e.currentTarget.getBoundingClientRect(), placement); + } + : undefined; + + return ; +} diff --git a/packages/web/src/components/ui/UserProfilePopout.test.tsx b/packages/web/src/components/ui/UserProfilePopout.test.tsx new file mode 100644 index 00000000..65af8f67 --- /dev/null +++ b/packages/web/src/components/ui/UserProfilePopout.test.tsx @@ -0,0 +1,129 @@ +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { render } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { MemoryRouter } from 'react-router-dom'; +import type { User } from '@backspace/shared'; + +// The popout reaches into the space store (origin routing), the API client and +// the federated-mutuals loader. None of that is under test here — stub it so the +// test exercises the card's own click and placement behaviour. +vi.mock('../../stores/spaceStore', () => ({ + useSpaceStore: Object.assign( + (selector: (s: Record) => unknown) => + selector({ addDmChannel: vi.fn(), findExistingDmForUser: vi.fn() }), + { getState: () => ({ addDmChannel: vi.fn(), findExistingDmForUser: vi.fn() }) }, + ), + getApiForOrigin: () => ({ uploads: { url: (k: string) => `/uploads/${k}` } }), + resolveUserOrigin: () => 'local', +})); +vi.mock('../../api/client', () => ({ api: { dm: { create: vi.fn() } } })); +vi.mock('../../utils/mutuals', () => ({ + loadFederatedMutuals: vi.fn().mockResolvedValue({ mutualFriends: [], mutualSpaces: [] }), +})); +vi.mock('../../utils/userViewLookup', () => ({ useCanonicalUserView: (u: User) => u })); + +import { UserProfilePopout } from './UserProfilePopout'; +import { useUIStore } from '../../stores/uiStore'; + +const CARD_W = 340; +const CARD_H = 420; + +function makeUser(): User { + return { + id: 'u-1', username: 'ada', displayName: 'Ada', avatar: null, banner: null, + accentColor: null, avatarColor: null, bio: null, status: 'online', + customStatus: null, isAdmin: false, createdAt: 0, homeInstance: null, + homeUserId: null, replicatedInstances: [], + }; +} + +function anchorAt(left: number, top: number, size = 40) { + return { top, left, right: left + size, bottom: top + size, width: size, height: size }; +} + +function setViewport(width: number, height: number) { + Object.defineProperty(window, 'innerWidth', { value: width, configurable: true }); + Object.defineProperty(window, 'innerHeight', { value: height, configurable: true }); +} + +function renderCard(anchor: ReturnType, placement?: 'left' | 'right') { + return render( + + {}} anchor={anchor} placement={placement} /> + , + ); +} + +describe('UserProfilePopout', () => { + beforeEach(() => { + setViewport(1920, 1080); + useUIStore.setState({ + isMobile: false, + activeModal: null, + modalData: {}, + userProfilePopout: { user: null, anchor: null, placement: 'right' }, + }); + // jsdom has no layout: give every element the card's real measured size so + // the popout can place itself off its own dimensions. + vi.spyOn(Element.prototype, 'getBoundingClientRect').mockReturnValue({ + top: 0, left: 0, right: CARD_W, bottom: CARD_H, width: CARD_W, height: CARD_H, + x: 0, y: 0, toJSON: () => ({}), + } as DOMRect); + }); + + it('does not reopen itself when its own picture is clicked (issue #37)', async () => { + const { container } = renderCard(anchorAt(300, 200)); + + const avatar = container.querySelector('[data-avatar]')!; + await userEvent.click(avatar); + await userEvent.click(avatar); + + expect(useUIStore.getState().userProfilePopout.user).toBeNull(); + }); + + it("escalates to the full profile when the card's picture is clicked", async () => { + // The picture is the obvious thing to click for "show me more about this + // person". Doing nothing there is a dead end — the only way forward would be + // the View Full Profile link. + let closed = false; + const { container } = render( + + { closed = true; }} anchor={anchorAt(300, 200)} /> + , + ); + + await userEvent.click(container.querySelector('[data-avatar]')!); + + expect(useUIStore.getState().activeModal).toBe('userProfile'); + expect(useUIStore.getState().modalData).toMatchObject({ userId: 'u-1' }); + expect(closed).toBe(true); + }); + + it('sits beside its anchor', () => { + const { container } = renderCard(anchorAt(300, 200)); + + const card = container.querySelector('[data-user-profile-popout]') as HTMLElement; + expect(parseFloat(card.style.left)).toBe(340 + 8); // anchor.right + offset + expect(parseFloat(card.style.top)).toBe(200); // top-aligned with its anchor + }); + + it('flips to the other side instead of running off the right edge', () => { + setViewport(1000, 800); + const { container } = renderCard(anchorAt(900, 100)); + + const card = container.querySelector('[data-user-profile-popout]') as HTMLElement; + const left = parseFloat(card.style.left); + expect(left).toBeGreaterThanOrEqual(8); + expect(left + CARD_W).toBeLessThanOrEqual(1000 - 8); + }); + + it('keeps a tall card on screen when anchored near the bottom', () => { + setViewport(1280, 700); + const { container } = renderCard(anchorAt(200, 660)); + + const card = container.querySelector('[data-user-profile-popout]') as HTMLElement; + const top = parseFloat(card.style.top); + expect(top).toBeGreaterThanOrEqual(8); + expect(top + CARD_H).toBeLessThanOrEqual(700 - 8); + }); +}); diff --git a/packages/web/src/components/ui/UserProfilePopout.tsx b/packages/web/src/components/ui/UserProfilePopout.tsx index c8fd2175..c071e7bb 100644 --- a/packages/web/src/components/ui/UserProfilePopout.tsx +++ b/packages/web/src/components/ui/UserProfilePopout.tsx @@ -1,4 +1,4 @@ -import React, { useEffect, useState } from 'react'; +import React, { useEffect, useLayoutEffect, useRef, useState } from 'react'; import { useNavigate } from 'react-router-dom'; import ReactMarkdown from 'react-markdown'; import type { User } from '@backspace/shared'; @@ -11,14 +11,20 @@ import { getAvatarGradient, adjustColor, mutedGradient } from '../../utils/gradi import { parseFederatedUsername } from '../../utils/identity'; import { useCanonicalUserView } from '../../utils/userViewLookup'; import { loadFederatedMutuals } from '../../utils/mutuals'; +import { computeFloatingPosition, type AnchorRect, type Placement } from '../../hooks/useFloatingPosition'; + +/** Gap between the card and the element it was opened from. */ +const ANCHOR_OFFSET = 8; interface UserProfilePopoutProps { user: User; onClose: () => void; - position?: { top: number; left: number }; + /** Rect of the element the card was opened from. */ + anchor: AnchorRect; + placement?: Placement; } -export function UserProfilePopout({ user: propUser, onClose, position }: UserProfilePopoutProps) { +export function UserProfilePopout({ user: propUser, onClose, anchor, placement = 'right' }: UserProfilePopoutProps) { const navigate = useNavigate(); const addDmChannel = useSpaceStore((s) => s.addDmChannel); const openModal = useUIStore((s) => s.openModal); @@ -43,12 +49,38 @@ export function UserProfilePopout({ user: propUser, onClose, position }: UserPro .catch(() => {}); }, [user.id, user.homeUserId]); - const top = position - ? Math.min(Math.max(8, position.top), window.innerHeight - 460) - : undefined; - const left = position - ? Math.min(Math.max(8, position.left), window.innerWidth - 356) - : undefined; + // Placed off the card's *measured* size rather than a guessed height: the card + // grows with the bio, the custom status and the mutuals row, so any constant + // here would cut tall cards off at the bottom of the viewport. + const cardRef = useRef(null); + const [placed, setPlaced] = useState<{ top: number; left: number } | null>(null); + + useLayoutEffect(() => { + const card = cardRef.current; + if (!card) return; + + const place = () => { + const { width, height } = card.getBoundingClientRect(); + // 'start': the card's top edge lines up with the row it came from, the + // way it always has — centring a tall card on a 32px avatar would drag it + // up over unrelated content. + const next = computeFloatingPosition(anchor, width, height, placement, ANCHOR_OFFSET, 'start'); + setPlaced((prev) => + prev && prev.top === next.top && prev.left === next.left + ? prev + : { top: next.top, left: next.left }, + ); + }; + + place(); + const observer = new ResizeObserver(place); + observer.observe(card); + window.addEventListener('resize', place); + return () => { + observer.disconnect(); + window.removeEventListener('resize', place); + }; + }, [anchor, placement]); const handleSendMessage = async () => { try { @@ -78,6 +110,11 @@ export function UserProfilePopout({ user: propUser, onClose, position }: UserPro openModal('userProfile', { userId: user.id, user, origin }); }; + const handleAvatarClick = (event: React.MouseEvent) => { + event.stopPropagation(); + handleViewFullProfile(); + }; + // Banner display const bannerSrc = user.banner ? (user.banner.startsWith('http') || user.banner.startsWith('/') ? user.banner : userApi.uploads.url(user.banner)) @@ -89,12 +126,17 @@ export function UserProfilePopout({ user: propUser, onClose, position }: UserPro return mutedGradient(g.from, g.to); })(); + // Parked off-screen for the one layout pass before the card knows how tall it + // is; `useLayoutEffect` places it before the browser paints, so it never + // renders visibly in the wrong spot. + const cardStyle = placed ?? { top: -9999, left: -9999 }; + return (
{/* Banner */}
{/* Avatar */} + {/* The picture escalates to the full profile — the card is a preview, and + clicking the face is the obvious way to ask for the whole thing. It + deliberately does NOT reopen the card (see issue #37). */} diff --git a/packages/web/src/components/voice/VoiceUser.tsx b/packages/web/src/components/voice/VoiceUser.tsx index 0f41aa05..ce65591f 100644 --- a/packages/web/src/components/voice/VoiceUser.tsx +++ b/packages/web/src/components/voice/VoiceUser.tsx @@ -1,5 +1,5 @@ import React, { useRef, useEffect, useState, useCallback } from 'react'; -import { Avatar } from '../ui/Avatar'; +import { ProfileAvatar } from '../ui/ProfileAvatar'; import { useVoiceStore } from '../../stores/voiceStore'; import { useContextMenuStore, type ContextMenuItem } from '../../stores/contextMenuStore'; import { buildVoiceModMenuItems, VolumeSliderItem } from './voiceMenuItems'; @@ -160,7 +160,7 @@ export function VoiceUser({ tile, large }: VoiceUserProps) { ) : (
- ({}) } as DOMRect; +} + +describe('computeFloatingPosition', () => { + beforeEach(() => { + Object.defineProperty(window, 'innerWidth', { value: 1000, configurable: true }); + Object.defineProperty(window, 'innerHeight', { value: 800, configurable: true }); + }); + + it('places a right-anchored surface just past the anchor', () => { + expect(computeFloatingPosition(rect(100, 100), 340, 420, 'right', 8)).toMatchObject({ + left: 148, + actualPlacement: 'right', + }); + }); + + it('flips left when the surface would overflow the right edge', () => { + const pos = computeFloatingPosition(rect(900, 100), 340, 420, 'right', 8); + expect(pos.actualPlacement).toBe('left'); + expect(pos.left).toBe(900 - 340 - 8); + }); + + it("aligns to the anchor's leading edge when asked, instead of centring on it", () => { + // The profile card lines its top edge up with the row it was opened from; + // centring a 420px card on a 32px avatar would drag it far up the screen. + const pos = computeFloatingPosition(rect(100, 300), 340, 420, 'right', 8, 'start'); + expect(pos.top).toBe(300); + }); + + it('centres on the anchor by default', () => { + const pos = computeFloatingPosition(rect(100, 300), 340, 420, 'right', 8); + expect(pos.top).toBe(300 + 20 - 210); + }); + + it('clamps a surface taller than the space below its anchor', () => { + const pos = computeFloatingPosition(rect(100, 700), 340, 420, 'right', 8); + expect(pos.top + 420).toBeLessThanOrEqual(800 - 8); + expect(pos.top).toBeGreaterThanOrEqual(8); + }); +}); diff --git a/packages/web/src/hooks/useFloatingPosition.ts b/packages/web/src/hooks/useFloatingPosition.ts index 31d0f277..719435ed 100644 --- a/packages/web/src/hooks/useFloatingPosition.ts +++ b/packages/web/src/hooks/useFloatingPosition.ts @@ -1,6 +1,23 @@ import { type RefObject, type CSSProperties, useState, useLayoutEffect, useCallback } from 'react'; -type Placement = 'top' | 'bottom' | 'left' | 'right'; +export type Placement = 'top' | 'bottom' | 'left' | 'right'; +/** Cross-axis alignment: centred on the anchor, or flush with its leading edge. */ +export type Alignment = 'center' | 'start'; + +/** The subset of DOMRect placement needs — lets callers pass a stored rect. */ +export interface AnchorRect { + top: number; + left: number; + right: number; + bottom: number; + width: number; + height: number; +} + +/** A zero-size anchor at a viewport point, for callers with no anchor element. */ +export function pointAnchor(x: number, y: number): AnchorRect { + return { top: y, left: x, right: x, bottom: y, width: 0, height: 0 }; +} interface UseFloatingPositionOptions { placement: Placement; @@ -22,12 +39,22 @@ const oppositePlacement: Record = { right: 'left', }; -function computePosition( - anchorRect: DOMRect, +/** + * Places a floating surface next to an anchor: preferred side first, flipping to + * the opposite side when it would overflow, then clamping into the viewport. + * + * Exported because not every floating surface has a live anchor element. The + * profile popout, for instance, is opened from a store and keeps only the + * anchor's rect — the row it came from may have re-rendered or scrolled away by + * the time the card mounts. + */ +export function computeFloatingPosition( + anchorRect: AnchorRect, floatingWidth: number, floatingHeight: number, placement: Placement, offset: number, + align: Alignment = 'center', ): { top: number; left: number; actualPlacement: Placement } { const vw = window.innerWidth; const vh = window.innerHeight; @@ -37,17 +64,24 @@ function computePosition( let actual = placement; // Compute initial position on primary axis + const crossLeft = align === 'start' + ? anchorRect.left + : anchorRect.left + anchorRect.width / 2 - floatingWidth / 2; + const crossTop = align === 'start' + ? anchorRect.top + : anchorRect.top + anchorRect.height / 2 - floatingHeight / 2; + if (placement === 'top') { top = anchorRect.top - floatingHeight - offset; - left = anchorRect.left + anchorRect.width / 2 - floatingWidth / 2; + left = crossLeft; } else if (placement === 'bottom') { top = anchorRect.bottom + offset; - left = anchorRect.left + anchorRect.width / 2 - floatingWidth / 2; + left = crossLeft; } else if (placement === 'left') { - top = anchorRect.top + anchorRect.height / 2 - floatingHeight / 2; + top = crossTop; left = anchorRect.left - floatingWidth - offset; } else { - top = anchorRect.top + anchorRect.height / 2 - floatingHeight / 2; + top = crossTop; left = anchorRect.right + offset; } @@ -113,7 +147,7 @@ export function useFloatingPosition( const anchorRect = anchor.getBoundingClientRect(); const floatingRect = floating.getBoundingClientRect(); - const pos = computePosition( + const pos = computeFloatingPosition( anchorRect, floatingRect.width, floatingRect.height, diff --git a/packages/web/src/stores/uiStore.ts b/packages/web/src/stores/uiStore.ts index 4f8a94d9..3fb612dd 100644 --- a/packages/web/src/stores/uiStore.ts +++ b/packages/web/src/stores/uiStore.ts @@ -1,6 +1,7 @@ import { create } from 'zustand'; import { persist, createJSONStorage } from 'zustand/middleware'; import type { User } from '@backspace/shared'; +import type { AnchorRect, Placement } from '../hooks/useFloatingPosition'; type ModalType = | 'createSpace' @@ -40,7 +41,11 @@ interface UIState { imagePreviewUrl: string | null; userProfilePopout: { user: User | null; - position: { top: number; left: number } | null; + /** Rect of the element the card was opened from. The card places itself off + * this rect once it knows its own measured size — callers never compute + * coordinates, so no surface can drift by re-anchoring to itself. */ + anchor: AnchorRect | null; + placement: Placement; }; toasts: Toast[]; toggleSidebar: () => void; @@ -51,7 +56,7 @@ interface UIState { setShowDms: (show: boolean) => void; openImagePreview: (url: string) => void; closeImagePreview: () => void; - openUserProfile: (user: User, position: { top: number; left: number }) => void; + openUserProfile: (user: User, anchor: AnchorRect, placement?: Placement) => void; closeUserProfile: () => void; addToast: (message: string, type?: 'info' | 'warning' | 'success', duration?: number) => void; removeToast: (id: string) => void; @@ -93,7 +98,8 @@ export const useUIStore = create()( imagePreviewUrl: null, userProfilePopout: { user: null, - position: null, + anchor: null, + placement: 'right', }, toasts: [], @@ -120,7 +126,7 @@ export const useUIStore = create()( openImagePreview: (url) => set({ activeModal: 'imagePreview', imagePreviewUrl: url }), closeImagePreview: () => set({ activeModal: null, imagePreviewUrl: null }), - openUserProfile: (user, position) => { + openUserProfile: (user, anchor, placement = 'right') => { if (get().isMobile) { // On mobile, push a full-screen user profile instead of a positioned popout set((state) => ({ @@ -128,11 +134,11 @@ export const useUIStore = create()( })); history.pushState({ mobileScreen: 'user-profile' }, ''); } else { - set({ userProfilePopout: { user, position } }); + set({ userProfilePopout: { user, anchor, placement } }); } }, closeUserProfile: () => set({ - userProfilePopout: { user: null, position: null } + userProfilePopout: { user: null, anchor: null, placement: 'right' } }), addToast: (message, type = 'info', duration = 5000) => { diff --git a/packages/web/src/test/setup.ts b/packages/web/src/test/setup.ts index d7d1a0ca..60c08386 100644 --- a/packages/web/src/test/setup.ts +++ b/packages/web/src/test/setup.ts @@ -112,3 +112,20 @@ const OriginalResponse = globalThis.Response; return b; } }; + +// jsdom does not implement ResizeObserver. Floating surfaces (tooltips, +// popovers, the profile card) observe their own box so they can re-place +// themselves when their content grows. Provide an inert stub — tests drive +// layout explicitly by stubbing getBoundingClientRect. +if (!('ResizeObserver' in globalThis)) { + class NoopResizeObserver implements ResizeObserver { + observe(): void {} + unobserve(): void {} + disconnect(): void {} + } + Object.defineProperty(globalThis, 'ResizeObserver', { + value: NoopResizeObserver, + configurable: true, + writable: true, + }); +}