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

RENAME: Branch to Photobooth 파일및 폴더명 변경 #15

Merged
merged 1 commit into from
Nov 1, 2023
Merged

Conversation

chj950807
Copy link
Contributor

@chj950807 chj950807 commented Nov 1, 2023

  1. Branch~ -> Photobooth~ 으로 파일 및 폴더명 변경
  2. SideBar - 지점관리 ->포토부스관리로 변경
  3. PhotoboothEdit - hashtag 초기값 없을때 발생하던 오류 수정
  4. Table ->TableForm 테이블이 들어가던 페이지(EventManage, UserManage, PhotoboothManage.. etc) validateDOMnesting 관련 오류 수정
  5. UserManage, ReviewManage - Key를 제공하지않아 생긴 오류 수정 (index를 넣어 뒀지만 추후 정확한 ID 제공 해줘야 합니다.)

@chj950807 chj950807 requested a review from jsee53 November 1, 2023 07:29
@jsee53 jsee53 merged commit 1978699 into main Nov 1, 2023
1 check passed
import BrnachNew from "./pages/BranchNew";
import BranchEdit from "./pages/BranchEdit";
import PhotoboothNew from "./pages/PhotoboothNew";
import PhotoboothEdit from "./pages/PhotoboothEdit";

Choose a reason for hiding this comment

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

index 파일을 추가하고 모듈들을 Export 하면 의존성을 쉽게 확인할 수 있어요. 즉 의존성 확인을 위해 import문 정리가 필요해보이네요.
예시

Copy link
Contributor Author

Choose a reason for hiding this comment

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

넵 확인했습니다. components 폴더 내에서도 마찬가지일듯 한데, 확인해 보시고 일치 하는방향으로 가는것이 좋을것 같습니다.

@@ -65,7 +65,7 @@ export default function BranchEdit() {
<SubmitButton value={"수정"} />
</Box>
</Box>
<BranchEditInputForm
<PhotoboothEditInputForm
photoboothName={photoboothName}
setPhotoboothName={setPhotoboothName}

Choose a reason for hiding this comment

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

setState를 Props으로 내린 이유가 있을까요?? 만약 없으시면 이 글 읽는거 추천드립니다. Article

Copy link
Contributor Author

Choose a reason for hiding this comment

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

달아주신 아티클 확인했습니다.
setState랑 handler 함수를 만들어 해당 함수를 props로 내려주는것의 차이점을 말씀을 주신것 같습니다.
setState를 props로 내리는것이 잘못된 것이나 불필요한 방법은 아닙니다. 오히려 보편적인 방법입니다.
자식 컴포넌트가 부모의 구현 세부 사항에 지나치게 의존한다는 말이 있는데,� handler 함수를 따로 만들어서 props로 내리는 것이 오히려 부모의 구현사항에 의존성이 심해진다고 생각이 듭니다. 반복되는 함수가 아닌이상, 이용하는 최하위 컴포넌트에서 해당함수를 만들어 구현하는것이 더 직관적이라고 생각되어 저는 위와같이 구현하였습니다.

아티클의 작성자가 말하는 점은 이해가 가지만 코딩스타일의 차이점이지 이것이 옳다는 방향은 아닌것 같습니다.
동준님이 원하시는 코딩 방향이 위와 같다면 필요성에 대해 논의해 본후 진행하는것이 좋을것 같습니다.

import SideBar from "../components/reuse/sidebar/SideBar";
import PhotoboothEditForm from "../components/photobooth-edit/PhotoboothEditForm";

export default function PhotoboothEdit() {

Choose a reason for hiding this comment

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

지금 현재 컴포넌트 이름이랑 겹치는 문제가 있는데 PhotoboothEditPage 같은 네이밍으로 전환할까 고민이네요. 어떻게 생각하시나요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

폴더 경로가 달라서 크게 문제가 되지 않는다고 생각되어 지정하였습니다. 앱 또한 같은 구조로 되어있어서 @sunujun 님과도 같이 고민해서 수정을 하면 좋을것 같습니다.

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.

3 participants