[FE] feat: 에러, 로딩 페이지 구현 및 적용

[soosoo22]

• resolves [FE] 에러, 로딩 페이지 구현 및 라우트에 errorElement 적용 #146

---

🚀 어떤 기능을 구현했나요 ?

• 에러, 로딩 페이지 구현 및 라우트에 errorElement 적용

🔥 어떻게 해결했나요 ?

에러 페이지

• 에러 로고, 문구
• 새로고침, 홈 이동 버튼
  • 새로고침 클릭 시, 리로드
  • 홈 이동 버튼 클릭 시, 홈으로 이동 ⇒ 아직 홈 페이지가 만들어지지 않아서 일단 임시로 ‘/’ 로 이동

const handleGoHome = () => {
    navigate('/'); // TODO: 홈 페이지 경로가 결정되면 변경 필요
  };

• 라우트에 errorElement 적용

const router = createBrowserRouter([
  {
    path: '/',
    element: <App />,
    errorElement: <ErrorPage />,
    children: [
      {
        path: 'user',
        element: <div>user</div>,
      },
      {
        path: 'user/review-writing',
        element: <ReviewWritingPage />,
      },
      {
        path: 'user/review-preview-list',
        element: <ReviewPreviewListPage />,
      },
      {
        path: 'user/detailed-review/:id',
        element: <DetailedReviewPage />,
      },
    ],
  },
]);

로딩 페이지 구현

• 로딩 텍스트, 로딩바
• 각 페이지에서 로딩 페이지 적용

📝 어떤 부분에 집중해서 리뷰해야 할까요?

• ErrorPage와 App에서 중복되는 레이아웃 코드가 존재해요...😭
• 로딩바를 페이지의 중앙에 위치시키기 위해서 현재는 height: calc(100vh - 21rem);을 사용하고 있는데, 더 좋은 방법이 있을까요?

📚 참고 자료, 할 말

• 리뷰미 화이팅😘

[BadaHertz52]

alt는 사진에 대한 자세한 설명이여야해요.
alt을 특정 아이콘에 대한 내용으로 하고 싶다면 icon 내용을 props로 받을 수 있고,
특정 하기 힘들다면 빈문자열을 사용하는 것을 추천드려요

[ImxYJL]

바다 말대로 alt를 props로 받으면 좋을 것 같습니다. 별도로 alt를 전달하지 않으면 빈 문자열('')을 주는 방식으로 개선해보면 좋을 것 같아요!!

[soosoo22]

너무나 동의하는 바입니다! 제가 작성한 코드인데도... alt="아이콘" 뭘 설명하는지 모르겠네요...^^

[BadaHertz52]

px??? 우리는 rem을 사용하는데 쑤쑤...🥺

[soosoo22]

px을 좋아하나봐요....^^
바로~~ 고칠게요~~ 예리하세요~~

[BadaHertz52]

리뷰 상세페이지에 LoadingPage를 적용시켰군요!
develop에 머지 후 리뷰 상세페이지의 suspense에 적용해볼게요. 😝

[BadaHertz52]

svg를 import 할때 Icon을 뒤에 붙이기로 했어요.

import HomeIcon from '@/assets/home.svg'

[soosoo22]

어머어머 고마워요~
코드컨벤션에도 기재되어 있는 내용인데.... 간과했네요!

[BadaHertz52]

ButtonContainer 부분이 반복되어서, 버튼의 icon,text,buttonType,click을 가지는 객체가 요소인 배열을 만들어서 활용해봐도 좋을 것 같아요.
click 핸들러에서 버튼의 name에 따라 다른 함수를 실행시킬 수 있도록 해도 되고요.

하나의 제안일 뿐이니 반영해야한다는 의견은 아니에요

[ImxYJL]

저도 리팩토링한다면 객체 배열을 만들 것 같아요~!

[soosoo22]

반복되는 코드들이 많아서 객체 배열을 만들고 map 을 이용해서 렌더링하는 게 더 효율적이겠네요~
고마워요~~😘

  const buttons = [
    {
      buttonType: 'primary' as ButtonType,
      text: '새로고침하기',
      image: ReloadIcon,
      imageDescription: '새로고침 이미지',
      onClick: handleReload,
    },
    {
      buttonType: 'secondary' as ButtonType,
      text: '홈으로 이동하기',
      image: HomeIcon,
      imageDescription: '홈 이미지',
      onClick: handleGoHome,
    },
  ];

[BadaHertz52]

21rem이 무엇에서 왔는지 알 수 있을까요?

[BadaHertz52]

key에 카멜이 아니라 스네이크 케이스를 쓴 이유가 있을까요?
이전 상수 변수들의 키값 네이밍 형식과는 안맞아서요.

[BadaHertz52]

ErrorPage가 route의 errorElement에 사용되는데 이 경우는 서버와의 통신 불안이 아닌, 찾는 페이지가 없는 오류 아닐까요?

[soosoo22]

key에 카멜이 아니라 스네이크 케이스를 쓴 이유가 있을까요? 이전 상수 변수들의 키값 네이밍 형식과는 안맞아서요.

어머어머... 이런... 저는.. 코드 컨벤션도 지키지 못하는 팀원이네요😅 수정할게요~~
커밋 주소 : 930b117

[BadaHertz52]

error, loading 페이지를 구현하느라 고생했어요. 쑤쑤 👏👏👏👏
수정해야하는 부분들 코멘트로 남겨놨어요 확인 부탁드려요

[ImxYJL]

새 페이지 만들고 적용하느라 수고 많았어요~~

[ImxYJL]

바다 말대로 alt를 props로 받으면 좋을 것 같습니다. 별도로 alt를 전달하지 않으면 빈 문자열('')을 주는 방식으로 개선해보면 좋을 것 같아요!!

[ImxYJL]

저도 리팩토링한다면 객체 배열을 만들 것 같아요~!

[ImxYJL]

TODO 주석 좋아요!

[chysis]

코드 잘 읽었어요 쑤쑤~ 변경 사항이 꽤 많았는데 고생 많았어요.
몇 가지 코멘트를 남겼으니 확인 부탁드려요👍👍

[chysis]

버튼에 텍스트 뿐 아니라 아이콘까지 추가돼서 이와 같이 코드를 작성하신 것 같아요. 추후에 텍스트, 아이콘 이외의 요소들이 더 들어올 상황이 생길 수도 있을 것 같아요. 이런 경우에 optional로 props를 계속 받기보다는, 공통 버튼 컴포넌트는 Children만을 받아서 그대로 출력해주는 것은 어떨까요?

그렇게 되면 버튼을 사용하는 쪽에서 필요한 요소들을 유연하게 넣을 수 있고, 현재 코드에서의 icon도 그 종류가 특정될 수 있을 것 같아요.

[soosoo22]

너무 동의합니다!
에프이가 말했던 것처럼 추가적으로 요소들이 더 들어오면 아래 코드처럼 옵셔널로 props를 받는 상황이 많아질텐데.... children을 받는 방법이 더 효율적일 것 같네요! 고마워요~~😊

// 안 좋은 코드
interface ButtonProps extends React.ButtonHTMLAttributes<HTMLButtonElement> {
  buttonType: ButtonType;
  text: string;
  image?: string;
  imageDescription?: string;
}

[chysis]

초기값을 false로 지정해주고 있어서 타입 추론이 자동으로 이루어질 것 같아요. boolean을 명시한 쑤쑤만의 이유가 있을까요?

[soosoo22]

isLoading에 타입을 boolean으로 더 명확하게 지정하고 싶었던 것 같아요

[chysis]

하나의 컨테이너에 두 개의 버튼이 들어가지 않고 각각을 컨테이너로 감싼 이유가 있는지 궁금해요!

[soosoo22]

아! 지금보니 쓸모없는 컨테이너네요!
아이콘이 버튼 안에 들어가야 하는데 처음에는 하나의 컨테이너에 아이콘과 버튼 이렇게 나란히 둬서 그랬던 것 같아요

[chysis]

useNavigate에서 제공하는 navigate(0)를 사용하면 현재 페이지 새로고침이 가능해요. navigate를 사용하고 있어서 제안드려요😊

[chysis]

사소한 부분이지만😅, reload를 하나의 단어로 보지 않고 ReLoad로 표기한 이유가 있을까요?

[soosoo22]

useNavigate에서 제공하는 navigate(0)를 사용하면 현재 페이지 새로고침이 가능해요. navigate를 사용하고 있어서 제안드려요😊

아하! 고마워요! 이미 홈으로 이동할 때, navigate를 사용하고 있어서 새로고침도 navigate를 사용하면 좋을 것 같네요!

[soosoo22]

사소한 부분이지만😅, reload를 하나의 단어로 보지 않고 ReLoad로 표기한 이유가 있을까요?

아하.....re+load 약간 이런 느낌이라고 생각해서 ReLoad라고 표기했던 것 같아요😭 수정할게요~

[BadaHertz52]

쑤쑤 로딩,에러 페이지 만드느라고 고생했어요.👏👏👏🎉

[chysis]

굉장히 중요한 요소였고, 그만큼 신경쓸 게 많았을 텐데 고생했어요. 우선 Approve할게요! 제안드린 내용은 추후 반영해주세요~

[ImxYJL]

수고 많았어요~~~~~~