fix(app): keep unread channel opens anchored to the divider

This commit is contained in:
Hampus authored and GitHub committed 2026-08-27 02:02:45 +02:00
1 parent 8e17970632
commit d3976e33f8
8 files changed
+222 -6

No files matched your search

@@ -23,6 +23,7 @@ interface BlockedMessageGroupsProps {
messageGroups: Array<ChannelStreamItem>; messageGroups: Array<ChannelStreamItem>;
onReveal: (messageId: string | null) => void; onReveal: (messageId: string | null) => void;
revealed: boolean; revealed: boolean;
hasUnread?: boolean;
compact: boolean; compact: boolean;
messageGroupSpacing: number; messageGroupSpacing: number;
variant: 'blocked' | 'spammer'; variant: 'blocked' | 'spammer';
@@ -33,11 +34,13 @@ interface BlockedMessageGroupsProps {
messageActionsClassName?: string; messageActionsClassName?: string;
renderMessageActions?: (message: Message) => React.ReactNode; renderMessageActions?: (message: Message) => React.ReactNode;
renderMessageWrapper?: (props: MessageGroupRenderWrapperProps) => React.ReactNode; renderMessageWrapper?: (props: MessageGroupRenderWrapperProps) => React.ReactNode;
suppressUnreadIndicator?: boolean;
} }
const arePropsEqual = (prevProps: BlockedMessageGroupsProps, nextProps: BlockedMessageGroupsProps): boolean => { const arePropsEqual = (prevProps: BlockedMessageGroupsProps, nextProps: BlockedMessageGroupsProps): boolean => {
if (prevProps.channel.id !== nextProps.channel.id) return false; if (prevProps.channel.id !== nextProps.channel.id) return false;
if (prevProps.revealed !== nextProps.revealed) return false; if (prevProps.revealed !== nextProps.revealed) return false;
if (prevProps.hasUnread !== nextProps.hasUnread) return false;
if (prevProps.compact !== nextProps.compact) return false; if (prevProps.compact !== nextProps.compact) return false;
if (prevProps.messageGroupSpacing !== nextProps.messageGroupSpacing) return false; if (prevProps.messageGroupSpacing !== nextProps.messageGroupSpacing) return false;
if (prevProps.variant !== nextProps.variant) return false; if (prevProps.variant !== nextProps.variant) return false;
@@ -49,6 +52,7 @@ const arePropsEqual = (prevProps: BlockedMessageGroupsProps, nextProps: BlockedM
if (prevProps.messageActionsClassName !== nextProps.messageActionsClassName) return false; if (prevProps.messageActionsClassName !== nextProps.messageActionsClassName) return false;
if (prevProps.renderMessageActions !== nextProps.renderMessageActions) return false; if (prevProps.renderMessageActions !== nextProps.renderMessageActions) return false;
if (prevProps.renderMessageWrapper !== nextProps.renderMessageWrapper) return false; if (prevProps.renderMessageWrapper !== nextProps.renderMessageWrapper) return false;
if (prevProps.suppressUnreadIndicator !== nextProps.suppressUnreadIndicator) return false;
if (prevProps.messageGroups.length !== nextProps.messageGroups.length) return false; if (prevProps.messageGroups.length !== nextProps.messageGroups.length) return false;
for (let i = 0; i < prevProps.messageGroups.length; i++) { for (let i = 0; i < prevProps.messageGroups.length; i++) {
const prevGroup = prevProps.messageGroups[i]; const prevGroup = prevProps.messageGroups[i];
@@ -69,6 +73,7 @@ export const BlockedMessageGroups = React.memo<BlockedMessageGroupsProps>((props
channel, channel,
compact, compact,
revealed, revealed,
hasUnread = false,
messageGroupSpacing, messageGroupSpacing,
onReveal, onReveal,
variant, variant,
@@ -79,6 +84,7 @@ export const BlockedMessageGroups = React.memo<BlockedMessageGroupsProps>((props
messageActionsClassName, messageActionsClassName,
renderMessageActions, renderMessageActions,
renderMessageWrapper, renderMessageWrapper,
suppressUnreadIndicator,
} = props; } = props;
const containerRef = useRef<HTMLDivElement>(null); const containerRef = useRef<HTMLDivElement>(null);
const scrollToBottomFrameRef = useRef<number | null>(null); const scrollToBottomFrameRef = useRef<number | null>(null);
@@ -169,6 +175,12 @@ export const BlockedMessageGroups = React.memo<BlockedMessageGroupsProps>((props
}; };
messageGroups.forEach((item, itemIndex) => { messageGroups.forEach((item, itemIndex) => {
if (item.type === ChannelStreamType.DIVIDER) { if (item.type === ChannelStreamType.DIVIDER) {
if (item.unreadId && suppressUnreadIndicator) {
return;
}
if (itemIndex === 0 && item.unreadId) {
return;
}
flushGroup(); flushGroup();
nodes.push( nodes.push(
<Divider <Divider
@@ -205,13 +217,23 @@ export const BlockedMessageGroups = React.memo<BlockedMessageGroupsProps>((props
messageActionsClassName, messageActionsClassName,
renderMessageActions, renderMessageActions,
renderMessageWrapper, renderMessageWrapper,
suppressUnreadIndicator,
]); ]);
const leadingUnreadDivider = messageGroups[0]?.type === ChannelStreamType.DIVIDER && !!messageGroups[0].unreadId;
return ( return (
<div <div
ref={containerRef} ref={containerRef}
className={clsx(styles.container, className)} className={clsx(styles.container, className)}
data-flx="channel.blocked-message-groups.container" data-flx="channel.blocked-message-groups.container"
> >
{hasUnread && (!revealed || leadingUnreadDivider) && (
<Divider
spacing={messageGroupSpacing}
red={true}
id="new-messages-bar"
data-flx="channel.blocked-message-groups.collapsed-unread-divider"
/>
)}
<button <button
type="button" type="button"
className={styles.toggle} className={styles.toggle}
@@ -189,11 +189,13 @@ export function renderChannelStream(props: RenderChannelStreamProps): Array<Reac
key={item.key} key={item.key}
revealed={item.key === unblurredMessageId} revealed={item.key === unblurredMessageId}
messageGroups={item.content as Array<ChannelStreamItem>} messageGroups={item.content as Array<ChannelStreamItem>}
hasUnread={item.hasUnread === true && !suppressUnreadIndicator}
onReveal={onReveal ?? (() => {})} onReveal={onReveal ?? (() => {})}
compact={messageDisplayCompact} compact={messageDisplayCompact}
channel={channel} channel={channel}
messageGroupSpacing={messageGroupSpacing} messageGroupSpacing={messageGroupSpacing}
variant={variant} variant={variant}
suppressUnreadIndicator={suppressUnreadIndicator}
data-flx="channel.channel-message-stream.render-channel-stream.blocked-message-groups" data-flx="channel.channel-message-stream.render-channel-stream.blocked-message-groups"
/>, />,
); );
@@ -236,7 +236,7 @@ export const Messages = observer(function Messages({
messages: safeMessages, messages: safeMessages,
channel, channel,
compact: state.messageDisplayCompact, compact: state.messageDisplayCompact,
hasPendingUnreads: state.unreadCount > 0, hasPendingUnreads: state.visualUnreadMessageId != null,
focusAnchorId: null, focusAnchorId: null,
unloadedSpacerHeight: selectChannelMessagesSpacerHeight(windowStatus, placeholderSpecs.totalHeight), unloadedSpacerHeight: selectChannelMessagesSpacerHeight(windowStatus, placeholderSpecs.totalHeight),
allowHistoryFetch: true, allowHistoryFetch: true,
@@ -0,0 +1,137 @@
// SPDX-License-Identifier: AGPL-3.0-or-later
import type {Channel} from '@app/features/channel/models/Channel';
import type {Message} from '@app/features/messaging/models/MessagingMessage';
import type {ChannelMessages} from '@app/features/messaging/state/ChannelMessages';
import {ChannelStreamType, createChannelStream} from '@app/features/messaging/utils/MessageGroupingUtils';
import {describe, expect, it, vi} from 'vitest';
vi.mock('@app/features/auth/state/Authentication', () => ({default: {currentUserId: 'me'}}));
vi.mock('@app/features/moderation/state/LocalUserSpamOverride', () => ({
default: {isUserMarkedAsSpammer: () => false},
}));
vi.mock('@app/features/user/utils/DateFormatting', () => ({
getFormattedFullDate: (date: Date) => date.toISOString().slice(0, 10),
}));
const AUTHOR = {id: 'author-1', flags: 0} as Message['author'];
const ID = {
first: '1519773906704011264',
second: '1519773906708205568',
third: '1519773906712399872',
fourth: '1519773906716594176',
} as const;
function message(id: string, minute: number, blocked = false): Message {
return {
id,
author: AUTHOR,
blocked,
ignored: false,
timestamp: new Date(Date.UTC(2026, 7, 27, 12, minute)),
type: 0,
webhookId: null,
mentions: [],
mentionRoles: [],
mentionEveryone: false,
isUserMessage: () => true,
hasFlag: () => false,
} as unknown as Message;
}
function channelMessages(messages: Array<Message>): ChannelMessages {
return {
forEach: (callback: (message: Message) => boolean | undefined) => {
for (const entry of messages) {
if (callback(entry) === false) return;
}
},
jumpDestinationId: null,
jumpHighlight: false,
jumpTicket: null,
} as unknown as ChannelMessages;
}
const channel = {id: 'channel-1', isPrivate: () => false, getGuildId: () => 'guild-1'} as unknown as Channel;
function buildStream(messages: Array<Message>, oldestUnreadMessageId: string | null) {
return createChannelStream({
channel,
messages: channelMessages(messages),
oldestUnreadMessageId,
treatSpam: false,
});
}
function countUnreadMarkers(stream: Array<ReturnType<typeof buildStream>[number]>): number {
let count = 0;
for (const item of stream) {
if (item.type === ChannelStreamType.DIVIDER && item.unreadId != null) count++;
if (item.type === ChannelStreamType.MESSAGE && item.showUnreadDividerBefore) count++;
if (Array.isArray(item.content)) {
count += countUnreadMarkers(item.content as Array<ReturnType<typeof buildStream>[number]>);
}
}
return count;
}
function findUnreadDivider(stream: Array<ReturnType<typeof buildStream>[number]>) {
for (const item of stream) {
if (item.type === ChannelStreamType.DIVIDER && item.unreadId != null) return item;
if (item.type === ChannelStreamType.MESSAGE && item.showUnreadDividerBefore) return item;
if (item.type === ChannelStreamType.MESSAGE_GROUP_BLOCKED) {
const nested = item.content as Array<ReturnType<typeof buildStream>[number]>;
const divider = nested.find((entry) => entry.type === ChannelStreamType.DIVIDER && entry.unreadId != null);
if (divider) return divider;
}
}
return null;
}
describe('createChannelStream unread divider', () => {
it('marks the unread boundary on a plain message', () => {
const messages = [message(ID.first, 0), message(ID.second, 1), message(ID.third, 2)];
expect(findUnreadDivider(buildStream(messages, ID.second))).not.toBeNull();
});
it('emits a divider inside the collapsed group when the boundary is a blocked message', () => {
const messages = [message(ID.first, 0), message(ID.second, 1, true), message(ID.third, 2, true)];
const stream = buildStream(messages, ID.second);
const group = stream.find((item) => item.type === ChannelStreamType.MESSAGE_GROUP_BLOCKED);
expect(group).toBeDefined();
const nested = group!.content as Array<(typeof stream)[number]>;
expect(nested.some((item) => item.type === ChannelStreamType.DIVIDER && item.unreadId === ID.second)).toBe(true);
expect(group!.hasUnread).toBe(true);
});
it('emits a divider inside the collapsed group when the stored boundary is no longer loaded', () => {
const messages = [message(ID.third, 0, true), message(ID.fourth, 1, true)];
const stream = buildStream(messages, ID.first);
const group = stream.find((item) => item.type === ChannelStreamType.MESSAGE_GROUP_BLOCKED);
expect(group).toBeDefined();
const nested = group!.content as Array<(typeof stream)[number]>;
expect(nested.some((item) => item.type === ChannelStreamType.DIVIDER && item.unreadId === ID.third)).toBe(true);
});
it('never leaves the boundary unrepresented when there is an unread message id', () => {
const messages = [message(ID.first, 0), message(ID.second, 1, true), message(ID.third, 2)];
expect(findUnreadDivider(buildStream(messages, ID.second))).not.toBeNull();
});
it('merges the boundary into the date divider when it is the first message of a day', () => {
const dayOne = message(ID.first, 0);
const dayTwo = message(ID.third, 1);
(dayTwo as {timestamp: Date}).timestamp = new Date(Date.UTC(2026, 7, 28, 9, 0));
const stream = buildStream([dayOne, dayTwo], ID.third);
const dateDivider = stream.find((item) => item.type === ChannelStreamType.DIVIDER && item.unreadId === ID.third);
expect(dateDivider).toBeDefined();
expect(countUnreadMarkers(stream)).toBe(1);
});
it('marks the boundary exactly once', () => {
const messages = [message(ID.first, 0), message(ID.second, 1, true), message(ID.third, 2)];
expect(countUnreadMarkers(buildStream(messages, ID.second))).toBe(1);
expect(countUnreadMarkers(buildStream([message(ID.first, 0), message(ID.second, 1)], ID.second))).toBe(1);
});
});
@@ -197,18 +197,22 @@ export function createChannelStream(props: {
} }
} }
let shouldShowUnreadDividerBefore = false; let shouldShowUnreadDividerBefore = false;
let shouldOpenCollapsedGroupWithUnreadDivider = false;
if (oldestUnreadMessageId === message.id && unreadTimestamp != null) { if (oldestUnreadMessageId === message.id && unreadTimestamp != null) {
if (lastItem?.type === ChannelStreamType.DIVIDER) { if (lastItem?.type === ChannelStreamType.DIVIDER) {
lastItem.unreadId = message.id; lastItem.unreadId = message.id;
} else if (collapsedGroupItem !== null) {
shouldOpenCollapsedGroupWithUnreadDivider = true;
} else { } else {
shouldShowUnreadDividerBefore = true; shouldShowUnreadDividerBefore = true;
if (collapsedGroupItem !== null) {
collapsedGroupItem.hasUnread = true;
}
} }
unreadTimestamp = null; unreadTimestamp = null;
} else if (unreadTimestamp != null && extractTimestamp(message.id) > unreadTimestamp) { } else if (unreadTimestamp != null && extractTimestamp(message.id) > unreadTimestamp) {
if (collapsedGroupItem !== null) {
shouldOpenCollapsedGroupWithUnreadDivider = true;
} else {
shouldShowUnreadDividerBefore = true; shouldShowUnreadDividerBefore = true;
}
unreadTimestamp = null; unreadTimestamp = null;
} }
let prevMessageForGrouping: Message | undefined; let prevMessageForGrouping: Message | undefined;
@@ -242,7 +246,12 @@ export function createChannelStream(props: {
messageItem.jumpTarget = true; messageItem.jumpTarget = true;
} }
if (collapsedGroupItem !== null) { if (collapsedGroupItem !== null) {
(collapsedGroupItem.content as Array<ChannelStreamItem>).push(messageItem); const collapsedContentItems = collapsedGroupItem.content as Array<ChannelStreamItem>;
if (shouldOpenCollapsedGroupWithUnreadDivider) {
collapsedContentItems.push({type: ChannelStreamType.DIVIDER, content: '', unreadId: message.id});
collapsedGroupItem.hasUnread = true;
}
collapsedContentItems.push(messageItem);
if (messageItem.jumpTarget) { if (messageItem.jumpTarget) {
collapsedGroupItem.hasJumpTarget = true; collapsedGroupItem.hasJumpTarget = true;
} }
@@ -1,12 +1,15 @@
// SPDX-License-Identifier: AGPL-3.0-or-later // SPDX-License-Identifier: AGPL-3.0-or-later
import {ChannelTypes} from '@fluxer/constants/src/ChannelConstants';
import {JumpTypes} from '@fluxer/constants/src/JumpConstants'; import {JumpTypes} from '@fluxer/constants/src/JumpConstants';
import {NEW_MESSAGES_BAR_BUFFER} from '@fluxer/constants/src/LimitConstants'; import {NEW_MESSAGES_BAR_BUFFER} from '@fluxer/constants/src/LimitConstants';
import {describe, expect, it} from 'vitest'; import {describe, expect, it} from 'vitest';
import { import {
CENTRE_ALIGNMENT_LIFT, CENTRE_ALIGNMENT_LIFT,
InitialScrollIntent,
MESSAGE_REVEAL_PADDING, MESSAGE_REVEAL_PADDING,
resolveContainerResizeShift, resolveContainerResizeShift,
resolveInitialScrollIntent,
type ScrollerState, type ScrollerState,
shouldAnimateMessageJump, shouldAnimateMessageJump,
} from './shared'; } from './shared';
@@ -83,3 +86,42 @@ describe('resolveContainerResizeShift', () => {
expect(resolve(10, state(1400, 400, 2000))).toEqual({kind: 'none'}); expect(resolve(10, state(1400, 400, 2000))).toEqual({kind: 'none'});
}); });
}); });
describe('resolveInitialScrollIntent', () => {
it('anchors to the unread boundary only when the divider is expected on screen', () => {
expect(
resolveInitialScrollIntent({
channelType: ChannelTypes.GUILD_TEXT,
rememberedScrollTop: 900,
hasPendingUnreads: true,
}),
).toBe(InitialScrollIntent.UNREAD_BOUNDARY);
expect(
resolveInitialScrollIntent({
channelType: ChannelTypes.GUILD_TEXT,
rememberedScrollTop: 900,
hasPendingUnreads: false,
}),
).toBe(InitialScrollIntent.SAVED_OFFSET);
});
it('falls back to the bottom rather than the top when there is no unread boundary and no saved offset', () => {
expect(
resolveInitialScrollIntent({
channelType: ChannelTypes.GUILD_TEXT,
rememberedScrollTop: null,
hasPendingUnreads: false,
}),
).toBe(InitialScrollIntent.BOTTOM);
});
it('never anchors to the unread boundary in voice channels', () => {
expect(
resolveInitialScrollIntent({
channelType: ChannelTypes.GUILD_VOICE,
rememberedScrollTop: null,
hasPendingUnreads: true,
}),
).toBe(InitialScrollIntent.BOTTOM);
});
});
@@ -457,6 +457,9 @@ class ReadStates {
this.refreshUnreadEstimate(state); this.refreshUnreadEstimate(state);
} else if (action.isAfter && state.ackMessageId != null && messages.has(state.ackMessageId, true)) { } else if (action.isAfter && state.ackMessageId != null && messages.has(state.ackMessageId, true)) {
state.unreadCount += action.messages.length; state.unreadCount += action.messages.length;
if (state.oldestUnreadMessageId == null) {
state.rebuild();
}
} }
this.notifyChange(action.channelId); this.notifyChange(action.channelId);
} }
@@ -533,6 +536,7 @@ class ReadStates {
} }
handleMessageDelete(action: {channelId: string}): void { handleMessageDelete(action: {channelId: string}): void {
this.getIfExists(action.channelId)?.rebuild();
this.notifyChange(action.channelId); this.notifyChange(action.channelId);
} }
@@ -280,7 +280,7 @@ export class ReadStateEntry {
} else { } else {
this.unreadCount = loadedUnreadCount; this.unreadCount = loadedUnreadCount;
} }
this.oldestUnreadMessageId = hasUnreadBoundary ? (this.storedOldestUnreadMessageId ?? oldestUnread) : null; this.oldestUnreadMessageId = this.storedOldestUnreadMessageId ?? oldestUnread;
} }
shouldMentionFor(message: MessageModel | WireMessage, userId: string, isPrivate: boolean): boolean { shouldMentionFor(message: MessageModel | WireMessage, userId: string, isPrivate: boolean): boolean {