Skip to content
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.

Sign up for GitHub

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] 여행생성시 제목 글자수 제한 작업 #396

Merged
merged 4 commits into from
Jan 19, 2024
Merged

[feat] 여행생성시 제목 글자수 제한 작업 #396

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9e84389
chore #394: TLToastView Layout 변경
otoolz Jan 12, 2024
e2afd47
feat #394: 여행 생성 제목 글자수 제한
otoolz Jan 12, 2024
7bf9a3a
feat #394: toast view 추가
otoolz Jan 12, 2024
a8f46aa
Merge branch 'ios-develop' into ios-feature/#394-travelTitleCharacter…
otoolz Jan 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion iOS/traveline/Sources/DesignSystem/View/TLToastView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ final class TLToastView: UIView {
translatesAutoresizingMaskIntoConstraints = false
alpha = 1.0
NSLayoutConstraint.activate([
bottomAnchor.constraint(equalTo: view.bottomAnchor, constant: -24),
bottomAnchor.constraint(equalTo: view.keyboardLayoutGuide.topAnchor, constant: -24),
leadingAnchor.constraint(equalTo: view.leadingAnchor, constant: Metric.margin),
Comment on lines 67 to 69
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

와 이런게 있었군요...??.. 너무 좋은거 같아요.....
야무지게 배워갑니다 ㅋㅋㅋ 😋

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저도 진짜 초면이네요 , , 너무 유용한 아이 ,,
감사합니다 홍기님 :)

trailingAnchor.constraint(equalTo: view.trailingAnchor, constant: -Metric.margin),
heightAnchor.constraint(equalToConstant: Metric.toastHeight)
Expand Down
13 changes: 13 additions & 0 deletions iOS/traveline/Sources/Feature/TravelFeature/TravelScene/VC/TravelVC.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ final class TravelVC: UIViewController {
}

private enum Constants {
static let titleLimit: Int = 14
static let titleLimitToastMessage = "제목은 1 - 14자 이내만 가능합니다."
static let title: String = "여행 생성"
static let textFieldPlaceholder: String = "제목 *"
static let done: String = "완료"
Expand Down Expand Up @@ -144,6 +146,7 @@ final class TravelVC: UIViewController {

private extension TravelVC {
func setupAttributes() {
view.keyboardLayoutGuide.followsUndockedKeyboard = true
view.backgroundColor = TLColor.black
titleTextField.placeholder = Constants.textFieldPlaceholder
baseScrollView.delegate = self
Expand Down Expand Up @@ -354,6 +357,16 @@ extension TravelVC: UITextFieldDelegate {
dismissKeyboard()
return true
}

func textField(_ textField: UITextField, shouldChangeCharactersIn range: NSRange, replacementString string: String) -> Bool {
let text = textField.text ?? ""
if text.count + string.count > Constants.titleLimit {
let textLimitToast = TLToastView(type: .failure, message: Constants.titleLimitToastMessage)
textLimitToast.show(in: self.view)
return false
}
return true
}
Comment on lines +361 to +369
Copy link
Member

@kth1210 kth1210 Jan 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

p1; 이 부분 제목 유효성 검사 플로우가 이미 TravelViewModel과 TravelUseCase에 구현되어 있습니다..!

TravelVC 내의 bind 메서드에서 시작하는 titleEdited action 플로우를 한 번 확인해보시면 좋을 것 같습니다 :)
스크린샷 2024-01-13 오후 5 11 40

TravelUseCase에서 유효성 검사를 하고, 해당 유효성 검사의 결과를 통해 state의 isValidTitle 값이 변경됩니다!
스크린샷 2024-01-13 오후 5 12 58

그래서 아마 해당 값을 바인딩해서 사용하면 되지 않을까 싶습니다..!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오, 맞아요. useCase에서 유효성 검사 부분은 확인했었는데, 이 부분이 제가 고민했던 부분입니다.
입력을 안받도록 했는데 글자 수 유효성 검사를 위해 useCase까지 플로우를 보내는게 맞는가를 고민했었습니다.

그래서 저렇게 textField에 바로 toast를 띄우게 작성했던 것 같습니다..!

근데 지금보니까 저기서 바로 toast를 띄울게 아니라 저기서 action을 보내주고 viewModel의 state를 통해서 toast를 띄우는게 좋을 것 같다는 생각이네요!
저는 개인적으로 useCase까지는 플로우를 안보내도 될 것 같아서 유효성 검사 부분은 코드 지워도 될 것 같은데, 어떻게 생각하시나요??
두 분 의견 주시면 반영해서 수정하겠읍니다.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

제가 제대로 이해한 것인지 모르겠지만 그럼 유효성 검사를 VC에서 하게 되는 것일까요??

물론 유효성 검사가 현재 간단한 로직이라 UseCase까지 갔다오는 것이 과한가? 싶기도 하지만 저희가 지금 비즈니스 로직을 UseCase에서 작성하고 있기 때문에 여기서도 마찬가지로 UseCase에서 처리해야하지 않을까 싶긴 합니다.

유효성 검사 조건을 변경하거나 로직이 복잡해질 경우 어떤 로직은 VC에 있고 어떤 로직은 UseCase에 있고 그럼 나중에 수정할 때 어려울 것 같다는 생각도 듭니다!

그래서 일단 저는 비즈니스 로직은 UseCase에서 관리하는걸로 통일하는게 낫지 않을까 하는 의견입니닷

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아하 저는 유효성 검사라기 보다는 제목이 14자를 초과했을 때 입력 자체를 받지않는다고 생각해서, 버튼의 활성/비활성같은 경우랑 비슷하다고 생각했습니다.

useCase까지 플로우를 주면, 지금과 같은 기능을 하는데 지금보다 복잡하고 많은 코드가 추가되는 단점도 있고,
현재 유효성 검사를 하고 있는 프로필 수정화면에서는 유효성에 대한 결과를 알려주는 라벨도 따로 있고, 토스트 뷰를 띄워주지도 않아서 약간은 다른 상황이라고 생각이 들어서 고민이 되네요.

두 방식 다 장단점이 있는 것 같아서, 영인님 의견도 들어보면 좋을 것 같습니다 ㅋㅋ

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저도 확실히 두 방식의 장단점이 존재하는 거 같아요 . . !

일단 비즈니스 로직을 어디까지 바라보느냐에 대한 기준이 모호할 수 밖에 없는데
저번에 같이 얘기했을 때는 기획에 따라 바뀌어야 할 의존성을 어디서 가져가느냐가 핵심이었던 것 같아요.

예를 들어, 10자로 제한이 바뀌었을 경우, 혹은 특수 문자를 포함하지 않아야 할 경우 어디를 바꿔야 할까요?
이걸 UseCase에서 처리하게 되면 비즈니스 로직 레이어인 Domain, 지금처럼이라면 UI쪽 레이어인 Presentation(Feature)을 살펴보게 되겠죠.

이런 상황일 때, 어느 레이어에서 처리하는 게 더 적합한지 고민해보면 좋을 것 같아요.

결론적으로 정답은 없지만, ,
저희가 빡세게 비즈니스 로직을 가져가거나 확장성을 고려한다면 UseCase에 추가하는 게 좋을 것 같고,
그게 아니라면 지금처럼 가져가도 좋을 것 같긴 합니다 :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ㅎㅎㅎㅎ 그럼 이 부분은 일단은 이렇게 가져가고, 이번 주 일요일 회의할 때 다시 얘기해봐도 좋을 것 같습니당.
우선은 머지해두겠습니당!!

}

// MARK: - TLBottomSheetDelegate
Expand Down