From 157304e383629ca67524bddfd71a956aba428adb Mon Sep 17 00:00:00 2001 From: Serena Li Date: Sun, 25 Jun 2023 15:52:03 -0400 Subject: [PATCH] address code review --- frontend2/src/App.tsx | 14 +++++++------- frontend2/src/components/EpisodeLayout.tsx | 6 +++--- .../components/sidebar/__test__/Sidebar.test.tsx | 8 ++++---- frontend2/src/components/sidebar/index.tsx | 4 ++-- frontend2/src/contexts/EpisodeContext.ts | 4 ++-- frontend2/src/views/Account.tsx | 4 ++-- frontend2/src/views/QuickStart.tsx | 4 +--- 7 files changed, 21 insertions(+), 23 deletions(-) diff --git a/frontend2/src/App.tsx b/frontend2/src/App.tsx index 750890fea..3563aeec1 100644 --- a/frontend2/src/App.tsx +++ b/frontend2/src/App.tsx @@ -18,9 +18,9 @@ import { DEFAULT_EPISODE } from "./utils/constants"; import NotFound from "./views/NotFound"; const App: React.FC = () => { - const [episode, setEpisode] = useState(DEFAULT_EPISODE); + const [episodeId, setEpisodeId] = useState(DEFAULT_EPISODE); return ( - + ); @@ -38,12 +38,12 @@ const router = createBrowserRouter([ element: , children: [ // Pages that should always be visible - // TODO: /:episode/resources, /:episode/tournaments, /:episode/rankings, /:episode/queue - { path: "/:episode/home", element: }, - { path: "/:episode/quickstart", element: }, - { path: "/:episode/*", element: }, + // TODO: /:episodeId/resources, /:episodeId/tournaments, /:episodeId/rankings, /:episodeId/queue + { path: "/:episodeId/home", element: }, + { path: "/:episodeId/quickstart", element: }, + { path: "/:episodeId/*", element: }, // Pages that should only be visible when logged in - // TODO: /:episode/team, /:episode/submissions, /:episode/scrimmaging + // TODO: /:episodeId/team, /:episodeId/submissions, /:episodeId/scrimmaging { path: "/account", element: }, // etc ], diff --git a/frontend2/src/components/EpisodeLayout.tsx b/frontend2/src/components/EpisodeLayout.tsx index 033bbfc45..41b9ba710 100644 --- a/frontend2/src/components/EpisodeLayout.tsx +++ b/frontend2/src/components/EpisodeLayout.tsx @@ -8,9 +8,9 @@ import { EpisodeContext } from "../contexts/EpisodeContext"; // Child route components are rendered with const EpisodeLayout: React.FC = () => { const episodeContext = useContext(EpisodeContext); - const { episode } = useParams(); - if (episode !== undefined && episode !== episodeContext.episode) { - episodeContext.setEpisode(episode); + const { episodeId } = useParams(); + if (episodeId !== undefined && episodeId !== episodeContext.episodeId) { + episodeContext.setEpisodeId(episodeId); } return (
diff --git a/frontend2/src/components/sidebar/__test__/Sidebar.test.tsx b/frontend2/src/components/sidebar/__test__/Sidebar.test.tsx index 35423c69f..99ebe9f69 100644 --- a/frontend2/src/components/sidebar/__test__/Sidebar.test.tsx +++ b/frontend2/src/components/sidebar/__test__/Sidebar.test.tsx @@ -5,20 +5,20 @@ import { DEFAULT_EPISODE } from "../../../utils/constants"; import { EpisodeContext } from "../../../contexts/EpisodeContext"; import { MemoryRouter } from "react-router-dom"; -test('should link to default episode', () => { +test('UI: should link to default episode', () => { render(); const linkElement = screen.getByText('Resources').closest('a')?.getAttribute('href'); expect(linkElement).toEqual(expect.stringContaining(`/${DEFAULT_EPISODE}/resources`)); }); -test('should collapse sidebar', () => { +test('UI: should collapse sidebar', () => { render(); expect(screen.queryByText('Home')).toBeNull(); }); -test('should link to episode in surrounding context', () => { +test('UI: should link to episode in surrounding context', () => { render( - undefined }}> + undefined }}> ); diff --git a/frontend2/src/components/sidebar/index.tsx b/frontend2/src/components/sidebar/index.tsx index 3cc885acc..be668c857 100644 --- a/frontend2/src/components/sidebar/index.tsx +++ b/frontend2/src/components/sidebar/index.tsx @@ -16,8 +16,8 @@ const Sidebar: React.FC = ({ collapsed }) => { collapsed = collapsed ?? false; - const { episode } = useContext(EpisodeContext); - const linkBase = `/${episode}/`; + const { episodeId } = useContext(EpisodeContext); + const linkBase = `/${episodeId}/`; return ( collapsed ? null : ( diff --git a/frontend2/src/contexts/EpisodeContext.ts b/frontend2/src/contexts/EpisodeContext.ts index 214c688ec..9e8efdb5d 100644 --- a/frontend2/src/contexts/EpisodeContext.ts +++ b/frontend2/src/contexts/EpisodeContext.ts @@ -3,8 +3,8 @@ import { DEFAULT_EPISODE } from "../utils/constants"; export const EpisodeContext = createContext({ // the default episode. - episode: DEFAULT_EPISODE, - setEpisode: (episode: string) => { + episodeId: DEFAULT_EPISODE, + setEpisodeId: (episodeId: string) => { console.log("default episode"); }, }); diff --git a/frontend2/src/views/Account.tsx b/frontend2/src/views/Account.tsx index 6516a9ad8..a6b41d31e 100644 --- a/frontend2/src/views/Account.tsx +++ b/frontend2/src/views/Account.tsx @@ -2,8 +2,8 @@ import React, { useContext } from "react"; import { EpisodeContext } from "../contexts/EpisodeContext"; const Account: React.FC = () => { - const { episode } = useContext(EpisodeContext); - return

the episode is {episode} and this is the account page

; + const { episodeId } = useContext(EpisodeContext); + return

the episode is {episodeId} and this is the account page

; }; export default Account; diff --git a/frontend2/src/views/QuickStart.tsx b/frontend2/src/views/QuickStart.tsx index 8c79a6404..f12d53556 100644 --- a/frontend2/src/views/QuickStart.tsx +++ b/frontend2/src/views/QuickStart.tsx @@ -1,8 +1,6 @@ -import React, { useContext } from "react"; -import { EpisodeContext } from "../contexts/EpisodeContext"; +import React from "react"; const QuickStart: React.FC = () => { - const { episode, setEpisode } = useContext(EpisodeContext); return

quickstart page

; };