-
Notifications
You must be signed in to change notification settings - Fork 0
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
[refactor] 원서 작성 페이지 리펙토링 #167
Open
gaoooon
wants to merge
33
commits into
develop
Choose a base branch
from
refactor/oneseoWritingPage
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
33 commits
Select commit
Hold shift + click to select a range
a3deb39
feat : step enum 추가
gaoooon 6c8f5ab
fix : step에 따른 schema로 변경
gaoooon c909fb4
del : zustand 제거
gaoooon c0be4c9
feat : getValuesByEnum util 추가
gaoooon 72ea55b
fix : options parameter 옵셔널화
gaoooon 7d56a47
fix : useStore 제거, 구조 변경, 컴포넌트 이름 변경
gaoooon 8f7119d
del : score 타입 삭제
gaoooon e03ede5
fix : 상수 이름 변경
gaoooon e1b80a4
feat : MAX_SCORE, MIN_SCORE, GED_MAX_SCORE 상수 추가
gaoooon 29f8fe3
feat : 원서 작성 값 enum 추가
gaoooon 15d6746
del : semesterType 삭제
gaoooon 7305915
fix : 원서 타입 수정
gaoooon e995563
fix : 모의성적 계산 페이지 구조 수정
gaoooon 66eae07
fix : 바뀐 상수 이름에 맞게 변경
gaoooon 10f5a22
del : zustand 로직 제거
gaoooon db4109f
Merge branch 'main' into refactor/oneseoWritingPage
gaoooon e6d0b22
feat : 계산후 dialog 추가
gaoooon 1f92375
fix : 잘못된 조건식 수정
gaoooon 70304dc
feat : ACHIEVEMENT_FIELD_LIST 상수 추가
gaoooon 9c66225
feat : schema 재사용되는 부분 분리
gaoooon 3b8a22e
feat : valueAsNumber 옵션 추가
gaoooon 6a4f76b
fix : reset 제거하고 setValue로 변경
gaoooon 6f7cfe8
fix : PostOneseoType null 타입을 옵셔널로 변경
gaoooon 977e0da
feat : AchievementType 추가
gaoooon 199190d
refactor : 구조 수정
gaoooon b5cbcbc
fix : map으로 반복되는 list props로 받도록 변경
gaoooon 2bed128
fix : reset에서 setValue로 변경
gaoooon 75be3c8
feat : baseUrl, handleCheckScoreButtonClick props 추가
gaoooon b4810c1
feat : api handling, dialog 추가
gaoooon d53e7d4
feat : dependency 업데이트
gaoooon 8201c19
del : EditStep4Page 삭제
gaoooon d3f9ec6
Merge branch 'develop' of https://github.com/themoment-team/hellogsm-…
gaoooon 6ddb2da
fix : setTimeout 두번쨰 인자 추가
gaoooon File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
export * from "./src"; | ||
export * from './src'; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
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.
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.
GraduationTypeValueEnum.CANDIDATE처럼 Enum 값을 여러 번 직접 사용하는 대신, 상위에서 구조 분해 할당하여 사용하는 게 어떨까요?
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.
enum도 구조 분해 할당이 되는지는 몰랐네요
그치만 제 의견을 말씀드리자면 enum이 현재 대문자로 되어있는데 구조분해 할당을 하여 사용한다면 추후에 코드를 볼떄 상수를 사용하고 있다고 생각할수도 있을것 같습니다 그래서 구조 분해 할당을 사용하지 않는 방식이 enum을 사용하고 있다는 사실을 명확하게 나타내는것 같습니다
다른 분들의 의견도 여쭈어봐야 할것 같습니다
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.
좋은 의견 감사합니다! 구조 분해 할당을 사용하면 코드가 간결해지긴 하지만, 말씀하신 대로 enum이 무엇을 나타내는지 명확하게 인지할 수 있는 점에서는 구조 분해 할당이 다소 혼동을 줄 수 있을 것 같습니다. 특히 대문자 상수와 비교했을 때 enum이라는 사실을 코드만 보고 구별하기 어려울 수 있겠네요.
해당 의견에 대해서 어떻게 생각하시는지 궁금합니다! @yoosion030 @frorong @hyeongrok7874
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.
좋은 리뷰 주제네요, 코드를 보았을 때 네이밍에 enum이라는 것이 드러나기 때문에, 구조 분해 할당을 사용했을 때 enum이 상수처럼 보인다고 느껴지진 않는 것 같습니다.
저는 구조 분해 할당을 사용하여 가독성을 줄이는 관점 보다는 enum을 사용하는게 맞는 관점인지 먼저 논의해보면 좋을 것 같다고 생각해요. enum을 자주 사용해보진 않아서, 현재 케이스에 enum을 사용하는게 맞는지 의문이 들긴하거든요.
https://xpectation.tistory.com/218
해당 자료 참고하여 enum, const enum, as const 중에 어떠한 것으로 타입을 관리할 것인지 먼저 의논한 뒤에 코드 가독성에 대한 내용을 다루면 좋겠네요.
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.
글을 읽고 처음 생각했을때는 enum은 양방향 맵핑이 가능하다고 해서 현재 상황에서는 사용하지 않기 떄문에 as const를 사용하면 된다고 생각했었는데 조금 더 찾아보니 숫자형 enum만 양방향 맵핑이 가능해서 기존 코드를 그대로 유지해도 상관 없다고 생각합니다. 그리고 제가 enum을 도입한 이유를 말씀드리자면 기존의 방식은 유니온 타입으로 되어있었는데 값을 할당할때나 값을 비교할때 더 명시적으로 사용할수 있어 도입하게 되었습니다