Add Multiple News Items on Homepage and Single News Item Page (Closes #281) #390

Merged
e26chiu merged 6 commits from support-multiple-news-homepage into main 2022-02-23 00:15:39 -05:00
9 changed files with 186 additions and 15 deletions

View File

@ -3,6 +3,7 @@
max-width: calc(524rem / 16);
background-color: var(--primary-background);
border-radius: calc(20rem / 16);
margin-bottom: 1rem;
}
.fit.card {
@ -30,6 +31,7 @@
padding: 0;
max-width: unset;
background-color: transparent;
border-radius: 0;
}
.date {

View File

@ -1,11 +1,14 @@
import React, { ReactNode } from "react";
import { Link } from "./Link";
import styles from "./NewsCard.module.css";
interface NewsCardProps {
date: Date;
author: string;
children: ReactNode;
permalink: string;
fit?: boolean;
Review

It might be helpful to leave a comment here explaining what the fit prop is used for.

It might be helpful to leave a comment here explaining what the `fit` prop is used for.
}
@ -13,7 +16,8 @@ export const NewsCard: React.FC<NewsCardProps> = ({
date,
author,
children,
fit = false,
permalink,
fit = false, // resizes the article to fit the parent container if it's not a mini card
}) => {
const classes = fit ? [styles.card, styles.fit] : [styles.card];
@ -30,6 +34,11 @@ export const NewsCard: React.FC<NewsCardProps> = ({
</h1>
<address className={styles.author}>{author}</address>
<div className={styles.content}>{children}</div>
{!fit && (
<Link href={permalink}>
<span>Learn more</span>
</Link>
)}
</article>
);
};

View File

@ -3,6 +3,7 @@ import path from "path";
import { parse } from "date-fns";
import matter from "gray-matter";
import truncateMarkdown from "markdown-truncate";
import { MDXRemoteSerializeResult } from "next-mdx-remote";
import { serialize } from "next-mdx-remote/serialize";
@ -15,6 +16,7 @@ export const NEWS_PATH = path.join("content", "news");
export interface Metadata {
author: string;
date: string;
permalink: string;
}
export interface News {
@ -40,6 +42,15 @@ export async function getNewsTermsByYear(year: string): Promise<Term[]> {
.sort((a, b) => TERMS.indexOf(a) - TERMS.indexOf(b));
}
export async function getNewsDateByTerm(
year: string,
term: Term
): Promise<string[]> {
return (await getNewsByTerm(year, term)).map(
(news) => news.split("-").slice(0, 3).join("-") // retrieves date from filename
);
}
export async function getNewsByTerm(
year: string,
term: Term
@ -56,13 +67,21 @@ export async function getNewsByTerm(
export async function getNewsBySlug(
year: string,
term: Term,
slug: string
slug: string,
shortened = false
): Promise<News> {
const raw = await fs.readFile(
path.join(NEWS_PATH, year, term, `${slug}.md`),
"utf-8"
);
const { content, data: metadata } = matter(raw);
const { content: rawContent, data: metadata } = matter(raw);
const slugDate = slug.split("-").slice(0, 3).join("-");
const content: string = shortened

Nitpick: We could probably use a ternary operator here instead of an if statement. This is mostly my opinion, but in general I feel like if statements are preferable for conditional behaviour, whereas ternary operators are preferable for conditional assignment.

An even nitpickier nitpick: It might be nice to name this variable content, since it might not always be truncated. To make this work, we could change the name of the variable on line 77 to rawContent.

Nitpick: We could probably use a ternary operator here instead of an if statement. This is mostly my opinion, but in general I feel like if statements are preferable for conditional behaviour, whereas ternary operators are preferable for conditional assignment. An even nitpickier nitpick: It might be nice to name this variable `content`, since it might not always be truncated. To make this work, we could change the name of the variable on line 77 to `rawContent`.
? truncateMarkdown(rawContent, {
limit: 150,
ellipsis: true,
})
: rawContent;
return {
content: await serialize(content),
@ -71,6 +90,7 @@ export async function getNewsBySlug(
date: getLocalDateFromEST(
parse(metadata.date, DATE_FORMAT, new Date())
).toString(),
permalink: `/news/${year}/${term}/${slugDate}`,
} as Metadata,
};
}
@ -91,7 +111,9 @@ export async function getRecentNews(): Promise<News[]> {
try {
const newsInTerm = await getNewsByTerm(year, TERMS[term]);
return await Promise.all(
newsInTerm.map((slug) => getNewsBySlug(year, TERMS[term], slug))
newsInTerm.map((slug) => {
return getNewsBySlug(year, TERMS[term], slug, true);
})
);
} catch (error) {
return [];

11
package-lock.json generated
View File

@ -16,6 +16,7 @@
"fs-extra": "^10.0.0",
"image-size": "^1.0.0",
"ldapts": "^3.1.0",
"markdown-truncate": "^1.0.4",
"next": "11.0.1",
"next-mdx-remote": "3.0.4",
"prettier": "^2.3.0",
@ -4719,6 +4720,11 @@
"url": "https://github.com/sponsors/wooorm"
}
},
"node_modules/markdown-truncate": {
"version": "1.0.4",
"resolved": "https://registry.npmjs.org/markdown-truncate/-/markdown-truncate-1.0.4.tgz",
"integrity": "sha512-sojm7PWqbgIfUoSVyKyyUN3glbwEgfXqL75HYvGjBHQuCkNaEHglyYt3biEIZG81H/CxhTtf2DEu4tLGWoK65Q=="
},
"node_modules/md5.js": {
"version": "1.3.5",
"resolved": "https://registry.npmjs.org/md5.js/-/md5.js-1.3.5.tgz",
@ -11576,6 +11582,11 @@
"repeat-string": "^1.0.0"
}
},
"markdown-truncate": {
"version": "1.0.4",
"resolved": "https://registry.npmjs.org/markdown-truncate/-/markdown-truncate-1.0.4.tgz",
"integrity": "sha512-sojm7PWqbgIfUoSVyKyyUN3glbwEgfXqL75HYvGjBHQuCkNaEHglyYt3biEIZG81H/CxhTtf2DEu4tLGWoK65Q=="
},
"md5.js": {
"version": "1.3.5",
"resolved": "https://registry.npmjs.org/md5.js/-/md5.js-1.3.5.tgz",

View File

@ -24,9 +24,10 @@
"@squoosh/lib": "^0.4.0",
"date-fns": "^2.11.1",
"date-fns-tz": "^1.1.6",
"ldapts": "^3.1.0",
"fs-extra": "^10.0.0",
"image-size": "^1.0.0",
"ldapts": "^3.1.0",
"markdown-truncate": "^1.0.4",
"next": "11.0.1",
"next-mdx-remote": "3.0.4",
"prettier": "^2.3.0",

View File

@ -20,7 +20,7 @@ import styles from "./index.module.css";
interface Props {
moreEvents: boolean; // true if there are more than 2 upcoming events
events: Event[]; // array of 0 - 2 events
news: News;
news: News[]; // array of 3 news items
}
export default function Home(props: Props) {
@ -86,14 +86,17 @@ export default function Home(props: Props) {
See past news <Link href="/news/archive">here</Link>
</p>
<hr className={styles.cardsDividingLine} />
{
<NewsCard
{...props.news.metadata}
date={new Date(props.news.metadata.date)}
>
<MDXRemote {...props.news.content} />
</NewsCard>
}
{props.news.length > 0
? props.news.map((news, idx) => (
<NewsCard
{...news.metadata}
date={new Date(news.metadata.date)}
key={`${news.metadata.date.toString()}${idx}`}

On the off chance that multiple news items have the same date, maybe we can make the key news.metadata.date.toString() + idx or something like that?

On the off chance that multiple news items have the same date, maybe we can make the key `news.metadata.date.toString() + idx` or something like that?
>
<MDXRemote {...news.content} />
</NewsCard>
))
: null}
</section>
</div>
</div>
@ -116,7 +119,7 @@ export const getStaticProps: GetStaticProps<Props> = async () => {
props: {
moreEvents: upcomingEvents.length > 2,
events: upcomingEvents.slice(0, 2),
news: recentNews[0],
news: recentNews.slice(0, 3),
},
};
};

View File

@ -0,0 +1,8 @@
.page {
padding-bottom: calc(30rem / 16);
}
.page > h1 {
padding-bottom: calc(16rem / 16);
border-bottom: calc(1rem / 16) solid var(--primary-heading);
}

View File

@ -0,0 +1,108 @@
import { ParsedUrlQuery } from "querystring";
import { GetStaticPaths, GetStaticProps } from "next";
import { MDXRemote } from "next-mdx-remote";
import React from "react";
import { NewsCard } from "@/components/NewsCard";
import {
ShapesConfig,
defaultGetShapesConfig,
GetShapesConfig,
} from "@/components/ShapesBackground";
import { Title } from "@/components/Title";
import {
getNewsBySlug,
getNewsByTerm,
getNewsTermsByYear,
getNewsDateByTerm,
getNewsYears,
News,
} from "@/lib/news";
import { Term } from "@/utils";
import styles from "./[date].module.css";
interface Props {
year: string;
term: Term;
news: News[];
}
export default function DateNews({ news }: Props) {
const date = new Date(news[0].metadata.date).toLocaleDateString("en-US", {
year: "numeric",
month: "long",
day: "numeric",
});
return (
<div className={styles.page}>
<Title>{["News", `${date}`]}</Title>
<h1>News: {date}</h1>
{news.map(({ content, metadata }, idx) => (
Review

Is the span here still necessary?

Is the `span` here still necessary?
<NewsCard
key={idx}
{...metadata}
date={new Date(metadata.date)}
fit={true}
>
<MDXRemote {...content} />
Review

It looks like we're passing fit={true} to the NewsCard, however the "fit" styling is declared on the MiniNewsCard. Is this intentional?

It looks like we're passing `fit={true}` to the `NewsCard`, however the "fit" styling is declared on the `MiniNewsCard`. Is this intentional?
Review

The "fit" style is also declared in NewsCard. I just copied the code from MiniNewsCard. The "fit" style is still important to keep the formatting of the NewsCard though. Since it's a duplicate code, do you think we need to change the way we include the stylesheet or no?

The "fit" style is also declared in `NewsCard`. I just copied the code from `MiniNewsCard`. The "fit" style is still important to keep the formatting of the `NewsCard` though. Since it's a duplicate code, do you think we need to change the way we include the stylesheet or no?
Review

Ah, I think what's happening is we're using the NewsCard component on both the Home page and on the New Archive pages - on the Home page we don't want the "fit" styles, however on the Archive pages we want them. Perhaps we can leave a quick comment in the NewsCard component explaining the purpose of the "fit" styles?

Ah, I think what's happening is we're using the `NewsCard` component on both the Home page and on the New Archive pages - on the Home page we don't want the "fit" styles, however on the Archive pages we want them. Perhaps we can leave a quick comment in the `NewsCard` component explaining the purpose of the "fit" styles?
</NewsCard>
))}
</div>
);
}
DateNews.getShapesConfig = ((width, height) => {
return window.innerWidth <= 768
? ({} as ShapesConfig)
: defaultGetShapesConfig(width, height);
}) as GetShapesConfig;
export const getStaticProps: GetStaticProps<Props, Params> = async (
context
) => {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const { year, term, date } = context.params!;
const slugs = (await getNewsByTerm(year, term)).filter((slug) =>
slug.includes(date)
);
const news = await Promise.all(
slugs.map((slug) => getNewsBySlug(year, term, slug))
);
// Reverse so that we are displaying the most recent news
// of term first
return { props: { year, term, news: news.reverse() } };
Review

Is there a reason why we want to console log here? If it's just for debugging/testing purposes, we should probably get rid of it before merging. 🙂

Is there a reason why we want to console log here? If it's just for debugging/testing purposes, we should probably get rid of it before merging. 🙂
};
interface Params extends ParsedUrlQuery {
year: string;
term: Term;
date: string;
}
export const getStaticPaths: GetStaticPaths<Params> = async () => {
const years = await getNewsYears();
const terms = await Promise.all(
years.map(async (year) => {
const termsInYear = await getNewsTermsByYear(year);
return termsInYear.map((term) => ({ year, term }));
})
);
const dates = await Promise.all(
terms.map(async (termInYear) => {
const datesInTerm: Params[] = [];
for (const { year, term } of termInYear) {
const dates = await getNewsDateByTerm(year, term);
dates.map((date) => datesInTerm.push({ year, term, date }));
}
return datesInTerm.flat();
})
);
return {
paths: dates.flat().map((params) => ({ params })),
fallback: false,
};
};

7
types.d.ts vendored
View File

@ -16,3 +16,10 @@ declare module "*.md" {
export default ReactComponent;
}
declare module "markdown-truncate" {
export default function truncateMarkdown(
inputText: string,
options: { limit: number; ellipsis: boolean }
): string;
}