Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[박상준] Week19 #495
[박상준] Week19 #495
Changes from 43 commits
650aaaa
e957f73
05c8c2c
55759a3
22741b8
447cd3b
1c30413
23c3207
81d32fe
7f8a9ba
603ec44
cceee4e
5a78e4d
31382aa
5da2477
d06117f
3ac39e8
a86d24b
9a3df14
70afb0d
46e5205
29d4bd4
1478bff
e8bd9ec
bbed201
c158130
dc6e24e
8fcc9b0
cc044d3
9c44f43
d8e5e8d
b647fd5
8efc686
7d61fd2
9993d82
ba2d670
dc65c45
a7547be
fa87972
60a0ab5
1937e8b
c28f12b
8e81eac
2f5349b
9940b9c
e1820b0
3239777
4cf9a4b
c07080c
cae884c
e6312ca
f35a293
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sj0724
getFolder
등 UI에 필요한 불러오는 서비스 레이어에서 각 함수들이 어떤 값을 반환하는지 타입을 설정해주면 UI에서 사용하기 편하지 않을까요?또한, 에러핸들링의 경우, 던져진 오류를 catch로 잡아주었는데, 굳이 다시 던져주는 이유가 있을까요~?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
api 함수는 초기에 작성하고 리펙토링을 따로 진행하지 않아 다른 부분에서 코드를 인용해서 사용하고 그대로 뒀습니다. 그래서 따로 에러를 다시 던지는 이유는 없습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sj0724
쿼리는 무언가를 조회하기 위한 조건을 이야기합니다.
그렇다면, 네트워크 요청이 실제 발생하는 요소는 뭐라고 하는게 좋을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
url이라는 이름으로 사용해도 될까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
url은 전체 주소를 주로 이야기를 하죠, 이 경우는 endpoint라는 변수로 만들어 사용해주면 어떨까 싶습니다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sj0724
요 녀석들도 적합한 에러 핸들링이 필요해요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sj0724
accessToken은 로그인한 사용자에 대한 중요한 정보를 담고있어요.
누군가가 이 정보를 해킹한다면 바로 사용자 계정으로 접근해 핵심 정보를 추출해올 수 있겠죠.
그렇게 중요한 데이터를 localStorage에 관리하는건 좋ㅈ ㅣ않아 보이는데요, 어떻게 개선해볼 수 있을지 고민해볼까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
우선 nextauth를 사용해서 세션에 저장하도록 변경했습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sj0724
isActive와 buttonActive는 어떻게 다른가요?
같은 목적으로 사용된다면 하나만 사용해주세요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sj0724
ButtonProps는 버튼 요소가 지닌 모든 attribute를 상속받도록 해주셨어요.
그러면 children, size, isActive 외에도 type, onClick 등 다양한 요소들을 사용할 수 있어야 공통 버튼 콤포넌트로 평가할 수 있겠는데요. 우리가 선언한 값 외에 모든 버튼이 지닌 attribute들도 전달되도록 한번 수정해볼까요?
또한, CTA라는 네임은 Call to Action으로, 화면에서 사용자가 고객으로 전환을 하도록 넛징해주는 요소를 뜻해요.따라서 우리가 공통으로 사용하는 버튼을 CTA라고 명세하는건 좋지 않을 것 같아요.