From 16788603ce21995e7c152dcd2d5d41e79bbb829d Mon Sep 17 00:00:00 2001 From: Drew Hess Date: Mon, 10 Jul 2023 18:53:45 +0100 Subject: [PATCH] fix: always render the session page nav bar The previous commit improved matters on the session page by handling `isError` and `isLoading` conditions, but has annoying behavior when the student is typing a program name into the search bar and we're doing live search, because the sessions page was rendered all-or-nothing. Now we always show the nav bar. This means we no longer have a nice single component for the sessions page (i.e., the `SessionsPage` component is no more), but `ChooseSession` is now more or less the same thing, only with better error and loading behavior. I haven't bothered to add a Storybook story for it, but it would be easyenough to add later if we need it. Signed-off-by: Drew Hess --- src/components/ChooseSession/index.tsx | 135 +++++++++++------- .../SessionsPage/SessionsPage.stories.tsx | 50 ------- src/components/SessionsPage/index.tsx | 134 ----------------- src/components/index.ts | 1 - 4 files changed, 85 insertions(+), 235 deletions(-) delete mode 100644 src/components/SessionsPage/SessionsPage.stories.tsx delete mode 100644 src/components/SessionsPage/index.tsx diff --git a/src/components/ChooseSession/index.tsx b/src/components/ChooseSession/index.tsx index b07c5db3..2c9d85b0 100644 --- a/src/components/ChooseSession/index.tsx +++ b/src/components/ChooseSession/index.tsx @@ -1,8 +1,13 @@ -import type { MouseEventHandler } from "react"; import { useState } from "react"; import { useCookies } from "react-cookie"; -import { exampleAccount, SessionsPage } from "@/components"; -import type { PaginatedMeta, Session, Uuid } from "@/primer-api"; +import { + exampleAccount, + SessionList, + SessionNameModal, + SessionsNavBar, + SimplePaginationBar, +} from "@/components"; +import type { Uuid } from "@/primer-api"; import { useGetSessionList, useCreateSession, @@ -15,6 +20,12 @@ import { useQueryClient } from "@tanstack/react-query"; const ChooseSession = (): JSX.Element => { const [cookies] = useCookies(["id"]); + const [importPrelude, setImportPrelude] = useState(true); + const [showModal, setShowModal] = useState(false); + const onClickNewProgram = (): void => { + setShowModal(true); + }; + // NOTE: pagination in our API is 1-indexed. const [page, setPage] = useState(1); const [pageSize] = useState(20); @@ -68,64 +79,88 @@ const ChooseSession = (): JSX.Element => { } ); - // Note that we show data if it's available, without first checking for + // Note that we show data if it's available, regardless of the status of // `isLoading` or `isError`. This means we may show stale data, but we prefer - // this over showing a loading message or an error for short server outages. See: + // this over showing a loading message or an error for short server outages. + // See: // // https://tkdodo.eu/blog/status-checks-in-react-query // // Note that React Query will not show stale data indefinitely, and will // eventually show an error message if the data is stale for too long. - if (data) { - const sessions: Session[] = data.items; - const meta: PaginatedMeta = data.meta; - const startIndex: number = (meta.thisPage - 1) * meta.pageSize + 1; - - const onClickNextPage: MouseEventHandler | undefined = - meta.thisPage < meta.lastPage ? () => setPage(page + 1) : undefined; - const onClickPreviousPage: MouseEventHandler | undefined = - meta.thisPage > 1 ? () => setPage(page - 1) : undefined; - - return ( - - newSession.mutate({ data: { name, importPrelude } }) - } - onClickNextPage={onClickNextPage} - onClickPreviousPage={onClickPreviousPage} - onClickDelete={(sessionId) => deleteSession.mutate({ sessionId })} - onSubmitSearch={(nameFilter: string) => { - // Unlike `onChangeSearch`, this callback is always triggered - // by an explicit action, and never by, e.g., a page refresh, - // so we always want to reset the page when this callback is - // invoked. - setSessionNameFilter(nameFilter); - setPage(1); - }} - onChangeSearch={(nameFilter: string) => { - // For technical reasons, this callback may be triggered even - // if the value of the search term didn't actually change - // (e.g., because the page is redrawn), and in these cases, we - // don't want to update the page, so we filter these spurious - // "changes" out. - if (nameFilter != sessionNameFilter) { + return ( +
+
+ { + // Unlike `onChangeSearch`, this callback is always triggered + // by an explicit action, and never by, e.g., a page refresh, + // so we always want to reset the page when this callback is + // invoked. setSessionNameFilter(nameFilter); setPage(1); - } + }} + onChangeSearch={(nameFilter: string) => { + // For technical reasons, this callback may be triggered even + // if the value of the search term didn't actually change + // (e.g., because the page is redrawn), and in these cases, we + // don't want to update the page, so we filter these spurious + // "changes" out. + if (nameFilter != sessionNameFilter) { + setSessionNameFilter(nameFilter); + setPage(1); + } + }} + /> +
+
+ {data ? ( + deleteSession.mutate({ sessionId })} + /> + ) : isError ? ( +
Error: {error.message}
+ ) : ( +
Loading...
+ )} +
+
+ {data && ( + setPage(page + 1) + : undefined + } + onClickPreviousPage={ + data.meta.thisPage > 1 ? () => setPage(page - 1) : undefined + } + /> + )} +
+ setShowModal(false)} + onCancel={() => setShowModal(false)} + onSubmit={(name: string, _importPrelude: boolean) => { + // Remember the student's choice of whether or not to import the Prelude. + setImportPrelude(_importPrelude); + newSession.mutate({ + data: { name, importPrelude: _importPrelude }, + }); }} /> - ); - } else if (isError) { - return
Error: {error.message}
; - } else { - return
Loading...
; - } +
+ ); }; export default ChooseSession; diff --git a/src/components/SessionsPage/SessionsPage.stories.tsx b/src/components/SessionsPage/SessionsPage.stories.tsx deleted file mode 100644 index a86acd23..00000000 --- a/src/components/SessionsPage/SessionsPage.stories.tsx +++ /dev/null @@ -1,50 +0,0 @@ -import { ComponentStory, ComponentMeta } from "@storybook/react"; -import { SessionsPage } from "./"; -import { exampleSessions } from "@/components/examples/sessions"; -import { exampleAccount } from "@/components/examples/accounts"; - -export default { - title: "Application/Component Library/Sessions/SessionsPage", - component: SessionsPage, - argTypes: { - sessions: { control: "object", name: "List of sessions" }, - account: { control: "object", name: "Account" }, - startIndex: { - description: "The 1-based index of the first item shown on this page.", - control: "number", - }, - totalItems: { - description: "The total number of items.", - control: "number", - }, - onClickNextPage: { - description: 'The event handler for the "Next" button.', - action: "clicked", - }, - onClickPreviousPage: { - description: 'The event handler for the "Previous" button.', - action: "clicked", - }, - onClickDelete: { action: "delete" }, - onSubmitSearch: { - description: "The search bar's onSubmit handler.", - action: "submitSearch", - }, - onChangeSearch: { - description: "The search bar's onChange handler.", - action: "changeSearch", - }, - }, -} as ComponentMeta; - -const Template: ComponentStory = (args) => ( - -); - -export const Default = Template.bind({}); -Default.args = { - account: exampleAccount, - sessions: exampleSessions(), - startIndex: 21, - totalItems: 95, -}; diff --git a/src/components/SessionsPage/index.tsx b/src/components/SessionsPage/index.tsx deleted file mode 100644 index 00fd86be..00000000 --- a/src/components/SessionsPage/index.tsx +++ /dev/null @@ -1,134 +0,0 @@ -import { useState } from "react"; -import type { MouseEventHandler } from "react"; - -import { - SessionsNavBar, - SimplePaginationBar, - SessionList, - SessionNameModal, -} from "@/components"; -import type { Account } from "@/Types"; -import type { Session, Uuid } from "@/primer-api"; - -import "@/index.css"; - -export interface SessionsPageProps { - /** - * The account whose sessions will be displayed. - * - * @type {Account} - */ - account: Account; - - /** - * The list of session metadata displayed on this page. - * - * @type {Session[]} - */ - sessions: Session[]; - - /** - * The event handler for the "New program" button. - */ - onClickNewProgram: (name: string, importPrelude: boolean) => void; - - /** - * The 1-based index of the first item shown on this page. - * - * @type {number} - */ - startIndex: number; - - /** - * The number of items displayed on this page. - * - * @type {number} - */ - numItems: number; - - /** - * The total number of items. - * - * @type {number} - */ - totalItems: number; - - /** - * The event handler for the "next page" button, if there is one. - * - * @type {MouseEventHandler | undefined} - */ - onClickNextPage: MouseEventHandler | undefined; - - /** - * The event handler for the "previous page" button, if there is one. - * - * @type {MouseEventHandler | undefined} - */ - onClickPreviousPage: MouseEventHandler | undefined; - - /** - * The event handler for deleting the given session, identified by its ID. - * - * @type {(id: Uuid) => void} - */ - - onClickDelete: (id: Uuid) => void; - - /** - * The search bar's onChange handler. - */ - onChangeSearch: (searchString: string) => void; - - /** - * The search bar's onSubmit handler. - */ - onSubmitSearch: (searchString: string) => void; -} - -export const SessionsPage = (p: SessionsPageProps): JSX.Element => { - const [importPrelude, setImportPrelude] = useState(true); - const [showModal, setShowModal] = useState(false); - const onClickNewProgram = (): void => { - setShowModal(true); - }; - - return ( -
-
- -
-
- -
-
- -
- setShowModal(false)} - onCancel={() => setShowModal(false)} - onSubmit={(name: string, _importPrelude: boolean) => { - // Remember the student's choice of whether or not to import the Prelude. - setImportPrelude(_importPrelude); - p.onClickNewProgram(name, _importPrelude); - }} - /> -
- ); -}; - -export default SessionsPage; diff --git a/src/components/index.ts b/src/components/index.ts index 8dc16d0a..649024c5 100644 --- a/src/components/index.ts +++ b/src/components/index.ts @@ -22,7 +22,6 @@ export { default as SessionList } from "./SessionList"; export { default as SessionNameModal } from "./SessionNameModal"; export { default as SessionPreview } from "./SessionPreview"; export { default as SessionsNavBar } from "./SessionsNavBar"; -export { default as SessionsPage } from "./SessionsPage"; export { default as SimplePaginationBar } from "./SimplePaginationBar"; export { default as Toolbar } from "./Toolbar"; export { default as TreeReactFlow, TreeReactFlowOne } from "./TreeReactFlow";