fix: keep the page shell rendered while a discussion loads

Navigating between discussions remounts DiscussionPage, and its view
bailed out to a bare loading indicator until the discussion arrived —
blanking the entire layout, including the pinned discussion list pane,
whose contents are already cached and need no network. With the pane
pinned, every discussion-to-discussion navigation flashed the whole
page white.

PageStructure was already built for this: given loading, it renders the
spinner inside the main area, keeps the pane, and skips the hero,
sidebar and content closures so none of them run without a discussion.
The early return was defeating that shell. The pane also already
restores its scroll position when coming from another discussion page,
so the rebuilt pane is visually stable.

The view now always renders PageStructure, passing loading while the
discussion or the post stream components are still pending.

Author: imorland

framework/core/js/src/forum/components/DiscussionPage.tsx

@@ -5,7 +5,6 @@ import Page, { IPageAttrs } from '../../common/components/Page';
 import ItemList from '../../common/utils/ItemList';
 import DiscussionHero from './DiscussionHero';
 import DiscussionListPane from './DiscussionListPane';
-import LoadingIndicator from '../../common/components/LoadingIndicator';
 import SplitDropdown from '../../common/components/SplitDropdown';
 import listItems from '../../common/helpers/listItems';
 import DiscussionControls from '../utils/DiscussionControls';
@@ -94,19 +93,24 @@ export default class DiscussionPage<CustomAttrs extends IDiscussionPageAttrs = I
   }
 
   view() {
-    if (this.loading || !this.discussion) {
-      return <LoadingIndicator />;
-    }
+    // Keep the page shell rendered while the discussion loads. Bailing out to
+    // a bare loading indicator here blanks the whole layout on every
+    // discussion-to-discussion navigation — including the pinned discussion
+    // list pane, whose contents are already cached and need no network.
+    // PageStructure renders the spinner inside the main area and skips the
+    // hero/sidebar/content closures while `loading` is set, so none of them
+    // run without a discussion.
+    const loading = this.loading || !this.discussion;
 
     return (
       <PageStructure
         className="DiscussionPage"
-        loading={this.loading}
+        loading={loading}
         hero={this.hero.bind(this)}
         sidebar={this.sidebar.bind(this)}
         pane={() => <DiscussionListPane state={app.discussions} />}
       >
-        {this.loading || (
+        {loading || (
           <div className="DiscussionPage-stream">
             <this.PostStream discussion={this.discussion} stream={this.stream} onPositionChange={this.positionChanged.bind(this)} />
           </div>

framework/core/js/tests/integration/forum/components/DiscussionPage.test.ts

@@ -57,13 +57,19 @@ describe('DiscussionPage', () => {
   beforeAll(() => {
     bootstrapForum();
     app.boot();
+
   });
 
   beforeEach(() => {
     routeParams = {};
     findCalls = [];
     pendingFinds = [];
 
+    // The pane helper is created by ForumApplication when mounting the real
+    // app; the pane component's lifecycle hooks expect it to exist.
+    // @ts-ignore
+    app.pane = app.pane || { enable() {}, disable() {}, hide() {}, show() {}, onmouseleave() {}, togglePinned() {} };
+
     // @ts-ignore
     m.route.param = (key?: string) => (key ? routeParams[key] : routeParams);
 
@@ -115,6 +121,24 @@ describe('DiscussionPage', () => {
     pending!.resolve(models);
   };
 
+  test('keeps the page structure while the discussion loads instead of blanking', () => {
+    // Navigating between discussions remounts the page. Rendering a bare
+    // loading indicator during the fetch blanks the whole layout — including
+    // the pinned discussion list pane, whose contents are already cached and
+    // need no network. The shell must stay; only the main area may spin.
+    seedDiscussionFromList();
+    routeParams = { id: '476-test-discussion' };
+
+    const page = mountPage();
+
+    // Nothing resolved yet: the page is loading. The pane slot must exist
+    // (its contents render whenever the list state has items), and the
+    // spinner must be inside the main area, not the whole page.
+    expect(page).toHaveElement('.DiscussionPage');
+    expect(page).toHaveElement('.Page-pane');
+    expect(page).toHaveElement('#page-main .LoadingIndicator');
+  });
+
   test('requests the discussion and the post window in parallel when the discussion is known', () => {
     seedDiscussionFromList();
     routeParams = { id: '476-test-discussion' };