fix(ui): AvatarStack — center inner Avatar in each tile (off-center clipping)
Each AvatarTile rendered at `size × size` with a 2px border under
`box-sizing: border-box`, giving a content area of `(size − 4) × (size − 4)`.
The inner `<Avatar size={size}>` exceeded the padding box, and the
`overflow-hidden + rounded-full` clip — centered on the wrapper — combined
with the avatar contents anchored at the padding-edge top-left to displace
photos and especially the centered initials gradient + letter toward the
lower-right of the visible disc, leaving a sliver of background opposite.
Compounding this, `Avatar`'s `inline-flex` root sat on the line-box text
baseline, so any inherited `line-height ≠ 1` (the DM list inherits the
row's line-height) drifted the avatar a further several px vertically.
Two corrections:
• Size the inner Avatar to `size − 2·TILE_BORDER_WIDTH` (matches the
padding box) — extracted as a constant so the dependency between
`border-2` and the inner size is visible.
• Center geometrically via `flex items-center justify-center` on the
wrapper, bypassing inline-flow placement so the Avatar is anchored
regardless of inherited type metrics.
Verified in the live app (DM sidebar, chat header, welcome header) and
with a 4-tile diamond at sizes 32 and 80: visible offset is now exactly
the border width on every tile, letters/photos sit dead-center.
Adds a regression test that pins both invariants (flex centering classes
present + inner Avatar style.width === tileSize − 4) so a future change
that re-introduces the bug fails fast. 12/12 AvatarStack tests, 365/365
web tests, typecheck clean.
design-system.md spec updated with the AvatarTile geometry contract.
This commit is contained in:
@@ -199,6 +199,8 @@ interface AvatarStackProps {
|
|||||||
|
|
||||||
**Hooks-in-loop safety:** each rendered slot is its own `<AvatarTile>` component so `useCanonicalUserView` is called exactly once per slot, never inside a variable-length `.map()`.
|
**Hooks-in-loop safety:** each rendered slot is its own `<AvatarTile>` component so `useCanonicalUserView` is called exactly once per slot, never inside a variable-length `.map()`.
|
||||||
|
|
||||||
|
**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`).
|
**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`).
|
||||||
|
|
||||||
**Usage sites** (all six call sites in the codebase):
|
**Usage sites** (all six call sites in the codebase):
|
||||||
|
|||||||
@@ -164,6 +164,35 @@ describe('AvatarStack', () => {
|
|||||||
expect(overflowTop).toBeGreaterThan(Math.max(...tileTops) - 1);
|
expect(overflowTop).toBeGreaterThan(Math.max(...tileTops) - 1);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('sizes the inner Avatar to the tile content area so contents stay centered', () => {
|
||||||
|
// Regression for the off-center clipping bug: when the inner Avatar was
|
||||||
|
// sized to the outer tile dimensions, `box-sizing: border-box` + 2px border
|
||||||
|
// pushed its contents (image crop, initials gradient + letter) toward the
|
||||||
|
// lower-right of the visible disc. The fix is to size the inner Avatar to
|
||||||
|
// (tileSize − 2·border) and center it geometrically with flex.
|
||||||
|
const { container } = render(
|
||||||
|
<AvatarStack members={makeUsers(4)} size={80} border="chat" />,
|
||||||
|
);
|
||||||
|
const tiles = Array.from(
|
||||||
|
container.querySelectorAll('[data-avatar-stack-tile]'),
|
||||||
|
) as HTMLElement[];
|
||||||
|
expect(tiles.length).toBe(4);
|
||||||
|
for (const tile of tiles) {
|
||||||
|
// Tile is centered as a flex container so the Avatar bypasses inline-flow
|
||||||
|
// baseline drift entirely.
|
||||||
|
expect(tile.className).toMatch(/\bflex\b/);
|
||||||
|
expect(tile.className).toMatch(/items-center/);
|
||||||
|
expect(tile.className).toMatch(/justify-center/);
|
||||||
|
// Inner Avatar is sized to (tileSize − 2·border) so it fits the padding
|
||||||
|
// box exactly. With size=80 and tileRatio=0.58, tileSize=46 → inner=42.
|
||||||
|
const tileSize = parseInt(tile.style.width, 10);
|
||||||
|
const inner = tile.querySelector('[data-avatar]') as HTMLElement | null;
|
||||||
|
expect(inner).toBeTruthy();
|
||||||
|
expect(inner!.style.width).toBe(`${tileSize - 4}px`);
|
||||||
|
expect(inner!.style.height).toBe(`${tileSize - 4}px`);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
it('renders icon override and ignores the stack', () => {
|
it('renders icon override and ignores the stack', () => {
|
||||||
const { container } = render(
|
const { container } = render(
|
||||||
<AvatarStack
|
<AvatarStack
|
||||||
|
|||||||
@@ -50,6 +50,18 @@ const BORDER_CLASS: Record<AvatarStackProps['border'], string> = {
|
|||||||
modal: 'border-surface-elevated',
|
modal: 'border-surface-elevated',
|
||||||
};
|
};
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Width (in px) of the `border-2` Tailwind class applied to every tile,
|
||||||
|
* placeholder, badge, and overflow slot. Centralized as a constant because
|
||||||
|
* `box-sizing: border-box` (Tailwind preflight default) makes a wrapper with
|
||||||
|
* `width: N` + `border-2` have a content area of `(N − 2·BORDER) × (N − 2·BORDER)`.
|
||||||
|
* Inner content (e.g. the `Avatar` inside an `AvatarTile`) must be sized to
|
||||||
|
* the content area, not the outer width — otherwise it overflows the padding
|
||||||
|
* box, gets clipped off-center by `overflow-hidden + rounded-full`, and the
|
||||||
|
* visible avatar/initials end up shifted toward the upper-left of the tile.
|
||||||
|
*/
|
||||||
|
const TILE_BORDER_WIDTH = 2;
|
||||||
|
|
||||||
/** Resolves a bare filename to /api/uploads/, leaves absolute URLs alone. */
|
/** Resolves a bare filename to /api/uploads/, leaves absolute URLs alone. */
|
||||||
function resolveIconSrc(iconUrl: string): string {
|
function resolveIconSrc(iconUrl: string): string {
|
||||||
if (iconUrl.startsWith('http') || iconUrl.startsWith('blob:') || iconUrl.startsWith('data:') || iconUrl.startsWith('/')) {
|
if (iconUrl.startsWith('http') || iconUrl.startsWith('blob:') || iconUrl.startsWith('data:') || iconUrl.startsWith('/')) {
|
||||||
@@ -92,16 +104,32 @@ function AvatarTile({
|
|||||||
}) {
|
}) {
|
||||||
const canonical = useCanonicalUserView(member);
|
const canonical = useCanonicalUserView(member);
|
||||||
const displayName = canonical.displayName ?? parseFederatedUsername(canonical.username).baseName;
|
const displayName = canonical.displayName ?? parseFederatedUsername(canonical.username).baseName;
|
||||||
|
// Two corrections on top of the previous "drop the Avatar straight in" form:
|
||||||
|
//
|
||||||
|
// 1. Box-sizing. The wrapper renders at `size × size` with a 2px border
|
||||||
|
// (border-box default), so its content area is `(size − 4) × (size − 4)`.
|
||||||
|
// An Avatar sized to `size` would overflow the padding box and get
|
||||||
|
// clipped off-center — the clip is centered on the wrapper but the
|
||||||
|
// Avatar starts at the padding-edge top-left, so its contents (image
|
||||||
|
// crop, initials gradient + letter) end up displaced toward the
|
||||||
|
// lower-right of the visible disc.
|
||||||
|
// 2. Inline-flex baseline. `Avatar`'s root is `inline-flex`, which makes
|
||||||
|
// it sit on the line box's text baseline. With any non-1 inherited
|
||||||
|
// `line-height`, the Avatar drifts vertically inside its container,
|
||||||
|
// not just horizontally. Centering it via the wrapper (`flex` +
|
||||||
|
// `items-center justify-center`) bypasses inline layout entirely so
|
||||||
|
// the Avatar is anchored geometrically, regardless of inherited type.
|
||||||
|
const innerSize = Math.max(0, size - 2 * TILE_BORDER_WIDTH);
|
||||||
return (
|
return (
|
||||||
<div
|
<div
|
||||||
data-avatar-stack-tile="true"
|
data-avatar-stack-tile="true"
|
||||||
className={`absolute rounded-full overflow-hidden border-2 ${borderClass} ${className}`}
|
className={`absolute rounded-full overflow-hidden border-2 flex items-center justify-center ${borderClass} ${className}`}
|
||||||
style={{ width: size, height: size, ...style }}
|
style={{ width: size, height: size, ...style }}
|
||||||
>
|
>
|
||||||
<Avatar
|
<Avatar
|
||||||
src={canonical.avatar}
|
src={canonical.avatar}
|
||||||
name={displayName}
|
name={displayName}
|
||||||
size={size}
|
size={innerSize}
|
||||||
userId={canonical.homeUserId ?? canonical.id}
|
userId={canonical.homeUserId ?? canonical.id}
|
||||||
user={canonical}
|
user={canonical}
|
||||||
/>
|
/>
|
||||||
|
|||||||
Reference in New Issue
Block a user