,
document.body,
@@ -411,7 +399,7 @@ const MobileQuotePreview = ({
{showTrailingBreak &&
+
,
document.body,
diff --git a/src/lib/utils/__tests__/thread-scroll-utils.test.ts b/src/lib/utils/__tests__/thread-scroll-utils.test.ts
index df4ff238..42cc9b3f 100644
--- a/src/lib/utils/__tests__/thread-scroll-utils.test.ts
+++ b/src/lib/utils/__tests__/thread-scroll-utils.test.ts
@@ -1,9 +1,12 @@
-import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
-import { clearThreadScrollSpacer, openThreadAtTop } from '../thread-scroll-utils';
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+import { getRequestedThreadTopCid, getThreadTopNavigationState, scrollThreadContainerToTop } from '../thread-scroll-utils';
-const appendThreadContainer = (cid: string, top = 240) => {
+const appendThreadContainer = ({ cid, top = 240, hidden = false, parent = document.body }: { cid: string; top?: number; hidden?: boolean; parent?: HTMLElement }) => {
const element = document.createElement('div');
element.dataset.threadContainerCid = cid;
+ if (hidden) {
+ element.style.display = 'none';
+ }
element.getBoundingClientRect = () =>
({
bottom: top + 100,
@@ -13,110 +16,87 @@ const appendThreadContainer = (cid: string, top = 240) => {
top,
width: 100,
}) as DOMRect;
- document.body.appendChild(element);
+ parent.appendChild(element);
return element;
};
describe('thread-scroll-utils', () => {
let scrollToMock: ReturnType
;
- let requestAnimationFrameMock: ReturnType;
beforeEach(() => {
vi.clearAllMocks();
scrollToMock = vi.fn();
- requestAnimationFrameMock = vi.fn((callback: FrameRequestCallback) => {
- callback(0);
- return 1;
- });
Object.defineProperty(window, 'scrollTo', {
configurable: true,
value: scrollToMock,
writable: true,
});
- Object.defineProperty(window, 'requestAnimationFrame', {
- configurable: true,
- value: requestAnimationFrameMock,
- writable: true,
- });
Object.defineProperty(window, 'scrollY', {
configurable: true,
- value: 0,
+ value: 32,
writable: true,
});
- Object.defineProperty(window, 'innerHeight', {
- configurable: true,
- value: 100,
- writable: true,
- });
- Object.defineProperty(document.documentElement, 'scrollHeight', {
- configurable: true,
- value: 200,
- writable: true,
- });
- });
- afterEach(() => {
- clearThreadScrollSpacer();
document.body.innerHTML = '';
});
- it('scrolls the current thread without pushing duplicate history entries', () => {
- appendThreadContainer('thread-cid');
- const navigateMock = vi.fn();
+ it('scrolls the matching thread container with a plain window scroll', () => {
+ appendThreadContainer({ cid: 'thread-cid', top: 240 });
- expect(
- openThreadAtTop({
- cid: 'thread-cid',
- currentPathname: '/mu/thread/thread-cid',
- navigate: navigateMock,
- threadRoute: '/mu/thread/thread-cid',
- }),
- ).toBe(true);
+ expect(scrollThreadContainerToTop('thread-cid')).toBe(true);
- expect(navigateMock).not.toHaveBeenCalled();
- expect(requestAnimationFrameMock).toHaveBeenCalledOnce();
- expect(scrollToMock).toHaveBeenCalledTimes(2);
- expect(scrollToMock).toHaveBeenNthCalledWith(1, {
+ expect(scrollToMock).toHaveBeenCalledOnce();
+ expect(scrollToMock).toHaveBeenCalledWith({
behavior: 'auto',
left: 0,
- top: 240,
+ top: 272,
});
});
- it('navigates to the OP thread before scrolling when the route differs', () => {
- appendThreadContainer('thread-cid');
- const navigateMock = vi.fn();
+ it('ignores preview copies when resolving the scroll target', () => {
+ const previewWrapper = document.createElement('div');
+ previewWrapper.dataset.threadScrollPreview = 'true';
+ document.body.appendChild(previewWrapper);
+ appendThreadContainer({ cid: 'thread-cid', top: 12, parent: previewWrapper });
+ appendThreadContainer({ cid: 'thread-cid', top: 180 });
- expect(
- openThreadAtTop({
- cid: 'thread-cid',
- currentPathname: '/mu/thread/reply-cid',
- navigate: navigateMock,
- threadRoute: '/mu/thread/thread-cid',
- }),
- ).toBe(true);
+ expect(scrollThreadContainerToTop('thread-cid')).toBe(true);
- expect(navigateMock).toHaveBeenCalledWith('/mu/thread/thread-cid');
- expect(requestAnimationFrameMock).toHaveBeenCalledOnce();
- expect(scrollToMock).toHaveBeenCalledTimes(2);
+ expect(scrollToMock).toHaveBeenCalledWith({
+ behavior: 'auto',
+ left: 0,
+ top: 212,
+ });
});
- it('returns false when the permalink cannot resolve a thread target', () => {
- const navigateMock = vi.fn();
+ it('prefers the visible thread container when cached duplicates are hidden', () => {
+ appendThreadContainer({ cid: 'thread-cid', top: 12, hidden: true });
+ appendThreadContainer({ cid: 'thread-cid', top: 180 });
- expect(
- openThreadAtTop({
- cid: undefined,
- currentPathname: '/mu/thread/thread-cid',
- navigate: navigateMock,
- threadRoute: '/mu/thread/thread-cid',
- }),
- ).toBe(false);
+ expect(scrollThreadContainerToTop('thread-cid')).toBe(true);
- expect(navigateMock).not.toHaveBeenCalled();
- expect(requestAnimationFrameMock).not.toHaveBeenCalled();
+ expect(scrollToMock).toHaveBeenCalledWith({
+ behavior: 'auto',
+ left: 0,
+ top: 212,
+ });
+ });
+
+ it('returns false when no thread container exists', () => {
+ expect(scrollThreadContainerToTop('missing-cid')).toBe(false);
expect(scrollToMock).not.toHaveBeenCalled();
});
+
+ it('serializes and reads thread-top navigation state', () => {
+ expect(getThreadTopNavigationState()).toBeUndefined();
+ expect(getThreadTopNavigationState('thread-cid')).toEqual({
+ scrollThreadContainerCid: 'thread-cid',
+ });
+ expect(getRequestedThreadTopCid(getThreadTopNavigationState())).toBeUndefined();
+ expect(getRequestedThreadTopCid({ scrollThreadContainerCid: 'thread-cid' })).toBe('thread-cid');
+ expect(getRequestedThreadTopCid({ scrollThreadContainerCid: 42 })).toBeUndefined();
+ expect(getRequestedThreadTopCid(null)).toBeUndefined();
+ });
});
diff --git a/src/lib/utils/thread-scroll-utils.ts b/src/lib/utils/thread-scroll-utils.ts
index c04a23ea..b048d020 100644
--- a/src/lib/utils/thread-scroll-utils.ts
+++ b/src/lib/utils/thread-scroll-utils.ts
@@ -1,43 +1,42 @@
-const THREAD_SCROLL_SPACER_ID = 'thread-scroll-spacer';
+const THREAD_SCROLL_PREVIEW_SELECTOR = '[data-thread-scroll-preview="true"]';
-const setThreadScrollSpacerHeight = (height: number) => {
- const existingSpacer = document.getElementById(THREAD_SCROLL_SPACER_ID);
- if (height <= 0) {
- existingSpacer?.remove();
- return;
- }
-
- const spacer =
- existingSpacer ||
- Object.assign(document.createElement('div'), {
- id: THREAD_SCROLL_SPACER_ID,
- });
-
- spacer.setAttribute('aria-hidden', 'true');
- spacer.style.height = `${Math.ceil(height)}px`;
- spacer.style.pointerEvents = 'none';
- spacer.style.opacity = '0';
-
- if (!existingSpacer) {
- document.body.appendChild(spacer);
- }
+type ThreadTopNavigationState = {
+ scrollThreadContainerCid?: string;
};
-export const clearThreadScrollSpacer = () => {
- document.getElementById(THREAD_SCROLL_SPACER_ID)?.remove();
+export const getThreadTopNavigationState = (cid?: string): ThreadTopNavigationState | undefined =>
+ cid
+ ? {
+ scrollThreadContainerCid: cid,
+ }
+ : undefined;
+
+export const getRequestedThreadTopCid = (state: unknown) => {
+ if (!state || typeof state !== 'object') return undefined;
+
+ const cid = (state as ThreadTopNavigationState).scrollThreadContainerCid;
+ return typeof cid === 'string' ? cid : undefined;
+};
+
+const isVisibleScrollTarget = (element: HTMLElement) => {
+ const style = window.getComputedStyle(element);
+ const rect = element.getBoundingClientRect();
+
+ return style.display !== 'none' && style.visibility !== 'hidden' && style.opacity !== '0' && rect.width > 0 && rect.height > 0;
+};
+
+export const findPreferredScrollTarget = (selector: string, excludedAncestorSelector = THREAD_SCROLL_PREVIEW_SELECTOR) => {
+ const candidates = Array.from(document.querySelectorAll(selector)).filter((element) => !element.closest(excludedAncestorSelector));
+ return candidates.find(isVisibleScrollTarget) ?? candidates[0];
};
export const scrollThreadContainerToTop = (cid?: string) => {
if (!cid) return false;
- const threadContainer = document.querySelector(`[data-thread-container-cid="${cid}"]`);
+ const threadContainer = findPreferredScrollTarget(`[data-thread-container-cid="${cid}"]`);
if (!threadContainer) return false;
const desiredTop = window.scrollY + threadContainer.getBoundingClientRect().top;
- const maxScrollTop = document.documentElement.scrollHeight - window.innerHeight;
- const extraSpace = Math.max(0, desiredTop - maxScrollTop);
-
- setThreadScrollSpacerHeight(extraSpace);
window.scrollTo({
top: desiredTop,
left: 0,
@@ -46,28 +45,3 @@ export const scrollThreadContainerToTop = (cid?: string) => {
return true;
};
-
-export const openThreadAtTop = ({
- cid,
- currentPathname,
- navigate,
- threadRoute,
-}: {
- cid?: string;
- currentPathname?: string;
- navigate: (route: string) => void;
- threadRoute?: string;
-}) => {
- if (!cid || !threadRoute) return false;
-
- if (currentPathname !== threadRoute) {
- navigate(threadRoute);
- }
-
- scrollThreadContainerToTop(cid);
- window.requestAnimationFrame(() => {
- scrollThreadContainerToTop(cid);
- });
-
- return true;
-};
diff --git a/src/views/post/__tests__/post.test.tsx b/src/views/post/__tests__/post.test.tsx
index 9566be33..8345dc1e 100644
--- a/src/views/post/__tests__/post.test.tsx
+++ b/src/views/post/__tests__/post.test.tsx
@@ -160,12 +160,12 @@ const flushEffects = async (count = 5) => {
}
};
-const renderPostPage = async (initialEntry: string) => {
+const renderPostPage = async (initialEntry: string | { pathname: string; state?: unknown }) => {
await act(async () => {
root.render(
createElement(
MemoryRouter,
- { initialEntries: [initialEntry] },
+ { initialEntries: [initialEntry as any] },
createElement(
Routes,
{},
@@ -202,11 +202,41 @@ describe('Post', () => {
value: vi.fn(),
writable: true,
});
+ Object.defineProperty(window, 'scrollY', {
+ configurable: true,
+ value: 0,
+ writable: true,
+ });
Object.defineProperty(HTMLElement.prototype, 'scrollIntoView', {
configurable: true,
value: vi.fn(),
writable: true,
});
+ Object.defineProperty(HTMLElement.prototype, 'getBoundingClientRect', {
+ configurable: true,
+ value: function () {
+ if ((this as HTMLElement).dataset.threadContainerCid) {
+ return {
+ bottom: 220,
+ height: 100,
+ left: 0,
+ right: 100,
+ top: 120,
+ width: 100,
+ } as DOMRect;
+ }
+
+ return {
+ bottom: 0,
+ height: 0,
+ left: 0,
+ right: 0,
+ top: 0,
+ width: 0,
+ } as DOMRect;
+ },
+ writable: true,
+ });
document.title = 'before';
container = document.createElement('div');
@@ -261,12 +291,34 @@ describe('Post', () => {
expect(container.querySelector('[data-testid="thread-footer-first-row"]')?.textContent).toBe('cached-cid:42:music-posting.eth:false');
expect(container.querySelector('[data-testid="thread-footer-mobile"]')?.textContent).toBe('cached-cid:42:music-posting.eth:false');
expect(document.title).toBe('/mu/ - Cached thread... - 5chan');
+ expect(window.scrollTo).toHaveBeenCalledWith(0, 0);
+ expect(HTMLElement.prototype.scrollIntoView).not.toHaveBeenCalled();
+ });
+
+ it('only aligns the OP container when navigation explicitly requests it', async () => {
+ testState.commentsByCid = {
+ 'thread-cid': {
+ cid: 'thread-cid',
+ number: 8,
+ replyCount: 0,
+ subplebbitAddress: 'music-posting.eth',
+ title: 'Thread title',
+ },
+ };
+
+ await renderPostPage({
+ pathname: '/mu/thread/thread-cid',
+ state: {
+ scrollThreadContainerCid: 'thread-cid',
+ },
+ });
+
expect(window.scrollTo).toHaveBeenCalledWith({
behavior: 'auto',
left: 0,
- top: 0,
+ top: 120,
});
- expect(HTMLElement.prototype.scrollIntoView).not.toHaveBeenCalled();
+ expect(window.scrollTo).not.toHaveBeenCalledWith(0, 0);
});
it('redirects thread routes whose fetched comment belongs to a different board', async () => {
diff --git a/src/views/post/post.tsx b/src/views/post/post.tsx
index 98bffbc9..b9364003 100644
--- a/src/views/post/post.tsx
+++ b/src/views/post/post.tsx
@@ -1,4 +1,4 @@
-import { memo, useEffect, useMemo } from 'react';
+import { memo, useEffect, useMemo, useRef } from 'react';
import { useTranslation } from 'react-i18next';
import { Comment, Role, useComment, useEditedComment, useSubplebbit } from '@bitsocialnet/bitsocial-react-hooks';
import useSubplebbitsPagesStore from '@bitsocialnet/bitsocial-react-hooks/dist/stores/subplebbits-pages';
@@ -13,7 +13,7 @@ import ErrorDisplay from '../../components/error-display/error-display';
import { PageFooterDesktop, ThreadFooterFirstRow, ThreadFooterStyleRow, ThreadFooterMobile } from '../../components/footer';
import PostDesktop from '../../components/post-desktop';
import PostMobile from '../../components/post-mobile';
-import { clearThreadScrollSpacer, scrollThreadContainerToTop } from '../../lib/utils/thread-scroll-utils';
+import { getRequestedThreadTopCid, scrollThreadContainerToTop } from '../../lib/utils/thread-scroll-utils';
import styles from './post.module.css';
// useComment may not return cached feed data immediately due to its updatedAt comparison logic.
@@ -134,6 +134,7 @@ const PostPage = () => {
const isInAllView = isAllView(location.pathname);
const comment = useCommentWithFeedCache({ commentCid });
+ const consumedThreadTopScrollRef = useRef(null);
const navigate = useNavigate();
useEffect(() => {
@@ -154,23 +155,32 @@ const PostPage = () => {
} else {
post = comment;
}
+ const requestedThreadTopCid = getRequestedThreadTopCid(location.state);
const { error } = post || {};
- useEffect(() => () => clearThreadScrollSpacer(), []);
-
+ // These two effects split normal opens from explicit OP-top intents:
+ // the first keeps ordinary thread visits on `window.scrollTo(0, 0)`, while the
+ // second consumes `requestedThreadTopCid` once per `location.key` via
+ // `consumedThreadTopScrollRef` so `scrollThreadContainerToTop(commentCid)` only
+ // replays for deliberate OP-link clicks and never for route-driven thread opens.
useEffect(() => {
- if (!commentCid || post?.cid === commentCid) return;
- clearThreadScrollSpacer();
- }, [commentCid, post?.cid]);
+ if (!comment?.cid || comment.parentCid) return;
+ if (requestedThreadTopCid === comment.cid) return;
+ window.scrollTo(0, 0);
+ }, [comment?.cid, comment?.parentCid, requestedThreadTopCid]);
useEffect(() => {
if (!commentCid || post?.cid !== commentCid) return;
+ if (requestedThreadTopCid !== commentCid) return;
+
+ const consumedKey = `${location.key}:${commentCid}`;
+ if (consumedThreadTopScrollRef.current === consumedKey) return;
+
if (scrollThreadContainerToTop(commentCid)) {
- return;
+ consumedThreadTopScrollRef.current = consumedKey;
}
- window.scrollTo(0, 0);
- }, [commentCid, post?.cid]);
+ }, [commentCid, location.key, post?.cid, requestedThreadTopCid]);
useEffect(() => {
const boardIdentifier = params.boardIdentifier;