-
Notifications
You must be signed in to change notification settings - Fork 69
[Team-11] 새로운 이슈 만들기 기능 추가, DIContainer 에 대한 고찰 #243
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
base: team-11
Are you sure you want to change the base?
Changes from 11 commits
aea9a63
ed0be9c
3611e1a
3c6694a
7753233
60069e4
fb53b06
a17829e
d14a0d7
09d8f7e
b425f41
547ac43
d494374
43aefc6
e24536b
c200084
a30d281
4ec1a9f
48e7ceb
94700a7
f4b1f07
3200b4f
bc006c4
5e8b77a
d2675b2
b398019
b86ec49
1ddb879
d5c4855
709b78b
cf39c51
94f6cb2
2cf9ed0
c5bca22
f1dc0ce
d791820
aaf1c2f
7e9b7ba
3dce5a4
2aa67de
da4fa97
4d2672b
a54d231
7eb9168
50e1cb4
c43f970
7bf8c02
1065a7b
93724ce
300373e
7170c83
9f80da2
e3a03eb
c94cb85
6f7ac28
7e2c599
cf7c960
00867fb
5865265
75a66ca
264ed97
d37d8c9
55b3b87
0d2db84
6e2fcec
448b9dd
f44007b
c5d1b0b
13517eb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| // | ||
| // Repository.swift | ||
| // IssueTracker | ||
| // | ||
| // Created by Bibi on 2022/06/27. | ||
| // | ||
|
|
||
| import Foundation | ||
|
|
||
|
|
||
| struct Repository: Codable { | ||
| let name: String | ||
| let owner: Owner | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,8 +25,6 @@ final class IssueViewController: UIViewController { | |
| })) | ||
| button.setImage(UIImage(systemName: "plus"), for: .normal) | ||
|
|
||
| // button.layer.cornerRadius = 50 / 2 | ||
|
|
||
| return button | ||
| }() | ||
|
|
||
|
|
@@ -48,7 +46,7 @@ final class IssueViewController: UIViewController { | |
| setupViews() | ||
| model.requestIssue() | ||
| model.updatedIssues = { [weak self] issues in | ||
| DispatchQueue.main.async { [weak self] in | ||
| DispatchQueue.main.async { [weak self] in | ||
| self?.collectionView.reloadData() | ||
| } | ||
| } | ||
|
|
@@ -64,7 +62,14 @@ final class IssueViewController: UIViewController { | |
| } | ||
|
|
||
| @objc func touchedAddButton() { | ||
| self.navigationController?.pushViewController(Container().buildViewController(.newIssue), animated: true) | ||
| guard let appdelegate = UIApplication.shared.delegate as? AppDelegate else { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. 음.. 앱델리게이트는 앱이 죽을때까지 살아있는데 그걸 오브젝트로 만들어서 위험하겠군요.ㅠㅠ |
||
| return | ||
| } | ||
| guard let viewController = appdelegate.container.buildViewController(.newIssue) as? NewIssueViewController else { | ||
| return | ||
| } | ||
| self.navigationController?.pushViewController(viewController, animated: true) | ||
| viewController.delegate = self | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. 해당 부분에서 제가 보이는 시야는 |
||
| } | ||
|
|
||
| private func setupNavigationBar() { | ||
|
|
@@ -149,3 +154,9 @@ extension IssueViewController: UICollectionViewDelegateFlowLayout { | |
| return CGSize(width: collectionView.frame.width, height: 200) | ||
| } | ||
| } | ||
|
|
||
| extension IssueViewController: NewIssueCreateDelegate { | ||
| func created() { | ||
| model.requestIssue() | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,9 +6,43 @@ | |
| // | ||
|
|
||
| import UIKit | ||
| import SwiftUI | ||
|
|
||
| protocol NewIssueCreateDelegate: AnyObject { | ||
| func created() | ||
| } | ||
|
|
||
| class NewIssueViewController: UIViewController { | ||
|
|
||
| weak var delegate: NewIssueCreateDelegate? | ||
|
|
||
| private let service = IssueService() | ||
|
|
||
| private let optionList = Option.allCases | ||
| private var selectedList = Array<String>(repeating: "", count: Option.allCases.count) | ||
|
|
||
| private var selectedRepo: Repository? | ||
|
|
||
| enum Option: CaseIterable { | ||
| case repository | ||
| case label | ||
| case milestone | ||
| case assignee | ||
|
|
||
| var description: String { | ||
| switch self { | ||
| case .repository: | ||
| return "저장소" | ||
| case .label: | ||
| return "레이블" | ||
| case .milestone: | ||
| return "마일스톤" | ||
| case .assignee: | ||
| return "담당자" | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private lazy var navSegmentedControl: UISegmentedControl = { | ||
| let buttonList = ["마크다운", "미리보기"] | ||
| var control = UISegmentedControl(items: buttonList) | ||
|
|
@@ -38,7 +72,6 @@ class NewIssueViewController: UIViewController { | |
| return devider | ||
| }() | ||
|
|
||
| private let optionList = ["저장소", "레이블", "마일스톤", "담당자"] | ||
| private let optionTableCellIdentifier = "optionTableCellIdentifier" | ||
|
|
||
| private lazy var optionTable: UITableView = { | ||
|
|
@@ -109,7 +142,6 @@ class NewIssueViewController: UIViewController { | |
| make.top.equalTo(horizontalDevider.snp.bottom) | ||
| make.leading.trailing.equalTo(self.view.safeAreaLayoutGuide) | ||
| make.bottom.equalTo(optionTable.snp.top) | ||
|
|
||
| } | ||
| } | ||
|
|
||
|
|
@@ -129,18 +161,53 @@ class NewIssueViewController: UIViewController { | |
| }() | ||
|
|
||
| private func touchedCreateButton() { | ||
| // TODO: 이슈생성 | ||
| //1. api 호출 | ||
| //2. api 가 성공적으로 응답을 보내줬다면 => | ||
| //2-1. 이전 화면으로 돌아가고 | ||
| //2-2. 이슈 목록 조회 다시해서 보여주기 | ||
| //3. api 가 실패했다면 => issue 생성실패 얼럿띄우기 | ||
|
|
||
| guard let token = GithubUserDefaults.getToken(), | ||
| let selectedRepo = selectedRepo else { | ||
| return | ||
| } | ||
| guard let titleString = self.titleField.text, | ||
| !titleString.isEmpty else { | ||
| // TODO: - 타이틀 입력 값이 없다 => 얼럿 | ||
| return | ||
| } | ||
|
|
||
| service.createIssue(title: titleString, repo: selectedRepo, accessToken: token) { boolResult in | ||
| if boolResult { | ||
| self.navigationController?.popViewController(animated: true) | ||
| self.delegate?.created() | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. 객체가 담당하는 역할에 대해서 분리하는 걸 뜻했습니다. NewIssueViewController의 경우도, 위 행동을 추상화 시킨다면, 버튼이 눌렸을때, 행동을 실행시키며 |
||
| } | ||
| } | ||
| } | ||
|
|
||
| extension NewIssueViewController: UITableViewDelegate { | ||
|
|
||
| func tableView(_ tableView: UITableView, didSelectRowAt indexPath: IndexPath) { | ||
| switch optionList[indexPath.row] { | ||
| case .repository: | ||
| // TODO: issueService의 requestRepos() 연결해서 저장소목록 보여주기 | ||
| guard let appdelegate = UIApplication.shared.delegate as? AppDelegate, | ||
| let token = GithubUserDefaults.getToken() else { | ||
| return | ||
| } | ||
|
|
||
| service.requestRepos(accessToken: token) { result in | ||
| switch result { | ||
| case .success(let repositoryList): | ||
| guard let viewController = appdelegate.container.buildViewController(.optionSelect(token: token, repositories: repositoryList)) as? OptionSelectViewController else { | ||
| return | ||
| } | ||
| self.navigationController?.pushViewController(viewController, animated: true) | ||
| viewController.delegate = self | ||
| case .failure(let error): | ||
| print(error) | ||
| } | ||
| } | ||
|
|
||
|
|
||
| default: | ||
| break | ||
| } | ||
| } | ||
| } | ||
|
|
||
| extension NewIssueViewController: UITableViewDataSource { | ||
|
|
@@ -152,8 +219,8 @@ extension NewIssueViewController: UITableViewDataSource { | |
| let cell = tableView.dequeueReusableCell(withIdentifier: optionTableCellIdentifier, | ||
| for: indexPath) | ||
| var sidebarCell = UIListContentConfiguration.sidebarCell() | ||
| sidebarCell.text = optionList[indexPath.item] | ||
| sidebarCell.secondaryText = "선택내용" | ||
| sidebarCell.text = optionList[indexPath.item].description | ||
| sidebarCell.secondaryText = selectedList[indexPath.item] | ||
| sidebarCell.prefersSideBySideTextAndSecondaryText = true | ||
|
|
||
| cell.contentConfiguration = sidebarCell | ||
|
|
@@ -162,3 +229,11 @@ extension NewIssueViewController: UITableViewDataSource { | |
| } | ||
|
|
||
| } | ||
|
|
||
| extension NewIssueViewController: OptionSelectDelegate { | ||
| func selected(item: Repository) { | ||
| selectedList[0] = item.name | ||
| selectedRepo = item | ||
| self.optionTable.reloadData() | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,30 @@ | ||
| import UIKit | ||
| import SnapKit | ||
|
|
||
| class OptionSelectViewController: UIViewController { | ||
| protocol OptionSelectDelegate: AnyObject { | ||
| func selected(item: Repository) | ||
| } | ||
|
|
||
| private let dummy = ["bug", "feature", "document"] | ||
| class OptionSelectViewController: UIViewController { | ||
|
|
||
| weak var delegate: OptionSelectDelegate? | ||
|
|
||
| private let service = IssueService() | ||
| private var token: String? | ||
| private var options: [Repository]? | ||
|
|
||
| private let tableViewCellIdentifier = "tableViewCellIdentifier" | ||
|
|
||
| init(token: String, options: [Repository]) { | ||
| super.init(nibName: nil, bundle: nil) | ||
| self.token = token | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. 제 생각에는 OptionSelectViewController에서 필요한건 [Repository] 인데 그럼 이것만 넣어주면 토큰이 필요없어지긴 할 것 같습니다! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ViewController는 즉 View의 경우 이벤트를 발생시키는 역할만 충실히 만족하며, |
||
| self.options = options | ||
| } | ||
|
|
||
| required init?(coder: NSCoder) { | ||
| super.init(coder: coder) | ||
| } | ||
|
|
||
| override func viewDidLoad() { | ||
| super.viewDidLoad() | ||
| setupViews() | ||
|
|
@@ -32,20 +51,31 @@ class OptionSelectViewController: UIViewController { | |
|
|
||
| extension OptionSelectViewController: UITableViewDelegate { | ||
| func tableView(_ tableView: UITableView, didSelectRowAt indexPath: IndexPath) { | ||
| // TODO: - 선택한 옵션을 이전 ViewController 에 넘겨주기 | ||
| guard let options = options else { | ||
| return | ||
| } | ||
| let selectedItem = options[indexPath.row] | ||
| delegate?.selected(item: selectedItem) | ||
| self.navigationController?.popViewController(animated: true) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 코디네이터 즉 컨테이너를 둔 상태에서 뷰컨끼리의 이동은 전부 추상화 될 수 있는 부분입니다. |
||
| } | ||
| } | ||
|
|
||
| extension OptionSelectViewController: UITableViewDataSource { | ||
| func tableView(_ tableView: UITableView, numberOfRowsInSection section: Int) -> Int { | ||
| return dummy.count | ||
| guard let options = options else { | ||
| return 0 | ||
| } | ||
| return options.count | ||
| } | ||
|
|
||
| func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell { | ||
| guard let options = options else { | ||
| return UITableViewCell() | ||
| } | ||
| let cell = tableView.dequeueReusableCell(withIdentifier: tableViewCellIdentifier, | ||
| for: indexPath) | ||
| var content = cell.defaultContentConfiguration() | ||
| content.attributedText = NSAttributedString(string: dummy[indexPath.row]) | ||
| content.attributedText = NSAttributedString(string: options[indexPath.row].name) | ||
| cell.contentConfiguration = content | ||
| return cell | ||
| } | ||
|
|
||
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.
앗..흔적이 남아있네요😅
Container를 써야 하는 곳은 많은데 객체는 하나만 만들고 싶어서, 싱글톤을 적용해보려 했다가
지난 PR에 static 객체에 접근하기보다는 다른 방법을 사용해보라고 하셨던게 생각나서 싱글톤 없이 해보려고 방향을 바꾸었습니다.
그러다가 (바로 아래와 같은) 더 안 좋은 상황이 발생한 것 같네요..🥲