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

[리팩토링] 메뉴 추가 페이지 컴포넌트 분할 및 에러 분기 추가 #351

Merged
merged 14 commits into from
Jun 17, 2024

Conversation

daepan
Copy link
Contributor

@daepan daepan commented Jun 11, 2024

What is this PR? 🔍

Changes 📝

  1. 초기 세팅 에러 몇 가지를 수정했습니다. 기본값으로 잘못된 값이 있었습니다.
  2. 페이지 내부에서 mutation 과정에서 다수의 변수명 수정 및 에러 처리가 가능하도록 toast 에러 표시 기능을 추가했습니다.
  3. 로직 분할이 필요한 MenuPrice를 컴포넌트로 분할하여 가독성과 재사용성을 높혔습니다.
  4. 분명 네이밍은 되어있는데 파일에 관측되지 않은 슈뢰딩거의 CSS를 제거했습니다.

ScreenShot 📷

Test CheckList ✅

  • test 1
  • test 2
  • test 3

Precaution

✔️ Please check if the PR fulfills these requirements

  • It's submitted to the correct branch, not the develop branch unconditionally?
  • If on a hotfix branch, ensure it targets main?
  • There are no warning message when you run yarn lint

@daepan daepan self-assigned this Jun 11, 2024
Copy link
Contributor

@haejinyun haejinyun left a comment

Choose a reason for hiding this comment

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

컴포넌트로 분리되니 훨씬 깔끔해졌네요.
아주 초반에 오너웹 작업을 했던 것 같은데 다시 보니 새롭군요...

궁금한 점 하나 남겼습니다.
수고많으셨습니다!

Comment on lines 63 to 68
if (addMenuError) {
showToast('error', '메뉴 추가에 실패했습니다.');
} else {
navigate('/owner');
}
};
Copy link
Contributor

Choose a reason for hiding this comment

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

위 부분에서 궁금한 점이 있습니다!
addMenuMutation 안에서 onSuccess와 함께

onError: () => { },

이런 방향으로 처리하는 방식은 어떤가요? mutationFn의 성공, 실패에 따른 처리가 한 곳에 있는 것이 좋지 않을까?라는 생각이 들어 남겨봅니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mutation에 onError 로 처리했습니다. 0a988a1

Copy link
Contributor

@hoooooony hoooooony left a comment

Choose a reason for hiding this comment

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

슈뢰딩거의 css는 아마 가격 추가 수정 로직 변경하면서 제가 삭제를 안 한것 같습니다..

goMyShop();

const onClickMenuAddConfirmHandler = (e: React.MouseEvent<HTMLButtonElement>) => {
e.preventDefault();
Copy link
Contributor

Choose a reason for hiding this comment

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

mouse이벤트에 막아줘야할 기본적인 동작이 있나요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

막아줘야할 부분이 딱히 없는 것으로 확인되어서 다시 제거하였습니다. 0a988a1

@daepan daepan merged commit cab9c6c into develop Jun 17, 2024
1 check passed
@daepan daepan deleted the feat/#350 branch June 17, 2024 08:59
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.

[리팩토링] AddMenu 페이지 리팩토링
3 participants