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.
This commit is contained in:
committed by
Umputun
parent
e62b3c830d
commit
a8dd527c45
@@ -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<StoreState> = {
|
||||
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('<ConnectedRoot />', () => {
|
||||
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(<ConnectedRoot />, 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(<ConnectedRoot />, stateStub);
|
||||
expect(updateIframeHeight).not.toHaveBeenCalled();
|
||||
|
||||
jest.advanceTimersByTime(5000);
|
||||
expect(updateIframeHeight).toHaveBeenCalled();
|
||||
} finally {
|
||||
jest.useRealTimers();
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -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<Props, State> {
|
||||
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<Props, State> {
|
||||
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 (
|
||||
<div className={clsx(styles.root, props.theme === 'dark' ? styles.themeDark : styles.themeLight, props.theme)}>
|
||||
<Root {...props} {...actions} intl={intl} />
|
||||
|
||||
Reference in New Issue
Block a user