Wedemy / eggeum-android

나에게 맞는 카페 찾기, 이끔
MIT License
12 stars 0 forks source link

정보수정제안 v1 MR #172

Closed kymjaehong closed 10 months ago

kymjaehong commented 10 months ago

147 구현 및 메인 화면 코드 반영에 따라 MR 후 추가 작업하겠습니다.

PlaceEntity와 InfoEntity의 경/위도 타입을 제가 nullable하게 해서 지훈님 코드 쪽으로 수정했습니다.

이후에 다이얼로그 및 placeId를 지도로 부터 가져오는 작업하겠습니다.

height[bot] commented 10 months ago

Link Height tasks by mentioning a task ID in the pull request title or commit messages, or description and comments with the keyword link (e.g. "Link T-123").

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

easyhooon commented 10 months ago

다음부터는 리뷰어로 저 선택해주세여 그래야 메일 알림이 와서 알수가 있어가지공

easyhooon commented 10 months ago

지금 보니까 domain 모듈 model 패키지의 entity 의 parameter 들이 어떤건 public 이 붙어있고, 어떤건 안붙어있군요 다 없애버려도 될 것 같은데 이건 제가 하도록 하겠습니다

=> 이게 어느 부분인지 잘 모르겠는데 예시 하나만 긁어주실 수 있으신가요?

easyhooon commented 10 months ago

뒤에 붙은 PR title 뒤에 MR 무슨 의미인가요?

-> 아.. 깃랩이랑 착각했네요

easyhooon commented 10 months ago
image

각 텍스트필드의 제목에 (회의실, 다인석...) 괄호 치고 단위를 표시하는 항목이 누락되어있습니다 넣어주세요

easyhooon commented 10 months ago

메뉴 수정 또는 카페 정보 수정이 완료된 이후 다음 버튼을 눌렀을 때 다시 처음 화면 (어떤 것을 개선하고 싶으세요?) 화면으로 이동하는 로직이 누락되어 있는데, 이번 또는 다음 피처에서 구현 부탁드려요

=> 아 아예 기존 main activity인 지도로 돌아가는 줄 알았습니다. 수정할게요

easyhooon commented 10 months ago

메뉴 수정시, 가격의 자릿수가 늘어날 경우 3자리씩 , 로 끊어주는 로직이 필요합니다. 구현하는데 어려움이 있다면 제가 나중에 할게여 예전에 만들어놨던 함수가 있어가지고

=> 넵, 감사합니다. 지훈님 코드 반영되면 공부하겠습니다.

easyhooon commented 10 months ago

피드백 반영 당장 하실 수 있는 것들 먼저 하시구, 다음 피처때(merge 한 이후에) 남은 피드백 반영하시겠다고 알려주시면 approve 누르겠습니다

=> 피드백 일부 반영했습니다. 다음 피처에서 아래 내용 추가해서 코드 수정할게요. 1) 뷰모델에서 데이터 관리 2) string resource에서 반복 사용되는 문자열 가져오기 (반영하려했으나, 에러발생해서 주석)

easyhooon commented 10 months ago

=> 이게 어느 부분인지 잘 모르겠는데 예시 하나만 긁어주실 수 있으신가요?

// domain 모듈 Entity -> FileEntity
public data class FileEntity( 
  public val uploadFileId: Int, // <- 그냥 val 이라고 해도 default 가 public 이라 상관없을것 같아서요
  val url: String,
  val url: String?,
)
easyhooon commented 10 months ago

=> 아 아예 기존 main activity인 지도로 돌아가는 줄 알았습니다. 수정할게요

같은 로직이 main 모듈의 유저 정보화면내에 문의하기 화면(reportFragment)에 구현되어있는데 거기에 있는 navigation 로직 참고해서 똑같은 패턴으로 구현하면 될거에여

==> 이 부분 반영했습니다

easyhooon commented 10 months ago

수고하셨어요!

근데 카페 정보를 수정 완료할 경우엔 "정보 수정을 완료 했어요."가 타이틀인 화면으로 이동하는게 맞는것 같군요. 피그마에 건의는 넣어놨습니다. 아마 화면이 추가될 수 도 있을 것 같아요

merge 하시면 됩니다!

=> 감사합니다 !