-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[#17] apply review page #33
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 👍 👍 👍 👍 👍 👍 👍 👍
궁금하고 권장되는 변경 사항만 몇 개 달아봤어요!
정적 페이지를 생성하시는 것이 처음이면 많이 어려우셨을 텐데 고생하셨어요 💯 🥇
src/components/Review/Introduce.tsx
Outdated
<div css={bgWrapper}> | ||
<div css={sizeWrapper}> | ||
<div css={colorWrapper}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 부분에 대해 main, section, article 등의 시맨틱 태그를 적절히 사용하면 괜찮지 않을까요!
src/components/Review/PostCard.tsx
Outdated
import { Link } from "@nextui-org/react"; | ||
import { PostMarkdown } from "util/getMarkdownBySlug"; | ||
|
||
export interface PostListItemProps extends PostMarkdown {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 파일에서는 이 인터페이스가 사용되고 있지 않은데, 여기서 선언한 이유가 있을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
착각했네용 😱
src/components/Review/PostCard.tsx
Outdated
function PostCard(props) { | ||
const post = props; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이렇게 바인딩하는 것보다 아래처럼 가져갈 수도 있을 거 같아요!!!
function PostCard(props) { | |
const post = props; | |
function PostCard(post) { |
src/components/Review/PostCard.tsx
Outdated
return ( | ||
<article css={cardWrapper}> | ||
<div css={contentWrapper}> | ||
<Link href={`/Review/${post?.slug}`}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<Link href={`/Review/${post?.slug}`}> | |
<Link href={`/Review/${post.slug}`}> |
slug가 있어야만 동작하는게 맞지 않을까요?
없는 경우가 있다면 따로 Link 태그가 아니도록 핸들링을 하면 될 거 같아요!
const file = await unified() | ||
.use(remarkParse) | ||
.use(remarkRehype) | ||
.use(rehypeSanitize) | ||
.use(rehypeStringify) | ||
.process(markdown); | ||
return file.toString(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 👍 👍 👍 👍
export const getPostBySlug = (slug: string, fields: string[]) => | ||
getMarkdownBySlug(postsDirectory, slug, fields) as PostMarkdown; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 함수가 하는 역할은 getMarkdownBySlug
에 postsDirectory
를 주입하는 것으로 보여요
그럼 getMarkdownBySlug
에서 기본적으로 사용하는 방향으로 하면 해당 함수는 필요없을 수도 있지 않을까요??
src/util/getMarkdownBySlug.ts
Outdated
if (field === "slug") { | ||
items[field] = realSlug; | ||
} | ||
|
||
if (field === "content") { | ||
items[field] = content; | ||
} | ||
|
||
if (field === "tags" && data[field]) { | ||
items[field] = data[field].split(""); | ||
} else if (data[field]) { | ||
items[field] = data[field]; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기는 switch 문을 사용하거나 else if
로 사용하는 것이 조금 더 읽기 쉬워질 거 같아요!
추가적으로 else
에 대해 에러 처리한다거나, undefined를 추가하는 형식으로도 접근할 수 있을 거 같아요!
package.json
Outdated
"framer-motion": "^6.3.2", | ||
"next": "^12.1.6", | ||
"gray-matter": "^4.0.3", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기 gray-matter
라이브러리는 마크다운 파일의 상단에 있는 카테고리를 반환하는 기능을 해요.
즉, 서버 사이드에서 (빌드 타임) 실행되기 때문에 devDependencies
로 내려도 될 거 같아요 👍
next.config.js
Outdated
webpack: (config) => { | ||
config.module.rules.push({ | ||
test: /\.md$/, | ||
use: 'raw-loader', | ||
}) | ||
return config | ||
}, | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 웹팩 설정은 무엇을 의미하나요?
서버 사이드에서만 마크다운 파일을 참조하기 때문에 로더는 필요 없지 않을까? 라고 생각되는데 경험을 공유해주시면 좋을 거 같아요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
그 점을 고려하지 못했네요 ...! 수정하는 게 맞을 것 같습니다 ㅎㅎ
설명
review page를 개발하였습니다.
중점적으로 봐줬으면 좋을 부분
처음해보는 부분이라 혹시 잘못된 곳이 있다면 알려주심 감사하겠습니다 ... 😵💫🥲
게시글은 후에 수정할 계획입니다