[Team10 - 쑤, Zeke] Issue, Label, Milestone View 구현 #16

lenaios · 2021-06-14T08:54:17Z

작업 목록

Issue, Label, Milestone, Login 화면을 코드로 구현
DTO 모델 구현
Github OAuth Login을 위한 모델 구현

고민한 것과 해결하지 못한 것

전체적으로 TableView를 사용했습니다.
이슈 화면에서 TableView의 Cell Height를 동적으로 구현하는 것을 해결하지 못했습니다.
toolBar 안에 textField를 추가하고나서, autolayout을 적용해 width를 동적으로 설정하는 것을 해결하지 못했습니다.

@ehgud0670
안녕하세요. 제이슨!
아직 구현하는 단계여서 코드 형식을 포함해 미흡한 부분이 많은 부분 감안하여 리뷰를 부탁드리겠습니다.
감사합니다.😊

[iOS] change build target

[iOS] LoginView

Conflicts: iOS/issue-tracker/issue-tracker.xcodeproj/project.pbxproj

[iOS] Tab Bar Controller

ehgud0670 · 2021-06-16T00:47:52Z

고민한 것

전체적으로 TableView를 사용했습니다.

상황에 맞게 테이블뷰 잘 쓰신것 같아요. 고민되시면 HIG 중에 Tables 와 Collections를 읽고 비교해보시는 것도 좋을 것 같아요 ~

ehgud0670 · 2021-06-16T00:54:23Z

해결하지 못한 것

이슈 화면에서 TableView의 Cell Height를 동적으로 구현하는 것을 해결하지 못했습니다.

이건 좀 더 고민해보시고 혼자만의 힘으로 구현하는 것을 추천드립니다. 힘든 것을 오롯이 혼자 힘으로 구현해보는 것이 다른 기능을 더 많이 구현하는 것보다 100배 더 가치가 있다고 생각합니다. 작년 멤버중에도 이 동적 높이를 구현한 친구가 한명으로 유일했습니다. 그 친구는 iOS-generic 채널에 문의도 하며 결국 구현하더라구요.

그 친구처럼 한번 구현해보시길 권합니다. 궁금한 점은 iOS-generic 채널에 문의해보세요. 해보고 모르시겠으면 그 친구 블로그를 알려드릴게요. 그럼 화이팅!

ehgud0670 · 2021-06-16T01:55:59Z

SwiftLint도 추가는 하셨지만 적용은 안하셨네요! 적용 부탁드립니다 ㅎㅎ
SwiftLint 사용법은 https://zeddios.tistory.com/447 이걸 참고하시면 될 것 같습니다.
그럼 퇴근후에 다시 리뷰드릴게요!

공용으로 사용될 버튼 컴포넌트들을 어떻게 사용할 수 있을까 고민됨, 머티리얼 ui에 startIcon이라는 prop이 있던데 그것을 사용할까 말까 고민됨.

[FE] 아이콘, 버튼 추가 LoginPage 짧게 구현 #16

ehgud0670

넵, 고생하셨습니다.
리뷰드렸는데 확인 부탁드릴게요~! 🙏

ehgud0670 · 2021-06-16T10:16:42Z

iOS/issue-tracker/LabelsCollectionViewCell.swift

+class LabelsCollectionViewCell: UICollectionViewCell {
+    static var identifiers = "LabelsCollectionViewCell"
+
+    let label: PaddingLabel = {


private 으로 둘 수 있지 않을까요? 빠르게 개발하시더라도 private 을 둬야하는지 고민하는 것은 습관을 들이셔야 합니다 ㅎㅎ

이 부분 말고도 외부에서 접근안하는 변수나 메소드는 private 처리 부탁드립니다.

다음 PR에 적용시키도록 하겠습니다!👍

ehgud0670 · 2021-06-16T10:18:04Z

iOS/issue-tracker/LabelsCollectionViewCell.swift

+import SnapKit
+
+class LabelsCollectionViewCell: UICollectionViewCell {
+    static var identifiers = "LabelsCollectionViewCell"


이 부분 protocol를 만들어서 적용하는 방법도 있습니다.
찾아보시고 괜찮으면 protocol 만들어서 사용하시는 것도 좋을 것 같아요 👍

ehgud0670 · 2021-06-16T10:22:16Z

iOS/issue-tracker/LabelsCollectionViewCell.swift

+    override init(frame: CGRect) {
+        super.init(frame: frame)
+        addSubview(label)
+        setAutolayout()


set은 java beans의 get set 메소드와 이름이 겹쳐서, 보통 설정(or 구성)하는 함수 네이밍으로 잘 쓰이는 setup이나 configure가 더 적절한 것 같습니다.

setup으로 수정했습니다.😊

ehgud0670 · 2021-06-16T10:46:53Z

iOS/issue-tracker/issue-tracker/Controller/IssueToolbar.swift

+
+    func setCheckMode(count: Int) {
+        checkBoxBarButtonItem.image = UIImage(systemName: "checkmark.circle")
+        labelBarButtonItem.title = "\(count)개의 이슈가 선택됨"


이렇게 "\(count)개의 이슈가 선택됨" 처럼 String 값을 만드는 것은 뷰가 아닌 프레젠터(ex. ViewModel, Presenter)의 역할입니다).

Mode -> Presenter -> View

이렇게 프레젠터 단을 만들면 뷰와의 의존성 없이 테스트 가능합니다.
MVVM 패턴을 사용하신다고 하여 말씀드립니다 ㅎㅎ

cell에 표시하기 위한 값을 ViewModel이 처리하도록 변경해보겠습니다. 다만, Rx를 활용해서 로직을 좀더 보완해야 할 것 같아서 우선 해당 메서드는 삭제하고 다음 리뷰에 포함시키겠습니다.😊

ehgud0670 · 2021-06-16T10:55:10Z

iOS/issue-tracker/issue-tracker/Controller/IssueDetailViewController.swift

+        return textField
+    }()
+
+    private lazy var toolbar: UIToolbar = {


lazy 선언으로 먼저 frame 방식으로 init 하고 이후에 autolayout을 적용하고 있네요!
한 가지만 해서 뷰를 구성할 수 있지 않을까요??

lazy는 사실 단순히 뷰컨트롤러 생성되기 전에 뷰를 만들려고 사용하는 것은 아닙니다

lazy var 는 메모리 효율을 높이기 위해 사용한다고 합니다. 이 블로그글 추천드려요.

lazy var는 동적으로 생성되는 순간은 atomic 하지 않아서 생성될 때 race condition(경쟁상태)의 위험이 있습니다. 해당 이슈로는 이 블로그글 추천드려요.

lazy는 위에 말씀드린 것처럼 많이 사용되지 않는 뷰를 만들때 사용해야 합니다. lazy 를 안 쓰고 어떻게 뷰를 만들지 고민해보세요 ㅎㅎ

toolbar를 frame으로 init을 한 이유는, toolbar의 frame 값을 지정하지 않으면 autolayout break warning이 뜨기 때문입니다.ㅠㅠ(원인을 전혀 모르겠습니다)

lazy var로 view를 생성하던 것을 lazy를 사용하지 않도록 수정했습니다.😊

ehgud0670 · 2021-06-16T11:35:50Z

iOS/issue-tracker/issue-tracker/Controller/AddViewController.swift

+            cell.textLabel?.text = "완료일"
+            return cell
+        default:
+            fatalError()


fatalError는 크래시로 앱에 매우 치명적입니다.
출시전에 확인해보시려는 목적이라면 assert를 추천드립니다.
어떻게 다른지 아래 글 추천드립니다
https://www.vadimbulavin.com/assert-vs-precondition-vs-fatalerror/

여기 말고도 다른 부분도 모두 없애는 게 좋아요~

fatalError() 대신 required init 구현부를 작성하도록 수정했습니다.😊

ehgud0670 · 2021-06-16T11:45:22Z

iOS/issue-tracker/issue-tracker/Controller/AddViewController.swift

+
+    override func viewDidLayoutSubviews() {
+        super.viewDidLayoutSubviews()
+        tableView.frame = view.bounds


여기 코드, 동적으로 계속 할당해야 하는 부분일까요?
다른 부분도 계속 동적으로 할당해야 하는지 고민해보시면 좋을 것 같습니다.

동적으로 할당할 필요가 없으므로 변경했습니다.😊

다른 viewDidLayoutSubviews 에도 동적으로 할당할 필요가 없는 코드가 있는 것 같은데
왜 viewDidLayoutSubviews를 자주 쓰셨는지 궁금합니다(정말 궁금해서 말씀드리는거에요 ㅎㅎ 동적으로 대응해야 하는 상황이 있는건가요?). 🧐

lenaios · 2021-06-17T08:15:50Z

@ehgud0670
피드백을 반영해 푸시완료했습니다!
확인 부탁드릴게요.😉

ehgud0670

네 반영하신 부분 모두 확인했습니다 😃
코멘트 하나 달았는데 답변만 부탁드립니다.
고생하셨습니다, 그럼 다음 스텝도 화이팅하시기 바랍니다. 💪

ehgud0670 · 2021-06-17T11:36:54Z

iOS/issue-tracker/issue-tracker/Controller/AddViewController.swift

+
+    override func viewDidLayoutSubviews() {
+        super.viewDidLayoutSubviews()
+        tableView.frame = view.bounds


다른 viewDidLayoutSubviews 에도 동적으로 할당할 필요가 없는 코드가 있는 것 같은데
왜 viewDidLayoutSubviews를 자주 쓰셨는지 궁금합니다(정말 궁금해서 말씀드리는거에요 ㅎㅎ 동적으로 대응해야 하는 상황이 있는건가요?). 🧐

lenaios · 2021-06-17T12:42:29Z

frame을 지정하는 view의 layout과 관련된 작업은 viewDidLoad 보다 viewDidLayoutSubviews 안에서 하는 것이 더 적합하다고 생각해서 viewDidLayoutSubviews 안에 작성했었습니다.🙂
다른 파일에서도 viewDidLayoutSubviews 안에서 매번 호출될 필요가 있는지 다시 한 번 살펴보겠습니다.😊
감사합니다!

ehgud0670 · 2021-06-17T23:43:47Z

넵, 저도 사실 viewDidLayoutSubviews에 대해 잘 몰랐어서 질문드렸어요.. ㅎㅎ
찾아보니 뷰가 동적으로 변화를 줄 필요가 있을때 쓰일 수 있는 메소드네요. 예를 들면 portrait 였다가 landscape으로 전환되는 경우 등 뷰의 크기를 바꿔줘야 할때 필요할 것 같습니다.
viewDidLoad vs viewDidAppear vs viewDidLayoutSubview 를 비교한 글을 찾아봤는데 읽으시면 도움될 것 같습니다! 💪

zekexros and others added 30 commits June 7, 2021 17:00

chore: 🔧 프로젝트 생성

ffa6744

chore: 🔧 .DS_Store 삭제

8685ae5

chore: 🔧 SwiftLint 라이브러리 추가

28a63d1

chore: 🔧 SnapKit 라이브러리 추가

1198271

chore:🔧 gitignore의 *.xcodeproj 주석처리

b36ed9d

feat: ✨add endpoint model

0519fd7

feat: ✨ add networkable protocol

ccaf439

chore: 🔧 clean up directory

a9cd883

chore: 🔧 change build target

3c7b011

Merge pull request #13 from swing-park/iOS/feature/4/network

46b3482

[iOS] change build target

chore: 🔧 delete unnecessary comment

0021a44

feat: ✨ add AuthManager (#15)

ad6a581

feat: ✨ Implement ID,Password TextField View

bb6bcaf

feat: ✨ Add AppleLoginButton (#14)

de0c7cb

feat: ✨Add IDPasswordTextField (#14)

dd3ea94

feat: ✨Add GitHubLoginButton (#14)

716b755

feat: ✨Add LoginView (#14)

737cba4

chore: 🔧 Add GitHub, Apple logo images

45f03d4

chore: 🔧 apply to mainstoryboard

6e21dae

Merge pull request #16 from swing-park/iOS/feature/14/LoginView

d56413a

[iOS] LoginView

Merge remote-tracking branch 'refs/remotes/origin/dev/iOS'

c978b1e

Conflicts: iOS/issue-tracker/issue-tracker.xcodeproj/project.pbxproj

chore: 🔧 update package info

41d8822

feat: ✨ embed in tab bar controller

c99c04d

Merge pull request #19 from swing-park/iOS/feature/#18/tabBarController

949208e

[iOS] Tab Bar Controller

feat: ✨ LabelTableViewCell 구현 (#20)

8e667fd

feat: ✨ LabelTableHeaderView 구현 (#20)

2f1fd6c

feat: ✨ extend UIColor to use HexColor (#20)

102c123

feat: ✨ AddButton 구현 (#20)

bdbb2ad

chore: 🔧 LabelViewController 추가(#20)

dc72fad

chore: 🔧 foldering view files

a755477

chore: change directory

39a18d7

lenaios added the review-iOS label Jun 14, 2021

lenaios requested a review from ehgud0670 June 14, 2021 08:54

lenaios assigned zekexros and lenaios Jun 14, 2021

Delete .DS_Store

5a83227

crongro pushed a commit that referenced this pull request Jun 14, 2021

feat: Main Page내 페이지별 라우팅 완성 (#16)

673cced

chore: add entry point to login storyboard

558db1f

ehgud0670 self-assigned this Jun 15, 2021

janeljs pushed a commit that referenced this pull request Jun 15, 2021

feat: (#16) IssueDTO 및APIService객체생성

a3591b1

janeljs pushed a commit that referenced this pull request Jun 15, 2021

feat: (#16) get메서드 생성

fb7adb4

janeljs pushed a commit that referenced this pull request Jun 15, 2021

feat: (#16) OpenIssue 정보 url 추가 및 get 메서드 수정

145f3e2

crongro pushed a commit that referenced this pull request Jun 16, 2021

Feat: 페이지 컴포넌트 초기세팅 #16

ecee87a

crongro pushed a commit that referenced this pull request Jun 16, 2021

Merge pull request #40 from hiro032/fe/16

3df5dd5

[FE] 아이콘, 버튼 추가 LoginPage 짧게 구현 #16

ehgud0670 reviewed Jun 16, 2021

View reviewed changes

lenaios added 6 commits June 17, 2021 16:10

chore: add custom textField to avoid lazy view

ec5bb6a

chore: remove check mode func

d21b2a7

chore: change fatalError to assert

0e84fb7

chore: implement required init

eb82907

chore: modify autolayout for table view

57c9ae4

chore: switch method name set to setup

3af47e8

ehgud0670 approved these changes Jun 17, 2021

View reviewed changes

ehgud0670 merged commit f99d1a7 into codesquad-members-2021:team10 Jun 17, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Team10 - 쑤, Zeke] Issue, Label, Milestone View 구현 #16

[Team10 - 쑤, Zeke] Issue, Label, Milestone View 구현 #16

lenaios commented Jun 14, 2021 •

edited

Loading

ehgud0670 commented Jun 16, 2021

고민한 것

ehgud0670 commented Jun 16, 2021 •

edited

Loading

해결하지 못한 것

ehgud0670 commented Jun 16, 2021

ehgud0670 left a comment

ehgud0670 Jun 16, 2021

lenaios Jun 17, 2021

ehgud0670 Jun 16, 2021

ehgud0670 Jun 16, 2021

lenaios Jun 17, 2021

ehgud0670 Jun 16, 2021

lenaios Jun 17, 2021 •

edited

Loading

ehgud0670 Jun 16, 2021

ehgud0670 Jun 16, 2021

lenaios Jun 17, 2021

ehgud0670 Jun 16, 2021

ehgud0670 Jun 16, 2021

lenaios Jun 17, 2021

ehgud0670 Jun 16, 2021

lenaios Jun 17, 2021

ehgud0670 Jun 17, 2021

lenaios commented Jun 17, 2021

ehgud0670 left a comment

ehgud0670 Jun 17, 2021

lenaios commented Jun 17, 2021

ehgud0670 commented Jun 17, 2021

[Team10 - 쑤, Zeke] Issue, Label, Milestone View 구현 #16

[Team10 - 쑤, Zeke] Issue, Label, Milestone View 구현 #16

Conversation

lenaios commented Jun 14, 2021 • edited Loading

작업 목록

고민한 것과 해결하지 못한 것

ehgud0670 commented Jun 16, 2021

고민한 것

ehgud0670 commented Jun 16, 2021 • edited Loading

해결하지 못한 것

ehgud0670 commented Jun 16, 2021

ehgud0670 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lenaios Jun 17, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lenaios commented Jun 17, 2021

ehgud0670 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lenaios commented Jun 17, 2021

ehgud0670 commented Jun 17, 2021

lenaios commented Jun 14, 2021 •

edited

Loading

ehgud0670 commented Jun 16, 2021 •

edited

Loading

lenaios Jun 17, 2021 •

edited

Loading