mirror of
https://github.com/block/buzz.git
synced 2026-11-12 12:22:06 +00:00
fix(desktop): collapse mark-read/unread menu into one toggling item
The message action menu rendered two independent items ("Mark unread"
and "Mark read"), each gated only on prop presence, so both showed at
once. Collapse them into a single DropdownMenuItem whose label, icon, and
handler branch on the clicked message's own read state, mirroring the
existing Follow-thread toggle in the same file.
The read state comes from isMessageUnread — the same computeThreadUnreadMarker
predicate the unread badge uses — so the menu label and badge cannot drift.
Its callback identity now depends on a forcedUnreadVersion counter so a
mark-read/unread re-flows through the memoized message subtree (the
forced-unread overlay is a ref, otherwise invisible to React).
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
This commit is contained in:
parent
4769564d8a
commit
d053578f15
@@ -39,10 +39,11 @@ const overrides = new Map([
|
||||
["src-tauri/src/nostr_convert.rs", 1126],
|
||||
["src/shared/api/relayClientSession.ts", 1022],
|
||||
["src-tauri/src/migration.rs", 1295],
|
||||
// onMarkRead prop-pair completion (mirrors the onMarkUnread prop already
|
||||
// threaded here) — a 1-line overage, not generic debt growth. Approved
|
||||
// override; still queued to split with the rest of this list.
|
||||
["src/features/messages/ui/MessageThreadPanel.tsx", 1002],
|
||||
// onMarkRead + isUnread prop threading (mirrors the onMarkUnread prop
|
||||
// already here) for the single-toggle mark-read/unread menu item — a small
|
||||
// overage from load-bearing per-message plumbing, not generic debt growth.
|
||||
// Approved override; still queued to split with the rest of this list.
|
||||
["src/features/messages/ui/MessageThreadPanel.tsx", 1006],
|
||||
]);
|
||||
|
||||
await runFileSizeCheck({
|
||||
|
||||
@@ -168,6 +168,7 @@ type ChannelPaneProps = {
|
||||
followThreadById?: (rootId: string) => void;
|
||||
unfollowThreadById?: (rootId: string) => void;
|
||||
isFollowingThreadById?: (rootId: string) => boolean;
|
||||
isMessageUnreadById?: (messageId: string) => boolean;
|
||||
};
|
||||
|
||||
export const ChannelPane = React.memo(function ChannelPane({
|
||||
@@ -186,6 +187,7 @@ export const ChannelPane = React.memo(function ChannelPane({
|
||||
followThreadById,
|
||||
isFollowingThread,
|
||||
isFollowingThreadById,
|
||||
isMessageUnreadById,
|
||||
isJoining = false,
|
||||
isSinglePanelView = false,
|
||||
isSending,
|
||||
@@ -658,6 +660,7 @@ export const ChannelPane = React.memo(function ChannelPane({
|
||||
hasOlderMessages={hasOlderMessages}
|
||||
isFetchingOlder={isFetchingOlder}
|
||||
isFollowingThreadById={isFollowingThreadById}
|
||||
isMessageUnreadById={isMessageUnreadById}
|
||||
personaLookup={personaLookup}
|
||||
profiles={profiles}
|
||||
unfollowThreadById={unfollowThreadById}
|
||||
@@ -807,6 +810,7 @@ export const ChannelPane = React.memo(function ChannelPane({
|
||||
editTarget={threadEditTarget}
|
||||
firstUnreadReplyId={threadFirstUnreadReplyId}
|
||||
isFollowingThread={isFollowingThread}
|
||||
isMessageUnreadById={isMessageUnreadById}
|
||||
isSending={isSending}
|
||||
isSinglePanelView={
|
||||
useSplitAuxiliaryPane ? false : isSinglePanelView
|
||||
|
||||
@@ -394,6 +394,7 @@ export function ChannelScreen({
|
||||
getReplyDescendantIdsForMessage,
|
||||
handleMarkMessageRead,
|
||||
handleMarkMessageUnread,
|
||||
isMessageUnread,
|
||||
markRevealedRepliesRead,
|
||||
openThreadHeadMessage,
|
||||
threadFirstUnreadReplyId,
|
||||
@@ -740,6 +741,7 @@ export function ChannelScreen({
|
||||
followThreadById={followThread}
|
||||
unfollowThreadById={unfollowThread}
|
||||
isFollowingThreadById={isFollowingThread}
|
||||
isMessageUnreadById={isMessageUnread}
|
||||
isFollowingThread={isNotifiedForEffectiveThread}
|
||||
isSending={sendMessageMutation.isPending}
|
||||
isSinglePanelView={isSinglePanelView}
|
||||
|
||||
@@ -86,7 +86,10 @@ export function useChannelUnreadState({
|
||||
// the marker for such channels to avoid that visible contradiction. The flag
|
||||
// is cleared on re-open (a fresh snapshot is recomputed for the channel).
|
||||
const forcedUnreadRef = React.useRef(new Set<string>());
|
||||
const [, forceUnreadRender] = React.useReducer((n: number) => n + 1, 0);
|
||||
const [forcedUnreadVersion, forceUnreadRender] = React.useReducer(
|
||||
(n: number) => n + 1,
|
||||
0,
|
||||
);
|
||||
// Per-message analog of forcedUnreadRef (LP4 v3 mark-unread). A monotonic
|
||||
// grow-only msg:<id> marker cannot move the read-line backward, so a
|
||||
// deliberate mark-unread lives in this session-local set, read ONLY as an
|
||||
@@ -147,6 +150,10 @@ export function useChannelUnreadState({
|
||||
() => buildCreatedAtByMessageId(timelineMessages),
|
||||
[timelineMessages],
|
||||
);
|
||||
const messageById = React.useMemo(
|
||||
() => new Map(timelineMessages.map((message) => [message.id, message])),
|
||||
[timelineMessages],
|
||||
);
|
||||
const threadPanelIndex = React.useMemo(
|
||||
() => buildThreadPanelIndex(timelineMessages),
|
||||
[timelineMessages],
|
||||
@@ -345,6 +352,40 @@ export function useChannelUnreadState({
|
||||
],
|
||||
);
|
||||
|
||||
// Per-message unread predicate for the mark-read/unread menu toggle. Reuses
|
||||
// computeThreadUnreadMarker — the exact function the badge counts call
|
||||
// (computeThreadBadgeCounts) — over a single-message array, so the menu label
|
||||
// and the badge can never disagree: one source of truth, no re-derived
|
||||
// predicate to drift. A message absent from the timeline (never loaded) is
|
||||
// treated as read, matching the badge, which only tallies loaded messages.
|
||||
// readStateVersion recomputes on marker advances; forcedUnreadVersion bumps
|
||||
// on every mark-read/unread so the callback identity changes and the value
|
||||
// re-flows through the memoized message subtree (forcedUnreadMsgRef is a ref,
|
||||
// invisible to React on its own). Both keep the menu label and the badge —
|
||||
// which read the same computeThreadUnreadMarker predicate — from drifting.
|
||||
// biome-ignore lint/correctness/useExhaustiveDependencies: readStateVersion and forcedUnreadVersion are intentional recompute triggers
|
||||
const isMessageUnread = React.useCallback(
|
||||
(messageId: string): boolean => {
|
||||
const message = messageById.get(messageId);
|
||||
if (!message) return false;
|
||||
const { firstUnreadReplyId } = computeThreadUnreadMarker(
|
||||
[message],
|
||||
getMessageReadAt,
|
||||
currentPubkey,
|
||||
isMsgForcedUnread,
|
||||
);
|
||||
return firstUnreadReplyId !== null;
|
||||
},
|
||||
[
|
||||
messageById,
|
||||
getMessageReadAt,
|
||||
currentPubkey,
|
||||
isMsgForcedUnread,
|
||||
readStateVersion,
|
||||
forcedUnreadVersion,
|
||||
],
|
||||
);
|
||||
|
||||
const handleMarkUnread = React.useCallback(() => {
|
||||
if (!activeChannelId) return;
|
||||
// Mirror the deliberate mark-unread locally so the timeline marker is
|
||||
@@ -427,6 +468,7 @@ export function useChannelUnreadState({
|
||||
handleMarkMessageRead,
|
||||
handleMarkMessageUnread,
|
||||
handleMarkUnread,
|
||||
isMessageUnread,
|
||||
markRevealedRepliesRead,
|
||||
openThreadHeadMessage,
|
||||
threadFirstUnreadReplyId,
|
||||
|
||||
@@ -84,6 +84,7 @@ function MoreActionsMenu({
|
||||
onUnfollowThread,
|
||||
open,
|
||||
isFollowingThread,
|
||||
isUnread,
|
||||
}: {
|
||||
/** Channel UUID for the "Copy link" action. When null/undefined, the
|
||||
* Copy link entry is hidden (e.g. inbox preview rows that don't have it). */
|
||||
@@ -99,6 +100,7 @@ function MoreActionsMenu({
|
||||
onUnfollowThread?: (message: TimelineMessage) => void;
|
||||
open: boolean;
|
||||
isFollowingThread?: boolean;
|
||||
isUnread?: boolean;
|
||||
}) {
|
||||
const [isDeleteDialogOpen, setIsDeleteDialogOpen] = React.useState(false);
|
||||
// Set true the moment the user picks "Edit message". The
|
||||
@@ -157,25 +159,23 @@ function MoreActionsMenu({
|
||||
</DropdownMenuItem>
|
||||
) : null}
|
||||
|
||||
{onMarkUnread ? (
|
||||
{onMarkRead || onMarkUnread ? (
|
||||
<DropdownMenuItem
|
||||
data-testid={`mark-read-toggle-${message.id}`}
|
||||
onClick={() => {
|
||||
onMarkUnread(message);
|
||||
if (isUnread) {
|
||||
onMarkRead?.(message);
|
||||
} else {
|
||||
onMarkUnread?.(message);
|
||||
}
|
||||
}}
|
||||
>
|
||||
<MailOpen className="h-4 w-4" />
|
||||
Mark unread
|
||||
</DropdownMenuItem>
|
||||
) : null}
|
||||
|
||||
{onMarkRead ? (
|
||||
<DropdownMenuItem
|
||||
onClick={() => {
|
||||
onMarkRead(message);
|
||||
}}
|
||||
>
|
||||
<MailCheck className="h-4 w-4" />
|
||||
Mark read
|
||||
{isUnread ? (
|
||||
<MailCheck className="h-4 w-4" />
|
||||
) : (
|
||||
<MailOpen className="h-4 w-4" />
|
||||
)}
|
||||
{isUnread ? "Mark read" : "Mark unread"}
|
||||
</DropdownMenuItem>
|
||||
) : null}
|
||||
|
||||
@@ -356,6 +356,7 @@ export function MessageActionBar({
|
||||
reactionErrorMessage = null,
|
||||
reactions,
|
||||
isFollowingThread,
|
||||
isUnread,
|
||||
}: {
|
||||
/** Channel UUID — required for the "Copy link" action; when omitted the
|
||||
* action is hidden (callers like the home inbox that lack the context). */
|
||||
@@ -374,6 +375,9 @@ export function MessageActionBar({
|
||||
reactionErrorMessage?: string | null;
|
||||
reactions: TimelineReaction[];
|
||||
isFollowingThread?: boolean;
|
||||
/** Current read state of the clicked message, from the same predicate the
|
||||
* unread badge uses. Drives the single mark-read/unread toggle label. */
|
||||
isUnread?: boolean;
|
||||
}) {
|
||||
const [isReactionPickerOpen, setIsReactionPickerOpen] = React.useState(false);
|
||||
const [isDropdownOpen, setIsDropdownOpen] = React.useState(false);
|
||||
@@ -552,6 +556,7 @@ export function MessageActionBar({
|
||||
onUnfollowThread={onUnfollowThread}
|
||||
open={isDropdownOpen}
|
||||
isFollowingThread={isFollowingThread}
|
||||
isUnread={isUnread}
|
||||
/>
|
||||
) : null}
|
||||
</div>
|
||||
|
||||
@@ -56,6 +56,7 @@ export const MessageRow = React.memo(
|
||||
actionBarPlacement = "floating",
|
||||
collapseDescendantsLabel,
|
||||
isFollowingThread,
|
||||
isUnread,
|
||||
layoutVariant = "default",
|
||||
message,
|
||||
onCollapseDepthGuide,
|
||||
@@ -89,6 +90,7 @@ export const MessageRow = React.memo(
|
||||
actionBarPlacement?: "floating" | "inside";
|
||||
collapseDescendantsLabel?: string;
|
||||
isFollowingThread?: boolean;
|
||||
isUnread?: boolean;
|
||||
layoutVariant?: "default" | "thread-reply";
|
||||
message: TimelineMessage;
|
||||
onCollapseDepthGuide?: (message: TimelineMessage) => void;
|
||||
@@ -346,6 +348,7 @@ export const MessageRow = React.memo(
|
||||
<MessageActionBar
|
||||
channelId={channelId}
|
||||
isFollowingThread={isFollowingThread}
|
||||
isUnread={isUnread}
|
||||
message={message}
|
||||
onDelete={onDelete}
|
||||
onEdit={onEdit}
|
||||
@@ -739,6 +742,7 @@ export const MessageRow = React.memo(
|
||||
prev.highlightThreadLineDepths === next.highlightThreadLineDepths &&
|
||||
prev.hoverBackground === next.hoverBackground &&
|
||||
prev.isFollowingThread === next.isFollowingThread &&
|
||||
prev.isUnread === next.isUnread &&
|
||||
prev.layoutVariant === next.layoutVariant &&
|
||||
prev.onCollapseDepthGuide === next.onCollapseDepthGuide &&
|
||||
prev.onCollapseDepthGuideHoverChange ===
|
||||
|
||||
@@ -89,6 +89,7 @@ type MessageThreadPanelProps = {
|
||||
toolbarExtraActions?: React.ReactNode;
|
||||
widthPx: number;
|
||||
isFollowingThread?: boolean;
|
||||
isMessageUnreadById?: (messageId: string) => boolean;
|
||||
onFollowThread?: () => void;
|
||||
onUnfollowThread?: () => void;
|
||||
};
|
||||
@@ -348,6 +349,7 @@ export function MessageThreadPanel({
|
||||
isSending,
|
||||
isSinglePanelView = false,
|
||||
isFollowingThread,
|
||||
isMessageUnreadById,
|
||||
onCancelEdit,
|
||||
onCancelReply,
|
||||
onClose,
|
||||
@@ -644,6 +646,7 @@ export function MessageThreadPanel({
|
||||
highlightedBranch?.id === threadHead.id
|
||||
}
|
||||
isFollowingThread={isFollowingThread}
|
||||
isUnread={isMessageUnreadById?.(threadHead.id)}
|
||||
layoutVariant="thread-reply"
|
||||
message={threadHead}
|
||||
onCollapseDescendants={
|
||||
@@ -764,6 +767,7 @@ export function MessageThreadPanel({
|
||||
}
|
||||
highlightThreadLineDepths={highlightedLineDepths}
|
||||
hoverBackground={!entry.summary}
|
||||
isUnread={isMessageUnreadById?.(entry.message.id)}
|
||||
layoutVariant="thread-reply"
|
||||
message={entry.message}
|
||||
onCollapseDepthGuide={handleCollapseDepthGuide}
|
||||
|
||||
@@ -54,6 +54,7 @@ type MessageTimelineProps = {
|
||||
profiles?: UserProfileLookup;
|
||||
followThreadById?: (rootId: string) => void;
|
||||
isFollowingThreadById?: (rootId: string) => boolean;
|
||||
isMessageUnreadById?: (messageId: string) => boolean;
|
||||
onDelete?: (message: TimelineMessage) => void;
|
||||
onEdit?: (message: TimelineMessage) => void;
|
||||
onMarkUnread?: (message: TimelineMessage) => void;
|
||||
@@ -146,6 +147,7 @@ const MessageTimelineBase = React.forwardRef<
|
||||
isFetchingOlder = false,
|
||||
followThreadById,
|
||||
isFollowingThreadById,
|
||||
isMessageUnreadById,
|
||||
messageFooters,
|
||||
personaLookup,
|
||||
profiles,
|
||||
@@ -535,6 +537,7 @@ const MessageTimelineBase = React.forwardRef<
|
||||
followThreadById={followThreadById}
|
||||
highlightedMessageId={highlightedMessageId}
|
||||
isFollowingThreadById={isFollowingThreadById}
|
||||
isMessageUnreadById={isMessageUnreadById}
|
||||
messageFooters={messageFooters}
|
||||
messages={deferredMessages}
|
||||
onDelete={onDelete}
|
||||
|
||||
@@ -36,6 +36,7 @@ type TimelineMessageListProps = {
|
||||
followThreadById?: (rootId: string) => void;
|
||||
highlightedMessageId?: string | null;
|
||||
isFollowingThreadById?: (rootId: string) => boolean;
|
||||
isMessageUnreadById?: (messageId: string) => boolean;
|
||||
messageFooters?: Record<string, React.ReactNode>;
|
||||
messages: TimelineMessage[];
|
||||
onDelete?: (message: TimelineMessage) => void;
|
||||
@@ -188,6 +189,7 @@ export const TimelineMessageList = React.memo(function TimelineMessageList({
|
||||
followThreadById,
|
||||
highlightedMessageId = null,
|
||||
isFollowingThreadById,
|
||||
isMessageUnreadById,
|
||||
messageFooters,
|
||||
messages,
|
||||
onDelete,
|
||||
@@ -236,6 +238,7 @@ export const TimelineMessageList = React.memo(function TimelineMessageList({
|
||||
followThreadById={followThreadById}
|
||||
highlightedMessageId={highlightedMessageId}
|
||||
isFollowingThreadById={isFollowingThreadById}
|
||||
isMessageUnreadById={isMessageUnreadById}
|
||||
isSendingVideoReviewComment={isSendingVideoReviewComment}
|
||||
key={row.key}
|
||||
messageFooters={messageFooters}
|
||||
@@ -279,6 +282,7 @@ const TimelineRenderRowView = React.memo(function TimelineRenderRowView({
|
||||
followThreadById,
|
||||
highlightedMessageId = null,
|
||||
isFollowingThreadById,
|
||||
isMessageUnreadById,
|
||||
isSendingVideoReviewComment = false,
|
||||
messageFooters,
|
||||
onDelete,
|
||||
@@ -374,6 +378,7 @@ const TimelineRenderRowView = React.memo(function TimelineRenderRowView({
|
||||
? isFollowingThreadById(message.id)
|
||||
: undefined
|
||||
}
|
||||
isUnread={isMessageUnreadById?.(message.id)}
|
||||
message={message}
|
||||
onDelete={
|
||||
onDelete && currentPubkey && message.pubkey === currentPubkey
|
||||
@@ -424,6 +429,7 @@ const TimelineRenderRowView = React.memo(function TimelineRenderRowView({
|
||||
agentPubkeys={agentPubkeys}
|
||||
channelId={channelId}
|
||||
highlighted={message.id === highlightedMessageId || isSearchActive}
|
||||
isUnread={isMessageUnreadById?.(message.id)}
|
||||
message={message}
|
||||
onDelete={
|
||||
onDelete && currentPubkey && message.pubkey === currentPubkey
|
||||
|
||||
@@ -1000,4 +1000,68 @@ test.describe("thread unread indicator screenshots", () => {
|
||||
await expect(page.getByTestId("chat-title")).toHaveText("general");
|
||||
await expect(badge).toHaveCount(0);
|
||||
});
|
||||
|
||||
// The mark-read/unread menu is a SINGLE item whose label toggles by the
|
||||
// clicked message's own read state — driven by the same predicate the unread
|
||||
// badge uses (computeThreadUnreadMarker over the message + its forced-unread
|
||||
// overlay), so the label and badge can never disagree. Pre-fix the menu
|
||||
// rendered TWO simultaneous items ("Mark unread" AND "Mark read") gated only
|
||||
// on prop presence. This pins the single-toggle contract in both states and
|
||||
// through a full round trip.
|
||||
//
|
||||
// A top-level message in the OPEN channel is read-on-open by construction:
|
||||
// ChannelScreen advances the channel frontier to the newest top-level message
|
||||
// and the channel→message fold clears it (this is the badge's own behaviour —
|
||||
// a message in the channel you are looking at is never unread). So the only
|
||||
// route to an unread top-level message here is the mark-unread action itself,
|
||||
// which is exactly what the toggle's forced-unread overlay exists to drive.
|
||||
test("15-mark-read-unread-menu-single-toggle", async ({ page }) => {
|
||||
await installMockBridge(page);
|
||||
await page.goto("/");
|
||||
|
||||
await page.getByTestId("channel-general").click();
|
||||
await expect(page.getByTestId("chat-title")).toHaveText("general");
|
||||
await waitForMockLiveSubscription(page, "general");
|
||||
|
||||
// Emit an Alice-authored (non-self) top-level message, read-on-open.
|
||||
const message = await emitMockMessage(page, "general", "Toggle me", {
|
||||
pubkey: TEST_IDENTITIES.alice.pubkey,
|
||||
createdAt: Math.floor(Date.now() / 1000) - 10,
|
||||
});
|
||||
const messageId = message?.id ?? "";
|
||||
|
||||
const toggle = page.getByTestId(`mark-read-toggle-${messageId}`);
|
||||
const moreActions = page.getByTestId(`more-actions-${messageId}`);
|
||||
|
||||
// Selecting a DropdownMenuItem closes the Radix menu and returns focus to
|
||||
// the trigger. Re-clicking the trigger before that close settles is eaten
|
||||
// by Radix's closing transition (the menu never reopens). Gate each reopen
|
||||
// on the previous menu being fully unmounted — the toggle testid only
|
||||
// exists while the dropdown content is mounted, so count 0 is a reliable
|
||||
// "closed" signal — then re-hover from a clean state before re-clicking.
|
||||
const openMenu = async () => {
|
||||
await expect(toggle).toHaveCount(0);
|
||||
await page.mouse.move(0, 0);
|
||||
await page.getByText("Toggle me").hover();
|
||||
await moreActions.click();
|
||||
await expect(toggle).toHaveCount(1);
|
||||
};
|
||||
|
||||
// Read → the single item reads "Mark unread", and there is exactly one
|
||||
// (never both items at once). Clicking it forces the message unread.
|
||||
await openMenu();
|
||||
await expect(toggle).toHaveText("Mark unread");
|
||||
await toggle.click();
|
||||
|
||||
// Now unread → the same single item shows the inverse label. Clicking it
|
||||
// marks the message read again.
|
||||
await openMenu();
|
||||
await expect(toggle).toHaveText("Mark read");
|
||||
await toggle.click();
|
||||
|
||||
// Back to read → the label has toggled back, still a single item. The
|
||||
// round trip proves the label tracks the live predicate, not prop presence.
|
||||
await openMenu();
|
||||
await expect(toggle).toHaveText("Mark unread");
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user