React-박재현-sprint5 #53

doctor-taco · 2025-01-20T07:24:40Z

요구사항

기본

중고마켓

중고마켓 페이지 주소는 “/items” 입니다.
페이지 주소가 “/items” 일때 상단네비게이션바의 '중고마켓' 버튼의 색상은 “3692FF”입니다.
상단 네비게이션 바는 이전 미션에서 구현한 랜딩 페이지와 동일한 스타일로 만들어 주세요.
상품 데이터 정보는 https://panda-market-api.vercel.app/docs/#/ 에 명세된 GET 메소드 “/products” 를 사용해주세요.
'상품 등록하기' 버튼을 누르면 “/additem” 로 이동합니다. ( 빈 페이지 )
전체 상품에서 드롭 다운으로 “최신 순” 또는 “좋아요 순”을 선택해서 정렬을 할 수 있습니다.

중고마켓 반응형

베스트 상품
- Desktop : 4개 보이기
- Tablet : 2개 보이기
- Mobile : 1개 보이기
전체 상품
- Desktop : 12개 보이기 (Figma 디자인 대로 10개로 세팅함)
- Tablet : 6개 보이기
- Mobile : 4개 보이기

심화

페이지 네이션 기능을 구현합니다.

주요 변경사항

items 페이지 구현
페이지네이션 및 반응형 pagesize 가변 리스트 구현

스크린샷

멘토에게

셀프 코드 리뷰를 통해 질문 이어가겠습니다.

[Fix] delete merged branch github action

Eliminate unnecessary files and code lines, and apply a favicon and an image for the OG meta tag

Add a rendering function for the best and recent product list into the Items page after the conversion of the previous HTML project to the React project

… yet

…ed sort order option

Change the <Link> tags of hyperlinks "자유게시판" and "중고마켓" to <a> tags and adjust their styles correctly.

Convert the button "상품 등록하기" to a Link tag and input the URL. And also,convert A tags of the "자유게시판" and "중고마켓" to Link tags.

kiJu2 · 2025-01-22T01:32:44Z

스프리트 미션 하시느라 수고 많으셨어요.
학습에 도움 되실 수 있게 꼼꼼히 리뷰 하도록 해보겠습니다. 😊

kiJu2 · 2025-01-22T01:56:23Z

src/components/AllItems.js

+        setLoading(true);
+        setError(null);


굿굿 ~! loading과 error도 상태로 나타내셨군요 !

좋습니다. try의 시작부와 finally까지. 비동기를 나타내는 사용자 경험을 잘 고려하셨네요 👍👍

kiJu2 · 2025-01-22T02:03:18Z

src/components/AllItems.js

+        let pageSize;
+
+        switch (deviceWidth) {
+          case "desktop":
+            pageSize = 10;
+            break;
+          case "tablet":
+            pageSize = 6;
+            break;
+          case "mobile":
+            pageSize = 4;
+            break;
+          default:
+            pageSize = 10;
+        }


(제안/선택) 다음과 같이 매핑을 통해 나타낼 수 있습니다:

Suggested change

let pageSize;

switch (deviceWidth) {

case "desktop":

pageSize = 10;

break;

case "tablet":

pageSize = 6;

break;

case "mobile":

pageSize = 4;

break;

default:

pageSize = 10;

}

const pageSize = {

desktop: 10,

pageSize: 6,

mobile: 4,

}[deviceWidth] || 10;

코드가 간결해지며 선언형으로 작성할 수 있다는 장점이 있습니다 😊

kiJu2 · 2025-01-22T02:06:57Z

src/components/AllItems.js

+            pageSize = 10;
+            break;
+          case "tablet":
+            pageSize = 6;
+            break;
+          case "mobile":
+            pageSize = 4;
+            break;
+          default:
+            pageSize = 10;
+        }
+
+        const { list, totalCount } = await getProducts({
+          page: currentPage,
+          pageSize,
+          orderBy: sortOrder,
+        });
+
+        setProducts(list);
+        setTotalPages(Math.ceil(totalCount / 10));


(제안) 10, 6, 4 등의 값을 상수로 정의해두는 것도 방법입니다:

const PAGE_SIZES = { desktop: 10, tablet: 6, mobile: 4, };

kiJu2 · 2025-01-22T02:10:33Z

src/components/AllItems.js

+      } catch (e) {
+        setError(e.message);
+      } finally {
+        setLoading(false);
+      }


굿굿 ~! 마무리가 좋군요 ! 😊

다만, 서버로 부터 받은 error의 message가 없을 수도 있으니 예외처리를 해두는 것도 방법입니다 !:

Suggested change

} catch (e) {

setError(e.message);

} finally {

setLoading(false);

}

} catch (e) {

if (e.message) setError(e.message);

else setError("알 수 없는 에러입니다.");

} finally {

setLoading(false);

}

kiJu2 · 2025-01-22T02:11:00Z

src/components/AllItems.js

+  if (loading) return <h2 className="allProdListMsg">리스트 가져오는 중...</h2>;
+  if (error) return <h2 className="allProdListMsg">에러: {error}</h2>;


크으 ~ 로딩과 에러 UI도 만드셨군요 👍

게다가 조기에 return을 해주어 해당 라인 이후부터는 loadign과 error에 대해서 신경쓰지 않기에 가독성 또한 좋을 것으로 기대됩니다 👍👍

kiJu2 · 2025-01-22T02:20:00Z

src/components/AllItems.js

+  function getPageNums(currentPage, totalPages) {
+    let pagination1stBtn = currentPage - 2;
+    let pagination5thBtn = currentPage + 2;


5, 2의 숫자는 어떤 의미인지 파악하기 어렵기에 가독성이 저하될 수 있어요

다음과 같이 상수 별칭을 추가하면 어떨까요? 😊:

Suggested change

function getPageNums(currentPage, totalPages) {

let pagination1stBtn = currentPage - 2;

let pagination5thBtn = currentPage + 2;

function getPageNums(currentPage, totalPages) {

const DISPLAYED_PAGE_COUNT = 5;

const SIDE_BUTTONS = Math.floor(DISPLAYED_PAGE_COUNT / 2);

let paginationFirstBtn = currentPage - SIDE_BUTTONS;

let paginationLastBtn = currentPage + SIDE_BUTTONS;

kiJu2 · 2025-01-22T02:24:09Z

src/components/AllItems.js

+    }
+  };
+
+  function getPageNums(currentPage, totalPages) {


(제안) 해당 함수는 컴포넌트의 자원을 사용하고 있지 않습니다 !

getPageNums 함수는 입력값만으로 동작을 정의하고, 외부 상태와 상호작용하지 않으며 항상 일관된 타입을 반환하고 있습니다. 잘 설계하셨어요. 😊

컴포넌트의 상태에 의존하지 않으며 독립적으로 충분히 기능을 수행할 수 있습니다. 오히려 컴포넌트 내부에 있으면 리렌더링 시 불필요한 재선언이 될 수 있습니다. 또한 이렇게 컴포넌트의 자원(state, props)을 사용하지 않는 함수들을 하나씩 제거한다면 컴포넌트의 가독성 또한 좋아질거예요 !

해당 함수를 컴포넌트 바깥에 정의하는게 어떨까요?

kiJu2 · 2025-01-22T02:27:58Z

src/components/functions.js

(의견) components 내부에 컴포넌트가 아닌 파일들이 존재하는군요? 🤔

해당 함수들이 필요하다면 src/utils/validations.js와 같은 파일을 따로 구분하는건 어떨까요?

kiJu2 · 2025-01-22T02:29:52Z

src/components/functions.js

+export function checkNickname() {
+  const nicknameInput = document.getElementsByClassName("sign-nickname")[0];
+  const nicknameError = document.getElementsByClassName(
+    "sign-nickname-error"
+  )[0];
+
+  nicknameInput.classList.remove("error");
+
+  if (!nicknameInput.value) {
+    nicknameInput.style.border = "1px solid red";
+    nicknameError.textContent = "닉네임을 입력해주세요.";
+  } else if (nicknameInput.value.length < 3) {
+    nicknameInput.style.border = "1px solid red";
+    nicknameError.textContent = "닉네임을 3자 이상 입력해주세요.";
+  } else {
+    nicknameInput.style.border = "";
+    nicknameError.textContent = "";
+  }
+  toggleLoginBtn();
+  toggleSignupBtn();
+}


리액트에서 직접 DOM을 조작하고 있군요. (이어서)

kiJu2 · 2025-01-22T02:33:34Z

src/pages/SignupPage.js

+            <input
+              type="text"
+              name="signup-nickname"
+              id="signup-nickname"
+              placeholder="닉네임을 입력해주세요"
+              minLength="3"
+              maxLength="20"
+              required
+              className="sign-nickname"
+              onBlur={checkNickname}
+            />


직접 DOM을 조작하게 되면 다음과 같은 문제가 발생할 수 있습니다:

리액트의 선언적 UI 관리 방식을 무시

리액트는 상태 기반으로 UI를 업데이트하는 선언적 프로그래밍 방식을 따릅니다. 리액트에서 상태는 컴포넌트가 렌더링될 때 UI가 자동으로 업데이트되도록 관리합니다. 하지만, DOM을 직접 조작하는 방식은 리액트의 이런 선언적 프로그래밍 방식을 따르지 않고, 명령형 프로그래밍 방식으로 동작하게 됩니다. 즉, 개발자가 명령을 내려 DOM을 직접 업데이트하는 것이므로, 리액트가 제공하는 자동화된 렌더링 시스템을 활용하지 못하게 됩니다.

상태와 UI가 일관성을 잃을 수 있음

리액트는 상태를 기반으로 UI를 렌더링하고 관리합니다. 상태에 의존하지 않고 DOM에서 직접 값을 가져와 처리하는 방식은, 리액트가 상태를 제대로 추적하지 못하게 합니다. 이는 상태와 UI 간의 일관성을 깨뜨릴 수 있는 위험을 초래합니다. 예를 들어, 입력 필드와 count 값이 서로 맞지 않거나, UI 업데이트가 제대로 반영되지 않는 상황이 발생할 수 있습니다.

리렌더링 시 값이 초기화될 수 있음

리액트 컴포넌트가 리렌더링될 때, DOM 요소의 상태가 초기화될 수 있습니다. 만약 상태로 값을 관리하지 않고 DOM에만 의존한다면, 리렌더링이 일어날 때마다 입력 필드에 입력된 값이 사라지거나 초기화될 수 있습니다. 리액트는 상태를 통해 렌더링 중에도 값을 유지하므로, 상태를 사용하지 않는 코드는 리렌더링 시에 값 손실이 발생할 수 있습니다.

유지보수 및 테스트의 어려움

DOM 접근 방식은 리액트의 상태 관리 시스템을 우회하기 때문에, 리액트의 특성인 유지보수성과 테스트 용이성이 떨어질 수 있습니다. 리액트는 컴포넌트의 상태와 동작을 독립적으로 관리할 수 있게 하여, 테스트가 용이한 구조를 제공합니다. 하지만 DOM을 직접 조작하면 컴포넌트의 상태와 UI를 추적하기 어려워져, 테스트와 유지보수가 복잡해질 수 있습니다.

출처

상태로 관리해볼 것을 권장드립니다 😉

그럼에도 불구하고 불가피하게 DOM을 직접 조작해야 한다면 ref를 사용해볼 수 있습니다:

Ref

kiJu2 · 2025-01-22T02:37:46Z

수고하셨습니다 재현님 !
DOM에 직접 접근하는 방식보다 가상돔과 실제돔을 일치시키고 UI와 상태가 일관될 수 있도록 React에서 제공하는 상태를 활용해볼 것을 권장드립니다 ! 😊
금방 금방 적응하시는게 느껴지시는군요 ! 자바스크립트를 이미 잘 다루고 계시니 리액트도 금방 잘 활용하실 것으로 기대가 됩니다 ! 👍👍

궁금하신 사항이 있으시다면 DM으로 편하게 질문주세요 ~!

hanseulhee and others added 15 commits October 10, 2023 14:15

reset

b2e37bd

Merge branch 'codeit-bootcamp-frontend:main' into main

6f8bbb0

fix: 머지 후 브랜치 삭제 github action 수정

e11e25f

env: workflows 폴더로 이동

212e864

Merge pull request #237 from hanseulhee/fix-github-actions

4dc5dd0

[Fix] delete merged branch github action

Merge branch 'codeit-bootcamp-frontend:React-박재현' into React-박재현

31c10de

chore: Organize the app before development

b658484

Eliminate unnecessary files and code lines, and apply a favicon and an image for the OG meta tag

feat: Add the best and recent product list

76437b4

Add a rendering function for the best and recent product list into the Items page after the conversion of the previous HTML project to the React project

feat: Add pagination for All Items list on the Items page

7ec2d7c

style: Change the arrow of the select for the All Items sorting order

521f8a3

feat: Apply media queries on the Items page

4bad6f9

feat: Give URLs to li elements in the header nav that don't have URLs…

39720b1

… yet

feat: Add a feature that gets an ordered list according to the select…

165a126

…ed sort order option

style: Adjust text style on the hyperlinks "자유게시판" and "중고마켓"

b6e97a4

Change the <Link> tags of hyperlinks "자유게시판" and "중고마켓" to <a> tags and adjust their styles correctly.

style: Add hyperlink into the button "상품 등록하기"

7c85f31

Convert the button "상품 등록하기" to a Link tag and input the URL. And also,convert A tags of the "자유게시판" and "중고마켓" to Link tags.

doctor-taco requested a review from beejugy January 20, 2025 07:26

doctor-taco added the 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. label Jan 20, 2025

kiJu2 self-requested a review January 22, 2025 01:32

kiJu2 approved these changes Jan 22, 2025

View reviewed changes

kiJu2 merged commit 7d149da into codeit-bootcamp-frontend:React-박재현 Jan 22, 2025

		if (loading) return <h2 className="allProdListMsg">리스트 가져오는 중...</h2>;
		if (error) return <h2 className="allProdListMsg">에러: {error}</h2>;

React-박재현-sprint5 #53

React-박재현-sprint5 #53

Uh oh!

Conversation

doctor-taco commented Jan 20, 2025

요구사항

기본

심화

주요 변경사항

스크린샷

멘토에게

Uh oh!

kiJu2 commented Jan 22, 2025

Uh oh!

Choose a reason for hiding this comment

굿굿 ~! loading과 error도 상태로 나타내셨군요 !

Uh oh!

Choose a reason for hiding this comment

(제안/선택) 다음과 같이 매핑을 통해 나타낼 수 있습니다:

Uh oh!

Choose a reason for hiding this comment

(제안) 10, 6, 4 등의 값을 상수로 정의해두는 것도 방법입니다:

Uh oh!

Choose a reason for hiding this comment

굿굿 ~! 마무리가 좋군요 ! 😊

Uh oh!

Choose a reason for hiding this comment

크으 ~ 로딩과 에러 UI도 만드셨군요 👍

Uh oh!

Choose a reason for hiding this comment

5, 2의 숫자는 어떤 의미인지 파악하기 어렵기에 가독성이 저하될 수 있어요

Uh oh!

Choose a reason for hiding this comment

(제안) 해당 함수는 컴포넌트의 자원을 사용하고 있지 않습니다 !

Uh oh!

Choose a reason for hiding this comment

(의견) components 내부에 컴포넌트가 아닌 파일들이 존재하는군요? 🤔

Uh oh!

Choose a reason for hiding this comment

리액트에서 직접 DOM을 조작하고 있군요. (이어서)

Uh oh!

Choose a reason for hiding this comment

직접 DOM을 조작하게 되면 다음과 같은 문제가 발생할 수 있습니다:

Uh oh!

kiJu2 Jan 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kiJu2 commented Jan 22, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

굿굿 ~! `loading`과 `error`도 상태로 나타내셨군요 !

(제안) `10`, `6`, `4` 등의 값을 상수로 정의해두는 것도 방법입니다:

`5`, `2`의 숫자는 어떤 의미인지 파악하기 어렵기에 가독성이 저하될 수 있어요

(의견) `components` 내부에 컴포넌트가 아닌 파일들이 존재하는군요? 🤔

kiJu2 Jan 22, 2025 •

edited

Loading