-
Notifications
You must be signed in to change notification settings - Fork 1
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
feat: 1.0.8 버전 업데이트에 따른 기능 추가 및 수정 #175
Conversation
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.
수고하셨습니다! 특히 Perception
경고는 진짜.. 힘들었을 것 같네요.. 딱히 문제는 없고 시간 나실때 자잘한 코드 수정만 해주세여(크리스마스 푸키 내리는거 너무 슬픈데..)
return Constants.serverURL.appendingPathComponent(Constants.versionPath, conformingTo: .url) | ||
let bundleID = Bundle.main.bundleIdentifier ?? "" | ||
let countryCode = Locale.current.language.region?.identifier ?? "" | ||
return URL(string: "https://itunes.apple.com/lookup?bundleId=\(bundleID)&country=\(countryCode)")! |
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.
써봤는데 최신 버전이랍시고 1.0.1 리턴되던데 버려진 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.
써봤는데 최신 버전이랍시고 1.0.1 리턴되던데 버려진 api느낌같아서 만들었습니다
어라..ㅋㅋㅋㅋㅋㅋㅋㅋ
@@ -169,10 +211,25 @@ private extension SplashFeature { | |||
} | |||
/// - Scope Effect | |||
func handleScopeAction(_ action: Action.ScopeAction, state: inout State) -> Effect<Action> { | |||
switch action { |
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.
Dependecies
에 기본 제공 Dependency values
에 openURL
이 있긴한데 이걸 쓰시는건 어떤가유
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.
죠습니다
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.
수정: b4b9073
|
||
case .alert: | ||
return .none | ||
} | ||
return .none |
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.
불필요 코드 제거 부탁합니당
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.
수정: 7b44d7f
return .none | ||
} | ||
/// - Delegate Effect | ||
func handleDelegateAction(_ action: Action.DelegateAction, state: inout State) -> Effect<Action> { | ||
return .none | ||
} | ||
|
||
func versionCheck() -> Effect<Action> { |
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.
이거 따로 쓰이는 곳이 있나요?
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.
따로 빼려나가 까먹고 방치된듯 지우겠읍니다
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.
수정: 7b44d7f
import Util | ||
|
||
|
||
public struct PokitSelectSheet<Item: PokitSelectItem>: View { |
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.
시트가 PokitSelect
에 너무 종속된건 아닌가 생각했었는데.. 확실히 빼는게 좋은거 같아요!
.padding(.top, 8) | ||
} | ||
|
||
var contentList: some View { |
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.
뷰를 내부 클로저로 빼는거랑 body
에 작성하는거랑 차이를 모르겠어요.. 진짜 그냥 Perception
버그 인거 같기도...
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.
저도 아직도 모르겠음 더 찾아봐야겠습니다
@@ -2,9 +2,66 @@ import ComposableArchitecture | |||
import XCTest | |||
|
|||
@testable import FeaturePokit | |||
@testable import Domain | |||
|
|||
final class FeaturePokitTests: XCTestCase { |
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.
헉.. 테스트!
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.
도메인이나 코어킷 테스트모듈도 필요할듯합니다..ㅠ 여기서 버전체크를 테스트 하고있음🥲
- 초기 folderType은 포킷이기 때문에 카테고리에 관한 정보만 받아온다. 따라서 미분류(store.contents)는 초기에 항상 Empty이기 때문에 필터 버튼이 보이지 않았음.
화면 전환이 되는 0.7초의 시간동안 버튼이 전환 되었음에도 이전 폴더 타입의 버튼액션을 받을 수 있었던 버그 수정
- 체크했을 때 id값 구분으로 체크/체크해제 구성 - 이전 화면에서 모든 미분류 링크를 받기 위해 생성자로 구성
- 포킷이동 버튼을 누르면 카테고리 선택 시트가 나오게 적용
- 에러로직 추가 - 삭제하기 바텀시트 구성 - 편집할 링크가 없을 때 보여줄 화면 분기처리
b4b9073
to
89594ce
Compare
#️⃣연관된 이슈
#173
📝작업 내용
스크린샷 (선택)
포킷이동 예시
포킷 추가하기 & 에러처리
미분류링크삭제
버전 분기
💬리뷰 요구사항(선택)
기존에 content를 기준으로 조건 처리를 하다보니 카테고리 화면에서 최신순/이름순 등 필터 버튼이 보이지 않는 것을 확인했습니다.
contents는 미분류 링크 목록을 말합니다.
초기 folderType은 포킷이기 때문에 카테고리에 관한 정보만 받아 오기 때문에 미분류(store.contents)는 초기에 항상 Empty일 수 밖에 없고 따라서 필터 버튼이 보이지 않았습니다.
적절히 분기처리를 통해 화면 별 필터 타입을 수정했습니다.
기존에 Modifier에 closure를 통해 뷰를 적용했었습니다. 항상 반복되는 패턴으로 extension으로 navigationBar를 분리한다면 사실 클로저가 의미가 없다고 생각했습니다.
다음과 같이 사용하게 추가해두었습니다.
PokitSelect PokitSelectSheet 중복코드
기존에 있던 PokitSelect가 활용되는 피쳐가 생겨 기능 수정이 이루어졌습니다. (짱짱!) @ShapeKim98
버전체크 로직 추가
기존 SplashFeature에 해당 기능이 추가되었습니다. 스플래시 뷰 이후 버전업데이트가 필요하다면 강제 alert으로 앱스토어로 이동되게끔 적용했습니다. 로직의 경우 major와 minor가 바뀌었을 때만 앱스토어로 이동되는 alert이 나타나게 됩니다. 맨 뒷자리는 신경쓰지 않습니다.
혹시몰라 테스트 케이스도 작성해두었습니다.
크리스마스 앱 아이콘도 내려야 하는데 PR리뷰하면서 고칠 때 같이 해버리겠읍니다.
fullScreenCover로 적용한 이유도 같이 주석 달아놨으니 참고하면 좋을듯!
편집하기 로직이 귀찮아도 간단해서 PR내용에는 상세하게 적지는 않았습니다. 바로 보고 이해하실 것 같음.
close #173