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

[Fix] 오브젝트 수정 기능 QA 반영 #140

Merged
merged 9 commits into from
Dec 3, 2024
Merged

Conversation

taipaise
Copy link
Collaborator

@taipaise taipaise commented Dec 2, 2024

🌁 Background

금일 마스터 클래스에서 진행한 버그 트리아즈에서 오브젝트 수정 관련해서 들어온 QA들을 반영했습니다.

📱 Screenshot

  • iPhone SE3
se3.mov
  • iPhone 13 mini
13mini.mov
  • iPhone 16 Pro
16pro.mov

👩‍💻 Contents

  • 사진이 추가되지 않던 문제 수정
  • 오브젝트 삭제 버튼 크기 확대
  • 오브젝트를 삭제 버튼을 탭해서 삭제 -> 삭제 영역으로 드래그하여 삭제로 변경
  • 텍스트 오브젝트 뷰를 선택하면 바로 텍스트 수정을 위해 키보드가 올라오던 현상 수정
  • 텍스트 편집 시 키보드가 삭제 버튼과 다른 오브젝트들을 가리던 문제 수정
  • 텍스트 편집 시 return 키를 누르지 않아도 텍스트가 편집되도록 수정
  • TextViewObject에 placeholder 제공
  • UX를 위한 햅틱 반응 추가

📝 Review Note

  • 사진이 추가되지 않던 문제 수정

    • 멍청하게도.. 아침에 수정할 떄 사진 오브젝트 생성 시 사진을 파일 시스템에 저장하는 코드를 삭제했었습니다.
  • 오브젝트 삭제 버튼 크기 확대

    • 이미지 크기를 40*40로 수정했습니다.
  • 오브젝트를 삭제 버튼을 탭해서 삭제 -> 삭제 영역으로 드래그하여 삭제로 변경

    • 오브젝트를 드래그할 때 손가락이 WhiteboardToolBardeletionImageframe에 들어오고, 이 상태로 손가락을 떼면 삭제하도록 수정했습니다.
    • 이제 버튼이 필요 없어졌기 때문에 deleteButton은 삭제했습니다.
    • 아래와 같이 WhiteboardToolBar에서 삭제 영역을 얻어올 수 있습니다. WhiteboardToolBar에서 deletionImage의 좌표를 superView(WhiteboardViewController.view)에서의 frame으로 변환해 return 합니다.
        var deleteZone: CGRect {
            return convert(deletionImage.frame, to: superview)
        }
  • 텍스트 오브젝트 뷰를 선택하면 바로 텍스트 수정을 위해 키보드가 올라오던 현상 수정

    • 텍스트 오브젝트 뷰의 수정 가능 여부를 설정하는 configureEditable함수에서 resignFirstResponder를 호출해 키보드가 바로 올라오지 않도록 수정했습니다.
  • 텍스트 편집 시 키보드가 삭제 버튼과 다른 오브젝트들을 가리던 문제 수정

    • 키보드가 보일 때, transform을 통해 키보드 높이만큼 canvasView를 위로 올렸습니다.
    • 키보드가 내려갈 때, 원복하도록 했습니다.
  • TextViewObject에 placeholder 제공

    • 커스텀 TextField와 delegate를 구현했습니다.
    • placeholder를 검은색으로 표시하기 위해 attributedPlaceholder를 사용했습니다.
  • 텍스트 편집 시 return 키를 누르지 않아도 텍스트가 편집되도록 수정

    • ViewModel의 input에 추가 case를 넣어서 해결했습니다.
    • 선택 해제를 하면 view.endEditting(true)를 통해 view의 first responder 상태를 resign하게 하고 있습니다. 이 과정에서 object의 deselect가 호출되면서 수정하던 내용이 모두 날아가버리는 문제가 있었습니다. (deselect 되면서 textObject가 update 되는데, 이때 변경한 text는 적용되지 않고, selectedBy 프로퍼티만 바뀌기 때문)
  • UX를 위한 햅틱 반응 추가

    • 위치, 크기, 각도 수정을 완료할 때 햅틱 반응 (medium)을 유저에게 전달합니다.
    • 오브젝트를 선택후 삭제 영역에 손가락을 드래그하여 옮기면 유저에게 햅틱반응 (heavy)를 전달합니다.

@taipaise taipaise added this to the 화이트보드 수정 milestone Dec 2, 2024
@taipaise taipaise self-assigned this Dec 2, 2024
Copy link
Collaborator

@choijungp choijungp 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 +56 to +57
let placeholderAttributes: [NSAttributedString.Key: Any] = [.foregroundColor: UIColor.airplainBlack]
self.attributedPlaceholder = NSAttributedString(string: placeholderText, attributes: placeholderAttributes)
Copy link
Collaborator

Choose a reason for hiding this comment

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

우아 이건 무슨 코드인가요 ?? 텍필의 placeholder 색상 변경해주는 건가유 ? ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

맞습니다!! 따로 간편하게 placeholder의 색을 바꿀 수 있는 방법이 없어서 요렇게 해줘야해요 ㅠㅠ

Copy link
Member

Choose a reason for hiding this comment

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

옵젝씨 코드 같아여..ㄷㄷ

Comment on lines +24 to +25
static let deletionDisabledImage = "trash.circle"
static let deletionEnabledImage = "trash.circle.fill"
Copy link
Collaborator

Choose a reason for hiding this comment

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

오 오 드래그 앤 드랍 성공해서 삭제 가능하면 deletionEnabledImage가 뜨는거군요 ~~ 굿뽀이

Copy link
Collaborator

Choose a reason for hiding this comment

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

+ 이렇게 이미지 네임도 LayoutConstant에 넣는거 너무 좋타고 생각 !!!

@@ -235,19 +269,30 @@ public final class WhiteboardViewModel: ViewModel {
}

private func changeObjectPosition(to point: CGPoint) {
defer { isDeletionZoneEnable.send(false) }
Copy link
Collaborator

Choose a reason for hiding this comment

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

defer를 자유자재로 잘 사용하시는군요 !!! 👍🏻👍🏻👍🏻👍🏻

Comment on lines +276 to +287
Task {
let isSuccess = await manageWhiteboardObjectUseCase
.removeObject(whiteboardObjectID: selectedObjectID, isReceivedObject: false)
if isSuccess { selectedObjectSubject.send(nil) }
}
} else {
Task {
await manageWhiteboardObjectUseCase.changePosition(whiteboardObjectID: selectedObjectID, to: point)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

오 ~ 이런 방식으로 drag 해서 delete를 구현하신거군요 !!!! 역시 띵똥 ..
나하아이이쓰 띵똥 ~

Comment on lines +252 to +257
if let editingText {
await textObjectUseCase.editText(id: selectedObjectID, text: editingText)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

이렇게 하면 텍스트 필드 수정하다가 canvasView 터치해서 디셀렉하면 수정 중인 텍스트로 바뀌는 건가유 ? ?? ?! 🥺

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

맞아요 안그럼 중간에 deselect 됐을 때 text를 수정할 수가 없더라구요 ㅜㅜ
요 부분 고민을 정말 많이 했는데 현재 구조적으로는 이게 최선이라 생각했습니다!
아니면 완전 갈아 엎어야 할거 같아요!

Comment on lines +348 to +356
if selectedObjectViewFrame.midY > view.bounds.midY {
canvasView.transform = CGAffineTransform(translationX: 0, y: -keyboardHeight)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

아까 설명해주셔서 바로 이해했슴 ~~ 굿쟙

.receive(on: DispatchQueue.main)
.sink { [weak self] isDeleteZoneEnable in
if isDeleteZoneEnable {
HapticManager.hapticImpact(style: .heavy)
Copy link
Collaborator

Choose a reason for hiding this comment

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

요기는 heavy인 이유가 먼가유 (단순 궁금)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

몬가 몬가 medium은 약한거 같구,, 삭제될수 있다!!! 고 알려줘야 하지 않을까해서 헤비로 했습니다!

Copy link
Member

@eemdeeks eemdeeks left a comment

Choose a reason for hiding this comment

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

고생하셨습니다 :)
구현한게 많으시다보니, QA 이후 수정할 것도 많아지네요ㅜ 다음엔 나눠 맡아도 됩니다 :)

Comment on lines +34 to +52
self.addAction(
UIAction { [weak self] _ in
guard let self else { return }
self.airplainTextFieldDelegate?.airplainTextFieldDidChange(self)
},
for: .editingChanged)

self.addAction(
UIAction { [weak self] _ in
self?.attributedPlaceholder = nil
},
for: .editingDidBegin)

self.addAction(
UIAction { [weak self] _ in
self?.configurePlaceHolder()
},
for: .editingDidEnd)
}
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 Author

Choose a reason for hiding this comment

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

editingChanged 에 대한 action을 등록해 준 것은, UITextFieldDelegate에서 텍스트가 수정될 때마다 호출되는 delegate가 없기 때문에 위와 같이 구현했습니다.

editingDidBegin에서 placeholder를 nil로 바꿔준 것은,텍스트 입력 시작하자마다 placeholder를 없애주기 위해서입니다. (사용자 경험을 위해) 이걸 없애지 않으면 사용자가 입력을 시작해도, text를 하나 이상 입력하지 않으면 placeholder가 없어지지 않기 때문입니다. 기본적인 placeholder처럼 회색인 경우는 자연스럽지만, 저희 placeholder는 검정색이라 없어지는게 자연스럽다고 생각했습니다.
editingDidEnd에서 placeholder를 다시 설정한 이유는, editingDidBegin에서 nil로 바꾼 placeholder를 다시 살려주기 위함입니다.

@@ -371,6 +381,7 @@
A8525DC52CE201C00089DA5E /* Common */ = {
isa = PBXGroup;
children = (
00C295092CFDF13700BD6768 /* View */,
Copy link
Member

Choose a reason for hiding this comment

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

airplaINTextField를 따로 common에 두신 이유가 있을까요??

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

UITextFiled처럼 여러 군데에서 활용할 수 있지 않을까?? 해서 Common으로 뺐습니다!!
우선은 화이트보드 화면에서만 사용하고 있지만 범용성 있게 사용할 수 있을 것이라 생각했거든요
예를 들면 색상 중 wordle.red가 초기 기획단계에서는 워들에서만 사용될 예정이었지만, 실제로는 여러 군데에서 사용하고 있는 것처럼요!

Comment on lines 85 to 86
UIImage(systemName: WhiteboardToolBarLayoutConstant.deletionEnabledImage) :
UIImage(systemName: WhiteboardToolBarLayoutConstant.deletionDisabledImage)
Copy link
Member

Choose a reason for hiding this comment

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

systemName안에 삼항 연산자가 들어가도 될 것 같기는 합니다!

Comment on lines 62 to 69
deinit {
configureTearDownObserver()
}
Copy link
Member

Choose a reason for hiding this comment

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

deinit이 잘 호출 되는 군요!

viewWillDisappear에 해주는 것과 차이점이 무엇일까요?

개인적으로는 deinit에 해준다면, 이후 화면(없기는 하지만) 즉, 다음 뎊스 에 들어갔을 때 해당 notification을 받을 때 메소드가 실행 될 것 같아요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

말씀해주신 이유 때문에 viewWillDisappear에서 해주는게 맞는것 같습니다! 바로 수정~


self.addAction(
UIAction { [weak self] _ in
self?.attributedPlaceholder = nil
Copy link
Member

Choose a reason for hiding this comment

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

attribute를 nil로 바꾸지 않고, "" 값으로 바꾸는건 어떠실까요?

그렇다면 아래에서 configurePlaceHoler를 다시 호출하지 않고, "Hello, AirplaIN"으로만 바꿔주면 될 것 같습니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

똑똑 딴 아주 좋은 생각인거 같습니다

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

NSAttributeString이라 바로 초기화가 안되서 현상유지 하는걸로 ㅠㅠ

@taipaise taipaise merged commit 371a773 into develop Dec 3, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants