Merge pull request #460 from BloomBooks/optimizeBRLangs

andrew-polk · web-flow · commit 47282f6d9794 · 2022-09-20T12:14:41.000-07:00
Various optimizations, mainly for BloomReader
diff --git a/public/index.html b/public/index.html
@@ -48,10 +48,11 @@
     </head>
     <body>
         <noscript>You need to enable JavaScript to run this app.</noscript>
-        <div id="root"></div>
+        <div id="root">Loading...</div>
         <!--
       This HTML file is a template.
-      If you open it directly in the browser, you will see an empty page.
+      If you open it directly in the browser, you will just see the Loading message.
+      (It's there so something can be seen while fetching the code.)
 
       You can add webfonts, meta tags, or analytics to this file.
       The build step will place the bundled scripts into the <body> tag.
diff --git a/src/components/BookCardGroup.tsx b/src/components/BookCardGroup.tsx
@@ -17,7 +17,7 @@ import {
 } from "../connection/LibraryQueryHooks";
 import { BookCard, useBookCardSpec } from "./BookCard";
 import { MoreCard } from "./MoreCard";
-import { CardSwiperLazy } from "./CardSwiper";
+import { CardSwiperCodeSplit } from "./CardSwiperCodeSplit";
 import { ICollection } from "../model/ContentInterfaces";
 import Typography from "@material-ui/core/Typography";
 import { getLocalizedCollectionLabel } from "../localization/CollectionLabel";
@@ -157,7 +157,7 @@ const BookCardGroupInner: React.FunctionComponent<IProps> = (props) => {
             data.push("more");
         }
         bookList = (
-            <CardSwiperLazy
+            <CardSwiperCodeSplit
                 data={data}
                 cardSpec={cardSpec}
                 getReactElement={(item: IBasicBookInfo | "more", index) => {
@@ -180,7 +180,7 @@ const BookCardGroupInner: React.FunctionComponent<IProps> = (props) => {
                         );
                     }
                 }}
-            ></CardSwiperLazy>
+            ></CardSwiperCodeSplit>
         );
     } else {
         const verticalSpacing = cardSpec.cardSpacingPx;
diff --git a/src/components/BookGroup.tsx b/src/components/BookGroup.tsx
@@ -15,15 +15,17 @@ import {
     IBasicBookInfo,
 } from "../connection/LibraryQueryHooks";
 import { BookCard, useBookCardSpec } from "./BookCard";
-import { SingleRowCardSwiper } from "./SingleRowCardSwiper";
 import { useResponsiveChoice } from "../responsiveUtilities";
 import { BookOrderingScheme } from "../model/ContentInterfaces";
+import { CardSwiperCodeSplit } from "./CardSwiperCodeSplit";
 
 interface IProps {
     title: string;
     order?: string;
     // I don't know... this could be "bookLimit" instead "rows". Have to think in terms
     // of mobile versus big screen.... hmmm...
+    // Warning: if rows is 1 or unspecified, BookGroup shows a swiper row; this does not seem to be
+    // working well at least in our stories, but it is not used in the real website as of Sep 2022.
     rows?: number;
 
     contextLangIso?: string;
@@ -172,10 +174,24 @@ export const BookGroupInner: React.FunctionComponent<IProps> = (props) => {
     //     );
     // }
 
-    // JH Jan 2020: I cannot find anywhere that this is showInOne Row is used.
-    // It's also puzzling that it has to have its own CardSwiper implementation.
+    // JT Sep 2022: showInOneRow is true only when BookGroup is either told to show exactly one row, or
+    // not told how many rows to show. Currently this happens only in the various stories which were written
+    // early on to test it; the only real use of BookGroup is in ByLanguageGroups, which passes 999.
+    // When I tried the stories in Sep 2022, they were working very badly: showing a set of cards
+    // with no spacing. I wanted to retire the obsolete SingleRowCardSwiper, so I switched this rather crudely
+    // to use our current CardSwiperLazy. This looks a bit better but the book cards are not getting their
+    // images. Since this currently only shows up in test stories that we don't seem to be using, I decided
+    // not to try to fix it.
+    const cardSpec = useBookCardSpec();
     const bookList = showInOneRow ? (
-        <SingleRowCardSwiper wrapperRole="list">{cards}</SingleRowCardSwiper>
+        <CardSwiperCodeSplit
+            data={cards}
+            getReactElement={(x) => x}
+            wrapperRole="list"
+            cardSpec={cardSpec}
+        >
+            {cards}
+        </CardSwiperCodeSplit>
     ) : (
         <div
             css={css`
diff --git a/src/components/ByLanguageGroups.tsx b/src/components/ByLanguageGroups.tsx
@@ -41,7 +41,6 @@ export const ByLanguageGroups: React.FunctionComponent<{
     titlePrefix: string;
     collection: ICollection;
     reportBooksAndLanguages?: (bookCount: number, langCount: number) => void;
-    rowsPerLanguage?: number;
     // Sometimes it's nice to drop English, in particular.
     excludeLanguages?: string[];
 }> = (props) => {
@@ -195,7 +194,7 @@ export const ByLanguageGroups: React.FunctionComponent<{
                         }`}
                         predeterminedBooks={books}
                         contextLangIso={l.isoCode}
-                        rows={props.rowsPerLanguage ?? 999}
+                        rows={999}
                     />
                 );
             })}
diff --git a/src/components/CardRow.tsx b/src/components/CardRow.tsx
@@ -5,7 +5,7 @@ import { jsx } from "@emotion/core";
 /** @jsx jsx */
 import React, { ReactElement } from "react";
 import LazyLoad from "react-lazyload";
-import { CardSwiperLazy } from "./CardSwiper";
+import { CardSwiperCodeSplit } from "./CardSwiperCodeSplit";
 import { ICollection } from "../model/ContentInterfaces";
 import {
     CollectionLabel,
@@ -39,7 +39,7 @@ export const CardRow: React.FunctionComponent<IProps> = (props) => {
                 padding-left: 0;
             `}
         >
-            <CardSwiperLazy
+            <CardSwiperCodeSplit
                 wrapperRole="list"
                 data={props.data}
                 getReactElement={props.getCards}
diff --git a/src/components/CardSwiper.tsx b/src/components/CardSwiper.tsx
@@ -13,10 +13,11 @@ import "swiper/components/a11y/a11y.min.css";
 import { useResponsiveChoice } from "../responsiveUtilities";
 import { ICardSpec } from "./CardGroup";
 import { commonUI } from "../theme";
+import { ICardSwiperProps } from "./CardSwiperCodeSplit";
 
 SwiperCore.use([Navigation, A11y]);
 
-export const swiperConfig: Swiper = {
+const swiperConfig: Swiper = {
     preloadImages: false,
     lazy: {
         loadPrevNext: true,
@@ -48,22 +49,7 @@ export const swiperConfig: Swiper = {
 // small windows.
 // Enhance: this could be made generic, with a type param indicating that the type of objects in data
 // is the same as the type passed as the first argument of getReactElement
-export const CardSwiperLazy: React.FunctionComponent<{
-    data: any[];
-    // Given one of the items in data (and its index), return the react element that should be
-    // shown for that card when it is visible.
-    getReactElement: (card: any, index: number) => ReactElement;
-
-    // Typically the swiper is a list. I can't find a way to configure it so that the element containing
-    // the cards is a UL, but by setting this to 'list' and making items with role listitem we achieve
-    // the same accessibility goals. This role becomes the value of the role attribute of the swiper
-    // wrapper element, which is the immediate parent of the items. Note that it's not always a list, e.g.,
-    // in the LanguageGroup a further-out element is a listbox and the items have role 'option'.
-    // If you set a wrapperRole, make sure the children you pass have role listitem.
-    wrapperRole?: string;
-
-    cardSpec: ICardSpec;
-}> = (props) => {
+const CardSwiperLazy: React.FunctionComponent<ICardSwiperProps> = (props) => {
     const [swiper, setSwiper] = useState<any | null>(null);
     const getResponsiveChoice = useResponsiveChoice();
     const [showAll, setShowAll] = useState(false);
@@ -212,3 +198,5 @@ export const CardSwiperLazy: React.FunctionComponent<{
         </Swiper>
     );
 };
+
+export default CardSwiperLazy;
diff --git a/src/components/CardSwiperCodeSplit.tsx b/src/components/CardSwiperCodeSplit.tsx
@@ -0,0 +1,34 @@
+import React, { ReactElement } from "react";
+import { ICardSpec } from "./CardGroup";
+
+export interface ICardSwiperProps {
+    data: any[];
+    // Given one of the items in data (and its index), return the react element that should be
+    // shown for that card when it is visible.
+    getReactElement: (card: any, index: number) => ReactElement;
+
+    // Typically the swiper is a list. I can't find a way to configure it so that the element containing
+    // the cards is a UL, but by setting this to 'list' and making items with role listitem we achieve
+    // the same accessibility goals. This role becomes the value of the role attribute of the swiper
+    // wrapper element, which is the immediate parent of the items. Note that it's not always a list, e.g.,
+    // in the LanguageGroup a further-out element is a listbox and the items have role 'option'.
+    // If you set a wrapperRole, make sure the children you pass have role listitem.
+    wrapperRole?: string;
+
+    cardSpec: ICardSpec;
+}
+
+// This is wrapped so that we can keep all the javascript involved in the CardSwiper
+// in a separate js file, downloaded to the user's browser only if he/she needs it.
+export const CardSwiperCodeSplit: React.FunctionComponent<ICardSwiperProps> = (
+    props
+) => {
+    const CardSwiperLazy = React.lazy(
+        () => import(/* webpackChunkName: "cardSwiper" */ "./CardSwiper")
+    );
+    return (
+        <React.Suspense fallback={<div>Loading Cards...</div>}>
+            <CardSwiperLazy {...props} />
+        </React.Suspense>
+    );
+};
diff --git a/src/components/LanguageGroup.tsx b/src/components/LanguageGroup.tsx
@@ -18,7 +18,7 @@ import matchSorter from "match-sorter";
 import searchIcon from "../search.png";
 import { CachedTablesContext } from "../model/CacheProvider";
 import { ILanguage } from "../model/Language";
-import { CardSwiperLazy } from "./CardSwiper";
+import { CardSwiperCodeSplit } from "./CardSwiperCodeSplit";
 import { Redirect } from "react-router-dom";
 import { FormattedMessage, useIntl } from "react-intl";
 import { propsToHideAccessibilityElement } from "../Utilities";
@@ -54,7 +54,7 @@ export const LanguageGroup: React.FunctionComponent = () => {
         if (languagesToDisplay.length) {
             return (
                 <div {...getMenuProps({})}>
-                    <CardSwiperLazy
+                    <CardSwiperCodeSplit
                         data={languagesToDisplay}
                         cardSpec={cardSpec}
                         getReactElement={(l: ILanguage, index: number) => (
diff --git a/src/components/SingleRowCardSwiper.tsx b/src/components/SingleRowCardSwiper.tsx
diff --git a/src/connection/LibraryQueryHooks.ts b/src/connection/LibraryQueryHooks.ts
@@ -57,15 +57,19 @@ export interface IParseResponseDataWithCount<T = any> {
 /**
  * @summary For things other than books, which should use `useBookQuery()`
  * @param T The type of the result record returned by Parse. Or use "any" or omit to ignore types.
+ * If doNotActuallyRunQuery is true, the query will not be executed, and the result will always be empty.
+ * (This is useful when rules of hooks force us to call this unconditionally, but we sometimes don't want to.)
  */
 function useLibraryQuery<T = any>(
     queryClass: string,
-    params: {}
+    params: {},
+    doNotActuallyRunQuery?: boolean
 ): IReturns<IParseResponseData<T>> {
     return useAxios<IParseResponseData<T>>({
         url: `${getConnection().url}classes/${queryClass}`,
         method: "GET",
         trigger: "true",
+        forceDispatchEffect: () => !doNotActuallyRunQuery,
         options: {
             headers: getConnection().headers,
             params,
@@ -78,11 +82,14 @@ function useLibraryQuery<T = any>(
  */
 function useLibraryQueryWithCount<T = any>(
     queryClass: string,
-    params: {}
+    params: {},
+    doNotActuallyRunQuery?: boolean
 ): IReturns<IParseResponseDataWithCount<T>> {
-    return useLibraryQuery(queryClass, { ...params, count: 1 }) as IReturns<
-        IParseResponseDataWithCount<T>
-    >;
+    return useLibraryQuery(
+        queryClass,
+        { ...params, count: 1 },
+        doNotActuallyRunQuery
+    ) as IReturns<IParseResponseDataWithCount<T>>;
 }
 
 function useGetLanguagesList() {
@@ -103,10 +110,20 @@ export function useGetCleanedAndOrderedLanguageList(): ILanguage[] {
     return [];
 }
 export function useGetTagList(): string[] {
-    const axiosResult = useLibraryQueryWithCount("tag", {
-        limit: Number.MAX_SAFE_INTEGER,
-        order: "name",
-    });
+    // I'd prefer to use useAppHosted but it crashes, I think because this code runs outside the
+    // scope where it works.
+    const appHosted = isAppHosted();
+    const axiosResult = useLibraryQueryWithCount(
+        "tag",
+        {
+            limit: Number.MAX_SAFE_INTEGER,
+            order: "name",
+        },
+        appHosted // don't actually run the query if we're app hosted; we don't need tags
+    );
+    if (appHosted) {
+        return ["dummy"]; // don't need the list, but need non-empty to stop waiting.
+    }
 
     if (axiosResult.response?.data?.results) {
         assertAllParseRecordsReturned(axiosResult.response);
diff --git a/src/connection/ParseServerConnection.ts b/src/connection/ParseServerConnection.ts
