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

PR: path3 task1,task2,task3,task4 #2

Open
wants to merge 28 commits into
base: main
Choose a base branch
from
Open

PR: path3 task1,task2,task3,task4 #2

wants to merge 28 commits into from

Conversation

ijimlnosk
Copy link

task1

  • RHF, yup을 이용하여 유효성 검사
  • 가입하기 버튼 클릭 시 이전모든 단계에서 했던 데이터 alert에 보여짐
  • 뒤로가기 버튼 및 웹 뒤로가기 클릭 시 이전 폼데이터 유지
  • 유효성 검사 시 실시간으로 에러메시지 보여짐
  • 유요성 검사에 어긋나면 버튼 비활성화

task2

  • 보일러 템플릿 만들기
  • 폴더구조
📦boiler-template
 ┣ 📂public
 ┃ ┣ 📜favicon.ico
 ┃ ┣ 📜index.html
 ┃ ┣ 📜logo192.png
 ┃ ┣ 📜logo512.png
 ┃ ┣ 📜manifest.json
 ┃ ┗ 📜robots.txt
 ┣ 📂src
 ┃ ┣ 📂assets
 ┃ ┃ ┣ 📂fonts
 ┃ ┃ ┗ 📂images
 ┃ ┣ 📂components
 ┃ ┃ ┗ 📂common
 ┃ ┃ ┃ ┣ 📜box.jsx
 ┃ ┃ ┃ ┗ 📜spacer.jsx
 ┃ ┣ 📂constants
 ┃ ┃ ┗ 📂design-token
 ┃ ┃ ┃ ┣ 📜color.js
 ┃ ┃ ┃ ┣ 📜layout.js
 ┃ ┃ ┃ ┣ 📜size.js
 ┃ ┃ ┃ ┣ 📜spacing.js
 ┃ ┃ ┃ ┗ 📜tokens.js
 ┃ ┣ 📂features
 ┃ ┣ 📂hooks
 ┃ ┣ 📂libs
 ┃ ┃ ┣ 📂axios
 ┃ ┃ ┃ ┗ 📜axiosInstance.js
 ┃ ┃ ┗ 📂redux
 ┃ ┃ ┃ ┗ 📜store.js
 ┃ ┣ 📂page
 ┃ ┣ 📂styles
 ┃ ┃ ┗ 📜global-style.js
 ┃ ┣ 📂utils
 ┃ ┃ ┗ 📜getDateDifferenceDescription.js
 ┃ ┣ 📜App.css
 ┃ ┣ 📜App.js
 ┃ ┣ 📜App.test.js
 ┃ ┣ 📜index.css
 ┃ ┣ 📜index.js
 ┃ ┣ 📜logo.svg
 ┃ ┣ 📜reportWebVitals.js
 ┃ ┗ 📜setupTests.js
 ┣ 📜.eslintrc.json
 ┣ 📜.gitignore
 ┣ 📜.prettierrc.js
 ┣ 📜README.md
 ┣ 📜package-lock.json
 ┗ 📜package.json

task3

  • todo 추가 버튼 전역상태로 관리
  • input custom hook으로 생성

task4

  • pagination 컴포넌트 하나로 합침
  • user 확인 따로 분리
  • limit_page, limit_task 전역상태로 관리
  • axios-instance 생성
  • api 관리 파일 생성

@ijimlnosk ijimlnosk self-assigned this Mar 23, 2024
Copy link
Member

@Zero-1016 Zero-1016 left a comment

Choose a reason for hiding this comment

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

3주차도 고생하셨습니다.👍

남은 기간도 파이팅하시길 응원합니다.

@@ -0,0 +1,13 @@
module.exports = {
Copy link
Member

Choose a reason for hiding this comment

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

👍

};
export default Spacer;

const SpaceBox = styled.span`
Copy link
Member

Choose a reason for hiding this comment

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

span태그는 단순 텍스트나 텍스트에 관련된 마크업 등 구문 콘텐츠에 스타일이나 속성의 범위를 적용하기 위해 감싸주는 태그입니다. 이는 주로 시각적 스타일을 위한 장식용 태그로 사용됩니다.

SpaceBox라는 이름에 맞는 태그는 레이아웃을 만들거나 콘텐츠를 나누는(division) 컨테이너로 사용하는 div 태그가 어울리지 않을까? 라고 생각이 드네요

@@ -0,0 +1,14 @@
import styled from "styled-components";

const Spacer = ({ width, height, ...rest }) => {
Copy link
Member

Choose a reason for hiding this comment

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

앞서 Chan님의 리뷰에서도 언급하였던 부분입니다...!

무슨 의도로 분리한 컴포넌트일까요?

},
});

axiosInstance.interceptors.request.use(
Copy link
Member

Choose a reason for hiding this comment

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

intercepter 안에 해당 로직까지 있으면 읽기에 불편하지 않을까? 라는 생각이 있습니다.

저 같은 경우 이전에 다음과 같이 분리하여 사용을 해보았습니다.

const checkAndSetToken  = () => {
 // 토큰을 검사하고 세팅해주는 함수
}

const handleTokenError = () => {
 // 에러를 검사해주고 각 에러에 맞게 처리해주는 함수
}

axiosInstance.interceptors.request.use(checkAndSetToken, handleAPIError);

axiosInstance.interceptors.response.use(
  (response) => response,
  handleTokenError
);

axiosInstance.interceptors.response.use((response) => response, handleAPIError);

}, [dispatch]);

// api 호출
const fetchPageNation = useCallback(async () => {
Copy link
Member

Choose a reason for hiding this comment

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

페이지네이션을 좀 더 가볍게 만들 수 있지 않을까? 라는 생각이 듭니다..!

상위 컴포넌트에서도 api를 호출할 경우 페이지네이션에 대한 정보를 불러오기 때문에 해당 api들이 두번 이상 호출되는게 좋은가? 라는 생각이 드네요

};
export default Header;

const Wrapper = styled.div`
Copy link
Member

Choose a reason for hiding this comment

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

<header/>, <footer/>와 같은 시맨틱 태그를 사용해보시는 것은 어떨까요?

.matches(/^010-\d{4}-\d{4}$/, "유효한 전화번호를 입력해주세요."),
birthday: Yup.string()
.required("생년월일을 입력해주세요")
.matches(
Copy link
Member

Choose a reason for hiding this comment

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

날짜 필드를 검증할 때, 2024-02-30과 같이 실제로 존재하지 않는 날짜를 걸러내는 것이 중요합니다.

🧐이미 구현하신 코드는 기본적인 형식을 검증하지만, 윤년이나 각 달의 일수를 고려하지 않아 실제로 유효하지 않은 날짜도 통과시킬 수 있을 것으로 보여집니다..!

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.

2 participants