Add /events/[year]/[term] page #158

													
				@ -0,0 +2,4 @@

				import React from "react";

				import { EventCard } from "../../../../components/EventCard";

				import { MiniEventCard } from "../../../../components/MiniEventCard";

a3thakra commented

2021-08-18 18:41:23 -04:00

Use "@/components/..." instead of "../../../../components/..."

j285he marked this conversation as resolved

a3thakra reviewed 2021-08-18 18:41:48 -04:00

pages/events/[year]/[term]/index.tsx Outdated

													
				@ -0,0 +9,4 @@

				  getEventsByTerm,

				  getEventBySlug,

				  Event,

				} from "../../../../lib/events";

a3thakra commented

2021-08-18 18:41:48 -04:00

use "@/lib/events" instead of "../../../lib/events"

j285he marked this conversation as resolved

a3thakra reviewed 2021-08-18 18:43:39 -04:00

pages/events/[year]/[term]/index.tsx Outdated

													
				@ -0,0 +13,4 @@

				import styles from "./term.module.css";

				export async function getStaticPaths(): Promise<{

a3thakra commented

2021-08-18 18:43:39 -04:00

This is not the correct way to type out getStaticPaths. You should do something like the news archive files.

export const getStaticPaths: GetStaticPaths = async () => {...} something like this

This is not the correct way to type out `getStaticPaths`. You should do something like the news archive files. `export const getStaticPaths: GetStaticPaths = async () => {...}` something like this

j285he marked this conversation as resolved

a3thakra reviewed 2021-08-18 18:44:34 -04:00

pages/events/[year]/[term]/index.tsx Outdated

													
				@ -0,0 +25,4 @@

				  const years = await getEventYears();

				  const paths = [];

				  for (const year of years) {

				    const terms = await getEventTermsByYear(year);

a3thakra commented

2021-08-18 18:44:34 -04:00

This is inefficient. It always wait for 1 year to complete before proceeding to the next. You should use Promise.all here.

j285he commented

2021-08-19 11:01:53 -04:00

I'm a little confused here. The problem is that await getEventTermsByYear(year) forces the function to stop until getEventTermsByYear returns, right?

But on the next line I am getting an error with being unable to map on a type of Promise<string[]>. How can I resolve this?

Thanks!

I'm a little confused here. The problem is that `await getEventTermsByYear(year)` forces the function to stop until `getEventTermsByYear` returns, right? But on the next line I am getting an error with being unable to map on a type of `Promise<string[]>`. How can I resolve this? Thanks!

a3thakra commented

2021-08-22 05:24:33 -04:00

yes.

you can await on a Promise.all to map over it.

so something like:

const paths = (await Promise.all(
  years.map(async (year) => {
    const terms = await getEventTermsByYear(year);
    return terms.map((curTerm) => ({ params: { year: year, term: curTerm } }))
  })
)).flat()

yes. you can `await` on a Promise.all to map over it. so something like: ```ts const paths = (await Promise.all( years.map(async (year) => { const terms = await getEventTermsByYear(year); return terms.map((curTerm) => ({ params: { year: year, term: curTerm } })) }) )).flat() ```

j285he commented

2021-08-22 15:03:37 -04:00

Would the use of await getEventTermsByYear here still force the async function on line 2 to wait until getEventTermsByYear returns?

Is the difference here that years.map can map each element asynchronously, whereas in my original function for (const year of years) cannot?

Would the use of `await getEventTermsByYear` here still force the async function on line 2 to wait until `getEventTermsByYear` returns? Is the difference here that `years.map` can map each element asynchronously, whereas in my original function `for (const year of years)` cannot?

a3thakra commented

2021-08-22 20:55:00 -04:00

the for loop is going to wait for each year to be completed before the next can start.

But with this approach, all years start simultaneously.

Within each year, we are still waiting for terms, that doesn't change in both cases.

2020
  - winter
  - spring
  - fall
2021
  - winter
  - spring
  - fall

vs

2020               | 2021
  - winter         |   - winter
  - spring         |   - spring
  - fall           |   - fall

the for loop is going to wait for each year to be completed before the next can start. But with this approach, all years start simultaneously. Within each year, we are still waiting for terms, that doesn't change in both cases. ``` 2020 - winter - spring - fall 2021 - winter - spring - fall ``` vs ``` 2020 | 2021 - winter | - winter - spring | - spring - fall | - fall ```

j285he commented

2021-08-23 20:14:48 -04:00

Ah that makes sense, thank you!

j285he marked this conversation as resolved

a3thakra reviewed 2021-08-18 18:46:22 -04:00

pages/events/[year]/[term]/index.tsx Outdated

													
				@ -0,0 +58,4 @@

				  const events = (

				    await Promise.all(

				      (

				        await getEventsByTerm(context.params.year, context.params.term)

a3thakra commented

2021-08-18 18:46:22 -04:00

This is a lot of nesting, can you move this to a separate line, and bind that to a variable?

j285he marked this conversation as resolved

a3thakra reviewed 2021-08-18 18:49:46 -04:00

pages/events/[year]/[term]/term.module.css Outdated

													
				@ -0,0 +2,4 @@

				  margin: auto;

				  margin-top: 7rem;

				  margin-bottom: 14rem;

				  width: 50rem;

a3thakra commented

2021-08-18 18:49:46 -04:00

Can you convert these rem values to pixels? Using the calc(<value in px>rem / 16) method that we use everywhere.

Can you convert these rem values to pixels? Using the `calc(<value in px>rem / 16)` method that we use everywhere.

j285he marked this conversation as resolved

a3thakra reviewed 2021-08-18 18:50:00 -04:00

pages/events/[year]/[term]/term.module.css Outdated

													
				@ -0,0 +16,4 @@

				}

				.miniEventCards {

				  margin-top: 30px;

a3thakra commented

2021-08-18 18:50:00 -04:00

This should be in rems

j285he marked this conversation as resolved

a3thakra reviewed 2021-08-18 18:51:31 -04:00

pages/events/[year]/[term]/index.tsx Outdated

													
				@ -0,0 +86,4 @@

				  };

				}

				const Term = (props: Props) => {

a3thakra commented

2021-08-18 18:51:31 -04:00

In all the other files, we place the react component at the top of the file, we should do that here too, for consistency. Also you can use export default function Term(props: Props) {...} syntax here, instead of the anonymous function.

In all the other files, we place the react component at the top of the file, we should do that here too, for consistency. Also you can use `export default function Term(props: Props) {...}` syntax here, instead of the anonymous function.

j285he marked this conversation as resolved

a3thakra referenced this pull request

2021-08-18 18:53:07 -04:00

Add /events/[year] page #151

j285he added 2 commits 2021-08-19 11:04:37 -04:00

25c83364d6 Merge branch 'main' into feat/events-term

continuous-integration/drone/push Build is passing Details

891b6b29a8 Resolve some issues

j285he added 2 commits 2021-08-22 15:05:36 -04:00

9e8b381064 Address CSS issues

continuous-integration/drone/push Build is passing Details

3a6f799538 Optimize getStaticPaths

j285he added 2 commits 2021-08-23 20:16:08 -04:00

bd7df3de6a Merge branch 'main' into feat/events-term

continuous-integration/drone/push Build is passing Details

56e4313ef2 60px margins

j285he added 1 commit 2021-08-23 20:20:29 -04:00

continuous-integration/drone/push Build is passing Details

24885e5185 Add margin-bottom for terms

a3thakra added 1 commit 2021-08-23 22:21:30 -04:00

continuous-integration/drone/push Build is passing Details

2abc7e75c1 PR fixes

j285he added 1 commit 2021-08-23 23:31:29 -04:00

continuous-integration/drone/push Build is failing Details

8d59ae1692 Add getNextTerm, getPreviousTerm

a3thakra added 1 commit 2021-08-27 16:04:33 -04:00

continuous-integration/drone/push Build is passing Details

24a237a8c6 Add header functionality (#183 )

Co-authored-by: Jared He <66887902+jaredjhe@users.noreply.github.com>
Co-authored-by: Aditya Thakral <a3thakra@csclub.uwaterloo.ca>
Reviewed-on: #183
Co-authored-by: j285he <j285he@localhost>
Co-committed-by: j285he <j285he@localhost>

a3thakra approved these changes 2021-08-27 16:11:36 -04:00

a3thakra merged commit e548bd9c5a into main

2021-08-27 16:14:55 -04:00

a3thakra referenced this issue from a commit

2021-08-27 16:14:55 -04:00

Add /events/[year]/[term] page (#158)

a3thakra deleted branch feat/events-term

2021-08-27 16:15:33 -04:00

j285he referenced this issue from a commit

2021-08-27 18:37:44 -04:00

Add /events page (#159)

Sign in to join this conversation.