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

[소그룹스터디] routes를 static으로 관리 #382

Merged
merged 6 commits into from
Sep 22, 2024

Conversation

kimeodml
Copy link
Contributor

  • Close #ISSUE_NUMBER

What is this PR? 🔍

  • 기능 : 모든 경로를 static으로 변경
  • issue : #

Changes 📝

  • static 폴더 내의 routes.ts 에서 모든 경로를 관리하는 것으로 변경하였습니다.
  • 추후에 페이지 추가 시 static에서 관리해주면 되겠습니다.

ScreenShot 📷

Test CheckList ✅

  • 페이지 이동이 오류 없이 잘 되는지 확인
  • static으로 관리되지 않고 있는 경로가 있는지 확인

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

@kimeodml kimeodml requested review from ChoiWonBeen and daepan August 12, 2024 07:57
Comment on lines 20 to 25
OwnerOrderManagement: () => '/owner/order-management',
OwnerSalesmanagement: () => '/owner/sales-management',
OwnerEvent: ({ id, isLink }: ROUTESParams<'id'>) => (isLink ? `/owner/event-add/${id}` : '/owner/event-add/:id'),
OwnerEventModify: ({ id, isLink }: ROUTESParams<'id'>) => (isLink ? `/owner/event-modify/${id}` : '/owner/event-modify/:id'),
Login: () => '/login',
Signup: () => '/signup',
Copy link
Member

Choose a reason for hiding this comment

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

소소하지만 / 구분과 - 구분이 전부 대문자로 통일되어서 헷갈리기 쉽겠다 싶은데,
/ 구분을 객체로 넣어서 담는건 어떨까요?? 실제로 /는 레포지토리 뎁스를 의미하니, 코드를 사용할때도 같은 느낌으로 사용할 수 있을 것 같아욥

소소하게 salesManagement만 중간에 대문자가 빠지기도 했구요!

  Owner: {
    OrderManagement: () => `/owner/order-management`,
    SalesManagement: () => `/owner/sales-management`,
  }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

좋은 방법인 것 같아요! 반영했습니다. 54691a6

Copy link
Member

@ChoiWonBeen ChoiWonBeen left a comment

Choose a reason for hiding this comment

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

고생하셨어용 🙇

@kimeodml kimeodml merged commit b3d9174 into develop Sep 22, 2024
1 check passed
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