Fix [#157] 릴리즈 QA 수정사항 1차 반영 #159
Merged
Conversation
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
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -49,7 +49,7 @@ enum StringLiterals { | |||
|
|
|||
| enum Onboarding { | |||
| static let placeHolder = "한 문장으로 소개를 남겨주세요!" | |||
| static let information = "설정한 닉네임, 한 줄 소개는 설정에서 변경 가능해요!\n작성한 한 줄 소개는 작성한 게시글로 업로드 돼요." | |||
| static let information = "설정한 닉네임, 한 줄 소개는 설정에서 변경 가능해요!\n작성한 한 줄 소개는 게시글로 업로드 돼요." | |||
| } | |||
|
|
|||
| enum Button { | |||
There was a problem hiding this comment.
이 코드 패치를 간단히 검토해보겠습니다.
버그 위험:
- 현재 코드 패치에서는 기능적인 버그가 아닌 오타 수정만 있으므로 해당 부분으로 인한 버그 위험이 낮습니다.
개선 제안:
- "소개를 남기세요!"처럼 더 나은 표현을 사용하여 텍스트를 좀 더 친밀하게 만들 수 있습니다.
- 정적 문자열을 다른 리소스 파일로 이동하여 다국어 지원을 추가하는 것이 좋습니다.
- 문자열 내용이 자주 변경되는 경우, 텍스트를 외부 파일에 저장하고 동적으로 로드하는 방법을 고려할 수 있습니다.
| @@ -242,7 +242,7 @@ extension MyPageAccountInfoViewController { | |||
|
|
|||
| @objc | |||
| private func backButtonTapped() { | |||
| self.navigationController?.popViewController(animated: true) | |||
| self.navigationController?.popViewController(animated: false) | |||
| } | |||
|
|
|||
| @objc | |||
There was a problem hiding this comment.
위의 코드 패치를 간단히 검토해보겠습니다.
- 변경 사항:
backButtonTapped메서드에서 navigation controller의 뷰 컨트롤러 팝 시, 애니메이션을 사용하지 않도록 변경되었습니다. - 버그 위험: 현재 코드에 명백한 버그는 없어 보입니다. 그러나 애니메이션 사용 여부는 UI/UX에 영향을 줄 수 있으므로 디자인 가이드에 맞게 설정해야 할 것입니다.
- 개선 제안:
- 메서드명을 더 명확하게 작성하여 개발자가 이해하기 쉽도록 하세요.
- Swift 개념에 높은 우선 순위를 두고 @objc 어노테이션을 제거해보세요.
- 수정된 코드에 주석을 달아 설명을 추가하는 것이 좋을 수 있습니다.
코드 검토를 통해 주요 내용을 요약해 드렸습니다.
| @@ -205,7 +205,7 @@ extension MyPageEditProfileViewController { | |||
|
|
|||
| @objc | |||
| private func navBackButtonTapped() { | |||
| self.navigationController?.popViewController(animated: true) | |||
| self.navigationController?.popViewController(animated: false) | |||
| } | |||
|
|
|||
| @objc | |||
There was a problem hiding this comment.
코드 리뷰:
introText.count를 이용해 소개글 글자 수를 세는 것은 안전하지 않을 수 있습니다.introText의 값이 nil 일 때 앱이 다운될 수 있습니다.- `인트로덕션 문자열은 닫히는 괄호가 누락되어 있습니다. ",50)" 로 수정되어야 합니다.
output.popViewController에서sink사용시, subscription을 캡쳐하는 방식을 확인하세요.popViewController호출 시에animated: false로 변경한 것은 화면 전황과 어울리지 않을 수 있으니 디자인적으로 고려가 필요합니다.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
👻 PULL REQUEST
💻 작업한 내용
💡 참고사항
📟 관련 이슈