Skip to content
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

[6주차] 셰어마인드 과제 제출합니다. #15

Open
wants to merge 63 commits into
base: master
Choose a base branch
from

Conversation

kyuhho
Copy link

@kyuhho kyuhho commented Nov 17, 2023

서론

셰어마인드 팀입니다! 아쉽게도 무한스크롤 추가구현은 완료하지 못했습니다 🥲 일주일동안 수고하셨습니다😊😊

배포

Next-Netflix

필수 요건

  • 결과화면의 상세 페이지와 검색 페이지를 구현합니다.
    • 상세 페이지는 동적 라우팅을 이용해 구현합니다.
    • 검색 페이지는 실시간 키워드 검색으로 구현합니다.
  • Figma의 디자인을 그대로 구현합니다.
  • SSR을 적용해서 구현합니다.
  • Open api를 사용해서 데이터 패칭을 진행합니다.

Key Question

  • 정적 라우팅(Static Routing)/동적 라우팅(Dynamic Routing)이란?

    정적 라우팅은 고정된 페이지로 라우팅하는 것이다. 예를 들어 /home과 같이 고정된 라우팅을 의미한다.

    반대로 동적 라우팅은 가변적인 페이지로 라우팅하는 것이다. detail 페이지를 구현하는 것이 동적 라우팅에 해당된다.

    Next.js에서는 []로 시작하는 파일을 통해 동적 라우팅을 구현한다.

  • 성능 최적화를 위해 사용한 방법

    1. next.js에서 지원하는 image 컴포넌트 사용
image
  1. SSR을 고려하여 css 파일 사용
image

kyuhho and others added 30 commits November 8, 2023 21:38
kyuhho and others added 26 commits November 10, 2023 14:15
클라이언트 사이드렌더링을 최대한 올려주고 서버사이드를 활용하기위함
머지날린줄알았는데 안날렸네요
Copy link

@mod-siw mod-siw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이번 주차 과제도 수고 많으셨습니다!
저번 주차와 비교했을 때 SSR 방식을 위해 새롭게 바뀐 부분들이 눈에 잘 보였던 리뷰였습니다ㅎㅎ
스터디때 뵙겠습니다~!

className={[
styles.menuText,
pathname === "/home" ? styles.white : "",
].join(" ")}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

각 로고에 들어가는 설정이 중복되는 경우가 많아서, 이동 위치에 대한 정보를 배열로 선언한 다음 map 함수로 뿌려주면 코드 가독성이 더 좋아질 수 있을 것 같습니다!! 버튼 색 변경을 어떻게 하면 좋을까 고민했었는데, 이렇게 css 에 접근 하는 방식도 좋은 것 같습니다 :)

@@ -0,0 +1,64 @@
.imageBox {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

상세 페이지에서 overview text가 밑에도 있는 것 같아서 스크롤 해보려고 했는데 스크롤이 안되더라구요🥲 따로 스크롤 가능하게 설정해주시면 사용성이 더 좋아질 것 같습니다!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저도 스크롤 설정까진 생각 못했는데 저희도 적용해봐야겠어요...!!

onChange={handleOnChange}
/>

<DeleteIcon className={styles.deleteIcon} onClick={handleDelete} />
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setInput만 변경하는 경우에는

 onChange={(e) => setInput(e.target.value)}
onClick={() => setInput ("")} 

이렇게 바로 지정해줘도 좋을 것 같습니다!

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저도 함수로 구현했는데, 이렇게 바로 지정해줘도 좋을 것 같네요~! 배워갑니다 ㅎㅎ

<div className={styles.cardList}>
{data.map((item) => {
return (
<Link
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

검색 페이지에서도 아이템 누르면 해당 페이지로 이동하게 설정해주신 디테일 좋네요!!👍👍

export default function StyledComponentsRegistry({ children }) {
const [styledComponentsStyleSheet] = useState(() => new ServerStyleSheet());

useServerInsertedHTML(() => {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기 이 파일은 styled components를 SSR 방식에서 사용해보려 했던 시도인가요?! CSS 파일로 스타일 구현하신 줄 알았는데 마지막 부분 코드에 styled-components 관련 설정이 있어서 궁금합니다!

@@ -0,0 +1,19 @@
import axios from "@/api/axios";
import request from "@/api/request";
export const useFetchData = async (type) => {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

useFetchData.js 항목도 api를 불러오는 항목인 것처럼 보여요! api 폴더가 아닌 hooks 페이지로 따로 빼서 분류하신 이유가 있을까요?

RecommendSection,
} from "@/components/home";
import styled from "styled-components";
const Home = () => {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이건 그냥 개인적인 궁금증인데, 타입스크립트를 사용하지 않으시고 js를 사용하셨더라구요! 팀 내에서 이런 결정을 어떻게 내리셨는지가 궁금합니다! 저도 ts를 잘 사용하고 있는지에 대한 확신이 들지 않아서 '이렇게 쓰는 게 맞나..?' 하면서 쭉 써오고 있어서 그런지 해당 결정사항을 내리게 된 이유가 궁금해요🥲

Copy link

@silverain02 silverain02 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이번주도 고생하셨습니다~! SSR,CSR컴포넌트 나누어서 깔끔하게 구현하신 것 같아요!


import React from "react";
async function Page({ params }) {
const data = await useFetchDetailData(params.slug);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

data id를 path variable이 아닌 props로 받아오셨네요! 저는 path variable을 받아오려고 client 컴포넌트로 만들었는데 이런 식으로 하면 use client선언 없이 server컴포넌트로 만들 수 있겠네요 배워갑니다~

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저도 배워갑니다~

@@ -0,0 +1,35 @@
"use client";

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

home과 search는 CSR컴포넌트이고 slug로 되어있는 상세페이지만 SSR컴포넌트인데 이유가 있을까용?

<nav className={styles.navWrapper}>
<ul className={styles.menuList}>
<li className={styles.menuItem} onClick={() => router.push("/home")}>
<HomeLogo stroke={pathname === "/home" ? "#FFF" : "#8C8787"} />

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pathname감지해서 로고 색 변경하는 방식 좋습니다!!

<Link href={`/home/${item.id}`} key={item.id}>
<PreviewItem
src={
"https://image.tmdb.org/t/p/original" + item.backdrop_path

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"https://image.tmdb.org/t/p/original"요부분도 img baseURL로 설정해줄 수 있을 것 같아요!

overflow: scroll;
`;

const PreviewItem = styled.img`

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Next에서 성능 최적화를 위해 Image를 권장하고 있어서 Next Image를 사용해보시는 것도 추천드려요!

Copy link

@dhshin98 dhshin98 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이번주 과제도 수고하셨습니다~ 오랜만에 따로 선언된 css 파일을 본것 같아 반가웠습니다ㅎㅎ SSR 방식을 위해 이번주에 구현 방식을 바꾸신 것 같아 인상깊었습니다~~ 고생하셨습니다!

Comment on lines +11 to +12
<DetailSection data={data} imageUrl={imageUrl} />
<BottomNav />

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

동적 렌더링 할때 요소들을 이렇게 컴포넌트화 시키니까 가독성이 좋은 것 같네요!!

@@ -0,0 +1,11 @@
// api 요청 경로 모음

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

자주 사용하는 경로 따로 저장해놓는 것 좋은것 같습니다~

Comment on lines +27 to +31
.deleteIcon {
position: absolute;
top: 19px;
right: 21px;
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

delete Icon에 마우스 포인터 효과를 줘도 좋을 것 같다는 개인적인 의견 드립니다 ㅎㅎ!

onChange={handleOnChange}
/>

<DeleteIcon className={styles.deleteIcon} onClick={handleDelete} />

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저도 함수로 구현했는데, 이렇게 바로 지정해줘도 좋을 것 같네요~! 배워갑니다 ㅎㅎ

Copy link
Member

@leejin-rho leejin-rho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이번주 과제도 수고하셨습니다! 항상 생각하지 못한 부분까지 구현해주시는거 같아서 많이 배워갑니다~

import Image from "next/image";
import styles from "./DetailSection.module.css";
import PlayLogo from "public/assets/images/icon/play.svg";
function DetailSection({ data, imageUrl }) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

data를 props로 넘겨서 받아와서 사용하는 방법도 좋은것 같습니다~

@@ -0,0 +1,64 @@
.imageBox {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저도 스크롤 설정까진 생각 못했는데 저희도 적용해봐야겠어요...!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants