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.

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

Conversation

otoolz
Copy link
Collaborator

@otoolz otoolz commented Jan 13, 2024

🌎 PR 요약

여행생성의 제목 입력의 글자수를 제한해요.

🌱 작업한 브랜치

📚 작업한 내용

  • 제목 입력을 14글자 이상 입력이 안되도록 수정했습니다.
  • 토스트는 실패 타입으로 붉은색으로 띄우도록 했습니다.

📍 참고 사항

피드백 요청

  • 글자 수에 제한을 둘꺼라면 입력했던 걸 지우게 하는 것보다 애초에 글자 수 초과 시 입력에 제한을 두는 게 사용성이 좋다고 생각해서 UITextField Delegate에서 처리를 해줬습니다.
    추가적으로 고민했던 부분은 뷰모델의 단방향 플로우를 거치지 않고 토스트를 띄워줬는데, 입력을 안받게 했으니까 여기서 토스트를 바로 추가해 줘도 되지 않을까라는 생각에 이렇게 작성하게 됐습니다. 근데 다른 분들의 의견은 어떤지 궁금해서 피드백 부탁드립니다!

키보드에 가려진 토스트 처리

  • 토스트 메세지가 키보드에 가려지는 것 때문에 개선이 필요했습니다. 이 전에는 NotificationCenter를 이용해서 높이를 직접 구하는 방법을 사용했었는데, 이번에 되게 간단하게 구현할 수 있는 방법을 알게되서 공유드립니다!

토스트 뷰 레이아웃인데 키보드 위로 올리고 싶은 뷰의 바텀앵커를 keyboardLayoutGuide.topAnchor로 두면 키보드 위로 올라오게 됩니다.

NSLayoutConstraint.activate([
    bottomAnchor.constraint(equalTo: view.keyboardLayoutGuide.topAnchor, constant: -24),
    leadingAnchor.constraint(equalTo: view.leadingAnchor, constant: Metric.margin),
    trailingAnchor.constraint(equalTo: view.trailingAnchor, constant: -Metric.margin),
    heightAnchor.constraint(equalToConstant: Metric.toastHeight)
])

근데 여기서 중요한게 이렇게만 하면 바로 적용이 안될 수가 있어서, 키보드를 정확하게 트래킹하도록 적용해야 합니다. ( 실제로 앱 시작 후, 첫 번째 키보드에는 반응을 안했었습니다.)
그래서 이걸 키보드가 활성/비활성되는 여행생성 뷰에서 적용해주는 코드입니다.

view.keyboardLayoutGuide.followsUndockedKeyboard = true

iOS 15.0이상이면 사용할 수 있어서 유용할 것 같습니다. 참고 자료 링크도 첨부할게요!
https://developer.apple.com/documentation/uikit/keyboards_and_input/adjusting_your_layout_with_keyboard_layout_guide

📸 스크린샷

RPReplay_Final1705129580.mov

관련 이슈

@otoolz otoolz added 🍎 iOS iOS 작업 시 🌟 Feature 새로운 기능 개발 시 labels Jan 13, 2024
@otoolz otoolz requested review from kth1210 and 0inn January 13, 2024 07:37
@otoolz otoolz self-assigned this Jan 13, 2024
Copy link
Member

@kth1210 kth1210 left a comment

Choose a reason for hiding this comment

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

글자 수 초과되었을 때 입력 막고 토스트로 보여주는거 너무 좋은거 같은데요..??
확실히 사용자가 지우게 하는 것보다 편할 것 같아서 좋습니다!

그리고 토스트 메세지 띄우는 부분에서 코멘트 남겨두었으니 한번 확인해주시면 감사드립니다..!

아니 키보드 레이아웃 잡아주는거 이런게 있었다니,,,,,,,
여러 군데서 활용도가 좋을 것 같은데요??..
잘배워갑니다 기웅님 😌😌

Comment on lines +361 to +369
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
}
Copy link
Member

@kth1210 kth1210 Jan 13, 2024

Choose a reason for hiding this comment

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

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

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.

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

Comment on lines 67 to 69
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),
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.

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

Copy link
Collaborator

@0inn 0inn left a comment

Choose a reason for hiding this comment

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

키보드 위로 뜨는 거 야무지네요 ㅎㅎ
고생하셨습니다 ~ ! ! 기웅님 ~ !
저는 따로 리뷰 사항 없어서 어푸룹해둘게요 :)

Comment on lines 67 to 69
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),
Copy link
Collaborator

Choose a reason for hiding this comment

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

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

@otoolz otoolz merged commit 2f7c047 into boostcampwm2023:ios-develop Jan 19, 2024
@otoolz otoolz deleted the ios-feature/#394-travelTitleCharacterLimit branch January 19, 2024 05:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌟 Feature 새로운 기능 개발 시 🍎 iOS iOS 작업 시
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants