From a8dd527c457c32ef6c35dd7e7484736b51d60f85 Mon Sep 17 00:00:00 2001 From: Dmitry Verkhoturov Date: Thu, 9 Jul 2026 22:03:12 +0100 Subject: [PATCH] Fix comments iframe collapsing to preloader height on load On mount ConnectedRoot immediately reported the iframe height to the parent page while the app was still showing the global preloader, so the parent shrank the iframe from its initial size to ~63px and then grew it back step by step as content rendered. On pages with many comments this reads as the widget blinking several times before loading (reported for radio-t.com). The June frontend dependency refresh (#2091) shifted render/effect timing enough to make the premature measurement happen on every load rather than only on slow connections. Move the height reporting into Root and start it in the setState callback that replaces the preloader with real content: the first height message now always describes rendered content, the iframe never shrinks below it, and subsequent ResizeObserver updates only grow the frame as comments arrive. Also adds the previously missing observer disconnect on unmount. Verified by instrumenting the embed with a height-message listener: master sent 63px then 316px on an empty test page (v1.16.1 sent a single 316px); with this fix the first message is 316px again. --- .../app/components/root/root.spec.tsx | 74 +++++++++++++++++++ .../remark42/app/components/root/root.tsx | 39 +++++++--- 2 files changed, 103 insertions(+), 10 deletions(-) create mode 100644 frontend/apps/remark42/app/components/root/root.spec.tsx diff --git a/frontend/apps/remark42/app/components/root/root.spec.tsx b/frontend/apps/remark42/app/components/root/root.spec.tsx new file mode 100644 index 00000000..221b0532 --- /dev/null +++ b/frontend/apps/remark42/app/components/root/root.spec.tsx @@ -0,0 +1,74 @@ +import '@testing-library/jest-dom'; +import { waitFor } from '@testing-library/preact'; + +import { render } from 'tests/utils'; +import * as api from 'common/api'; +import * as postMessage from 'utils/post-message'; +import type { User } from 'common/types'; +import type { StoreState } from 'store'; + +import { ConnectedRoot } from './root'; + +const stateStub: Partial = { + comments: { + sort: '-active', + isFetching: false, + childComments: {}, + topComments: [], + pinnedComments: [], + allComments: {}, + activeComment: null, + }, + collapsedThreads: {}, + theme: 'light', + info: { url: 'test-url', count: 0, read_only: false }, + hiddenUsers: {}, + bannedUsers: [], + user: null, +}; + +describe('', () => { + afterEach(() => { + jest.restoreAllMocks(); + }); + + it('reports iframe height only after the initial user fetch settles', async () => { + let resolveUser!: (user: User | null) => void; + jest.spyOn(api, 'getUser').mockImplementation( + () => + new Promise((resolve) => { + resolveUser = resolve; + }) + ); + // keep comments loading so only the user fetch controls the first height report + jest.spyOn(api, 'getPostComments').mockImplementation(() => new Promise(() => undefined)); + const updateIframeHeight = jest.spyOn(postMessage, 'updateIframeHeight').mockImplementation(() => undefined); + + render(, stateStub); + + // while the global preloader is shown, no height must be sent to the parent page, + // otherwise the parent shrinks the iframe to the preloader size and it blinks + expect(updateIframeHeight).not.toHaveBeenCalled(); + + resolveUser(null); + + await waitFor(() => expect(updateIframeHeight).toHaveBeenCalled()); + }); + + it('falls back to reporting iframe height when the user fetch hangs', () => { + jest.useFakeTimers(); + try { + jest.spyOn(api, 'getUser').mockImplementation(() => new Promise(() => undefined)); + jest.spyOn(api, 'getPostComments').mockImplementation(() => new Promise(() => undefined)); + const updateIframeHeight = jest.spyOn(postMessage, 'updateIframeHeight').mockImplementation(() => undefined); + + render(, stateStub); + expect(updateIframeHeight).not.toHaveBeenCalled(); + + jest.advanceTimersByTime(5000); + expect(updateIframeHeight).toHaveBeenCalled(); + } finally { + jest.useRealTimers(); + } + }); +}); diff --git a/frontend/apps/remark42/app/components/root/root.tsx b/frontend/apps/remark42/app/components/root/root.tsx index bf07f854..5bb065d0 100644 --- a/frontend/apps/remark42/app/components/root/root.tsx +++ b/frontend/apps/remark42/app/components/root/root.tsx @@ -1,5 +1,4 @@ import { h, Component, Fragment } from 'preact'; -import { useEffect } from 'preact/hooks'; import { useSelector } from 'react-redux'; import { IntlShape, useIntl, FormattedMessage, defineMessages } from 'react-intl'; import clsx from 'clsx'; @@ -112,8 +111,29 @@ export class Root extends Component { isSettingsVisible: false, }; + heightObserver: ResizeObserver | null = null; + heightFallbackTimeout: number | null = null; + + startHeightReporting = () => { + if (this.heightObserver) { + return; + } + + updateIframeHeight(); + this.heightObserver = new ResizeObserver(() => updateIframeHeight()); + this.heightObserver.observe(document.body); + }; + componentDidMount() { - const userloading = this.props.fetchUser().finally(() => this.setState({ isUserLoading: false })); + // if the user fetch hangs, report the preloader height anyway so the parent page + // is not left with an unbounded iframe + this.heightFallbackTimeout = window.setTimeout(this.startHeightReporting, 5000); + + const userloading = this.props.fetchUser().finally(() => + // start reporting iframe height only after the global preloader is replaced with + // real content, so the parent page never shrinks the iframe to the preloader size + this.setState({ isUserLoading: false }, this.startHeightReporting) + ); Promise.all([userloading, this.props.fetchComments()]).finally(() => { setTimeout(this.checkUrlHash); @@ -123,6 +143,13 @@ export class Root extends Component { window.addEventListener('message', this.onMessage); } + componentWillUnmount() { + if (this.heightFallbackTimeout !== null) { + window.clearTimeout(this.heightFallbackTimeout); + } + this.heightObserver?.disconnect(); + } + checkUrlHash = (e: Event & { newURL: string }) => { const hash = e ? `#${e.newURL.split('#')[1]}` : window.location.hash; @@ -325,14 +352,6 @@ export function ConnectedRoot() { const props = useSelector(mapStateToProps); const actions = useActions(boundActions); - useEffect(() => { - const observer = new ResizeObserver(() => updateIframeHeight()); - - updateIframeHeight(); - observer.observe(document.body); - return () => observer.disconnect(); - }, []); - return (