Skip to content

Commit

Permalink
address code review
Browse files Browse the repository at this point in the history
  • Loading branch information
acrantel committed Aug 15, 2023
1 parent cc771a0 commit 157304e
Show file tree
Hide file tree
Showing 7 changed files with 21 additions and 23 deletions.
14 changes: 7 additions & 7 deletions frontend2/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
<EpisodeContext.Provider value={{ episode, setEpisode }}>
<EpisodeContext.Provider value={{ episodeId, setEpisodeId }}>
<RouterProvider router={router} />
</EpisodeContext.Provider>
);
Expand All @@ -38,12 +38,12 @@ const router = createBrowserRouter([
element: <EpisodeLayout />,
children: [
// Pages that should always be visible
// TODO: /:episode/resources, /:episode/tournaments, /:episode/rankings, /:episode/queue
{ path: "/:episode/home", element: <Home /> },
{ path: "/:episode/quickstart", element: <QuickStart /> },
{ path: "/:episode/*", element: <NotFound /> },
// TODO: /:episodeId/resources, /:episodeId/tournaments, /:episodeId/rankings, /:episodeId/queue
{ path: "/:episodeId/home", element: <Home /> },
{ path: "/:episodeId/quickstart", element: <QuickStart /> },
{ path: "/:episodeId/*", element: <NotFound /> },
// 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: <Account /> },
// etc
],
Expand Down
6 changes: 3 additions & 3 deletions frontend2/src/components/EpisodeLayout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ import { EpisodeContext } from "../contexts/EpisodeContext";
// Child route components are rendered with <Outlet />
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 (
<div className="h-screen">
Expand Down
8 changes: 4 additions & 4 deletions frontend2/src/components/sidebar/__test__/Sidebar.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(<MemoryRouter><Sidebar /></MemoryRouter>);
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(<MemoryRouter><Sidebar collapsed={true} /></MemoryRouter>);
expect(screen.queryByText('Home')).toBeNull();
});

test('should link to episode in surrounding context', () => {
test('UI: should link to episode in surrounding context', () => {
render(<MemoryRouter>
<EpisodeContext.Provider value={{ episode: "something", setEpisode: (_) => undefined }}>
<EpisodeContext.Provider value={{ episodeId: "something", setEpisodeId: (_) => undefined }}>
<Sidebar />
</EpisodeContext.Provider></MemoryRouter>
);
Expand Down
4 changes: 2 additions & 2 deletions frontend2/src/components/sidebar/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ const Sidebar: React.FC<SidebarProps> = ({
collapsed
}) => {
collapsed = collapsed ?? false;
const { episode } = useContext(EpisodeContext);
const linkBase = `/${episode}/`;
const { episodeId } = useContext(EpisodeContext);
const linkBase = `/${episodeId}/`;

return (
collapsed ? null : (
Expand Down
4 changes: 2 additions & 2 deletions frontend2/src/contexts/EpisodeContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
},
});
4 changes: 2 additions & 2 deletions frontend2/src/views/Account.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ import React, { useContext } from "react";
import { EpisodeContext } from "../contexts/EpisodeContext";

const Account: React.FC = () => {
const { episode } = useContext(EpisodeContext);
return <p>the episode is {episode} and this is the account page</p>;
const { episodeId } = useContext(EpisodeContext);
return <p>the episode is {episodeId} and this is the account page</p>;
};

export default Account;
4 changes: 1 addition & 3 deletions frontend2/src/views/QuickStart.tsx
Original file line number Diff line number Diff line change
@@ -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 <p>quickstart page</p>;
};

Expand Down

0 comments on commit 157304e

Please sign in to comment.