Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,11 @@ test("timelineRowReserveStyle: message item yields containIntrinsicSize", () =>
assert.match(String(style.containIntrinsicSize), /^auto \d+px$/);
});

test("timelineRowReserveStyle: divider is short fixed height", () => {
test("timelineRowReserveStyle: divider reserves its visual breathing room", () => {
const style = timelineRowReserveStyle({
kind: "day-divider",
key: "k",
headingTimestamp: 0,
});
assert.equal(style.containIntrinsicSize, "auto 32px");
assert.equal(style.containIntrinsicSize, "auto 56px");
});
2 changes: 1 addition & 1 deletion desktop/src/features/messages/lib/rowHeightEstimate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ export function estimateRowHeight(

// Dividers are short, fixed-height rows; reserving their true height keeps the
// estimate honest without a content scan.
const DIVIDER_HEIGHT = 32;
const DIVIDER_HEIGHT = 56;
const SYSTEM_GROUP_HEIGHT = 80;

/**
Expand Down
6 changes: 6 additions & 0 deletions desktop/src/features/messages/lib/systemEventCopy.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,16 @@ import assert from "node:assert/strict";
import test from "node:test";

import {
addedByActionPrefix,
describeChannelTextFieldChange,
toInlineName,
} from "./systemEventCopy.ts";

test("an add to the reader uses passive wording", () => {
assert.equal(addedByActionPrefix(true), "were added by");
assert.equal(addedByActionPrefix(false), "added by");
});

test("a set topic is quoted verbatim", () => {
assert.equal(
describeChannelTextFieldChange("topic", "Release planning"),
Expand Down
9 changes: 9 additions & 0 deletions desktop/src/features/messages/lib/systemEventCopy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,15 @@ const CLOSE_QUOTE = "”";

export type ChannelTextField = "topic" | "purpose";

/**
* The reader is the recipient of an add, while every other member is the
* subject of one. Keep that distinction in the caption: "You were added by"
* rather than the ungrammatical "You added by".
*/
export function addedByActionPrefix(isCurrentUser: boolean): string {
return isCurrentUser ? "were added by" : "added by";
}

/**
* Caption for a channel topic or purpose change.
*
Expand Down
56 changes: 43 additions & 13 deletions desktop/src/features/messages/lib/timelineItems.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,15 @@ function memberJoinedEntry({ createdAt, id, target }) {
return memberAddedEntry({ actor: target, createdAt, id, target });
}

function memberLeftEntry({ createdAt, id, target }) {
return entry({
id,
createdAt,
kind: KIND_SYSTEM_MESSAGE,
body: JSON.stringify({ type: "member_left", actor: target }),
});
}

function kinds(items) {
return items.map((item) => item.kind);
}
Expand Down Expand Up @@ -103,12 +112,12 @@ test("buildTimelineItems: system messages flatten to a 'system' item", () => {
assert.deepEqual(kinds(items), ["day-divider", "message", "system"]);
});

test("buildTimelineItems: member additions by one actor group within five minutes", () => {
test("buildTimelineItems: contiguous member additions by one actor group", () => {
const start = dayAt(2026, 6, 14);
const entries = [
memberAddedEntry({ id: "a", target: "target-a", createdAt: start }),
memberAddedEntry({ id: "b", target: "target-b", createdAt: start + 60 }),
memberAddedEntry({ id: "c", target: "target-c", createdAt: start + 300 }),
memberAddedEntry({ id: "c", target: "target-c", createdAt: start + 3_600 }),
];

const { items } = buildTimelineItems(entries, null);
Expand All @@ -121,7 +130,7 @@ test("buildTimelineItems: member additions by one actor group within five minute
assert.equal(group?.key, "c");
});

test("buildTimelineItems: self-joins group across different members within five minutes", () => {
test("buildTimelineItems: contiguous self-joins group across different members", () => {
const start = dayAt(2026, 6, 14);
const entries = [
memberJoinedEntry({ id: "a", target: "target-a", createdAt: start }),
Expand All @@ -133,7 +142,7 @@ test("buildTimelineItems: self-joins group across different members within five
memberJoinedEntry({
id: "c",
target: "target-c",
createdAt: start + 300,
createdAt: start + 3_600,
}),
];

Expand All @@ -149,9 +158,9 @@ test("buildTimelineItems: self-joins group across different members within five
test("buildTimelineItems: prepending membership history preserves the loaded suffix", () => {
const start = dayAt(2026, 6, 14);
const loaded = [
memberAddedEntry({ id: "b", target: "target-b", createdAt: start + 240 }),
memberAddedEntry({ id: "c", target: "target-c", createdAt: start + 360 }),
entry({ id: "message", createdAt: start + 600 }),
memberAddedEntry({ id: "b", target: "target-b", createdAt: start + 3_500 }),
memberAddedEntry({ id: "c", target: "target-c", createdAt: start + 3_601 }),
entry({ id: "message", createdAt: start + 3_700 }),
];
const prepended = [
memberAddedEntry({ id: "a", target: "target-a", createdAt: start }),
Expand All @@ -164,23 +173,28 @@ test("buildTimelineItems: prepending membership history preserves the loaded suf
const prependedKeys = prependedItems.slice(1).map((item) => item.key);

assert.deepEqual(loadedKeys, ["c", "message"]);
assert.deepEqual(prependedKeys, ["a", "c", "message"]);
assert.deepEqual(prependedKeys, ["c", "message"]);
assert.deepEqual(prependedKeys.slice(-loadedKeys.length), loadedKeys);
});

test("buildTimelineItems: member-add window is fixed from the newest addition", () => {
test("buildTimelineItems: contiguous member additions extend a group outside one hour", () => {
const start = dayAt(2026, 6, 14);
const entries = [
memberAddedEntry({ id: "a", target: "target-a", createdAt: start }),
memberAddedEntry({ id: "b", target: "target-b", createdAt: start + 240 }),
memberAddedEntry({ id: "c", target: "target-c", createdAt: start + 301 }),
memberAddedEntry({ id: "b", target: "target-b", createdAt: start + 3_599 }),
memberAddedEntry({ id: "c", target: "target-c", createdAt: start + 3_601 }),
];

const { items } = buildTimelineItems(entries, null);
assert.deepEqual(kinds(items), ["day-divider", "system", "system-group"]);
assert.deepEqual(kinds(items), ["day-divider", "system-group"]);
const group = items.find((item) => item.kind === "system-group");
assert.deepEqual(
group?.entries.map((groupEntry) => groupEntry.message.id),
["a", "b", "c"],
);
});

test("buildTimelineItems: actor changes and intervening rows break member-add groups", () => {
test("buildTimelineItems: incompatible arrivals remain separate", () => {
const start = dayAt(2026, 6, 14);
const entries = [
memberAddedEntry({ id: "a", target: "target-a", createdAt: start }),
Expand Down Expand Up @@ -216,6 +230,22 @@ test("buildTimelineItems: actor changes and intervening rows break member-add gr
]);
});

test("buildTimelineItems: a member joining then leaving is one lifecycle group", () => {
const start = dayAt(2026, 6, 14);
const entries = [
memberJoinedEntry({ id: "joined", target: "member-a", createdAt: start }),
memberLeftEntry({ id: "left", target: "member-a", createdAt: start + 90 }),
];

const { items } = buildTimelineItems(entries, null);
assert.deepEqual(kinds(items), ["day-divider", "system-group"]);
const group = items.find((item) => item.kind === "system-group");
assert.deepEqual(
group?.entries.map((groupEntry) => groupEntry.message.id),
["joined", "left"],
);
});

test("buildTimelineItems: consecutive same-author messages within the window are grouped", () => {
const entries = [
entry({ id: "a", pubkey: "author-a", createdAt: dayAt(2026, 6, 14) }),
Expand Down
41 changes: 27 additions & 14 deletions desktop/src/features/messages/lib/timelineItems.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,13 +62,10 @@ function entryRenderKey(entry: MainTimelineEntry): string {
return entry.message.renderKey ?? entry.message.id;
}

const MEMBERSHIP_GROUP_WINDOW_SECONDS = 5 * 60;

type MembershipChangePayload = {
actor: string | null;
mode: "added" | "joined";
target: string;
};
type MembershipChangePayload =
| { mode: "self-arrival"; target: string }
| { actor: string; mode: "addition"; target: string }
| { mode: "departure"; target: string };

function parseMembershipChangePayload(
entry: MainTimelineEntry,
Expand All @@ -81,6 +78,10 @@ function parseMembershipChangePayload(
actor?: unknown;
target?: unknown;
};
if (payload.type === "member_left" && typeof payload.actor === "string") {
const target = payload.actor.trim().toLowerCase();
return target ? { mode: "departure", target } : null;
}
Comment thread
klopez4212 marked this conversation as resolved.
if (
payload.type !== "member_joined" ||
typeof payload.actor !== "string" ||
Expand All @@ -92,10 +93,9 @@ function parseMembershipChangePayload(
const actor = payload.actor.trim().toLowerCase();
const target = payload.target.trim().toLowerCase();
if (!actor || !target) return null;

return actor === target
? { actor: null, mode: "joined", target }
: { actor, mode: "added", target };
? { mode: "self-arrival", target }
: { actor, mode: "addition", target };
} catch {
return null;
}
Expand All @@ -105,9 +105,16 @@ function membershipChangesCanGroup(
first: MembershipChangePayload,
second: MembershipChangePayload,
): boolean {
if (first.mode === "self-arrival") {
return (
second.mode === "self-arrival" ||
(second.mode === "departure" && first.target === second.target)
);
}
return (
first.mode === second.mode &&
(first.mode === "joined" || first.actor === second.actor)
first.mode === "addition" &&
second.mode === "addition" &&
first.actor === second.actor
);
}

Expand All @@ -116,6 +123,12 @@ function membershipChangesCanGroup(
* history cannot repartition the rows that are already loaded. Their key is
* likewise the newest entry's key: extending the oldest visible group changes
* its contents, but not its identity or the virtual list's existing key suffix.
*
* Compatible membership activities stay together while they are contiguous.
* Self-joins and additions from one administrator each form their own summary;
* a self-join immediately followed by that member leaving becomes a single
* lifecycle summary. Each adjacent event must fall within the one-hour activity
* window, so uninterrupted activity can extend beyond an hour overall.
*/
function buildMembershipGroups(
entries: readonly MainTimelineEntry[],
Expand All @@ -134,14 +147,14 @@ function buildMembershipGroups(
let start = end;
while (start > 0) {
const candidate = entries[start - 1];
const nextEntry = entries[start];
const candidatePayload = parseMembershipChangePayload(candidate);
if (
barrierIndexes.has(start) ||
!candidatePayload ||
!membershipChangesCanGroup(candidatePayload, newestPayload) ||
newestEntry.message.createdAt < candidate.message.createdAt ||
newestEntry.message.createdAt - candidate.message.createdAt >
MEMBERSHIP_GROUP_WINDOW_SECONDS
nextEntry.message.createdAt - candidate.message.createdAt > 60 * 60
) {
Comment thread
klopez4212 marked this conversation as resolved.
break;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ test("virtualized rows preserve their heterogeneous height estimates", () => {
);
const estimates = items.map(estimateVirtualizedTimelineItemHeight);

assert.equal(estimates[0], 32);
assert.equal(estimates[0], 56);
assert.ok(estimates[2] > estimates[1] + 200);
assert.equal(estimates.at(-1), 96);
});
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export function estimateVirtualizedTimelineItemHeight(
): number {
if (item.kind === "bottom-spacer") return 96;
if (item.kind === "leading-content") return 60;
if (item.kind === "day-divider") return 32;
if (item.kind === "day-divider") return 56;
return estimateTimelineItemHeight(item.item);
}

Expand Down
24 changes: 21 additions & 3 deletions desktop/src/features/messages/ui/DayDivider.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,27 @@
export function DayDivider({ label }: { label: string }) {
import { cn } from "@/shared/lib/cn";
import { channelChrome } from "@/shared/layout/chromeLayout";

export function DayDivider({
label,
sticky = true,
testId = "message-timeline-day-divider",
}: {
label: string;
sticky?: boolean;
testId?: string;
}) {
return (
<section
aria-label={label}
className="pointer-events-none sticky top-(--buzz-channel-content-top-padding,5.75rem) z-20 flex justify-center"
data-testid="message-timeline-day-divider"
className={cn(
sticky
? cn(
"pointer-events-none sticky z-20 flex justify-center",
channelChrome.stickyTimelineTop,
)
: "pointer-events-none flex justify-center",
)}
data-testid={testId}
data-day-label={label}
>
<p className="relative z-10 shrink-0 rounded-full border border-border/70 bg-background px-2.5 py-1 text-2xs font-medium tracking-[0.02em] text-muted-foreground/70">
Expand Down
8 changes: 4 additions & 4 deletions desktop/src/features/messages/ui/MessageTimeline.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -689,8 +689,8 @@ const MessageTimelineBase = React.forwardRef<
{showUnreadPill ? (
<div
className={cn(
"pointer-events-none absolute inset-x-0 z-30 flex translate-y-3 justify-center px-4",
channelChrome.top,
"pointer-events-none absolute inset-x-0 z-30 flex justify-center px-4",
channelChrome.stickyTimelineTop,
)}
>
<UnreadPill
Expand All @@ -709,8 +709,8 @@ const MessageTimelineBase = React.forwardRef<
isRenderedTimelineBehindHistoryPrepend(deferredMessages, messages) ? (
<div
className={cn(
"pointer-events-none absolute inset-x-0 z-30 flex translate-y-3 justify-center px-4",
channelChrome.top,
"pointer-events-none absolute inset-x-0 z-30 flex justify-center px-4",
channelChrome.stickyTimelineTop,
)}
data-testid="message-timeline-fetching-older"
>
Expand Down
Loading
Loading