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

[Team-03][Android] 아이템 롱클릭 이벤트 처리, 리사이클러뷰 상단 UI 구현, 네트워크&Repository, DialogFragment , ActionLog message 구현 #163

Open
wants to merge 28 commits into
base: team-03
Choose a base branch
from

Conversation

HyungMinKang
Copy link

  • 아이템 롱클릭 이벤트 처리

    • 롱클릭시 popupMenu가 등장하도록 구현
    • 각 메뉴별 기능 (완료로 이동, 삭제, 수정)구현
  • 리사이클러뷰 상단 UI 구현

    • 타이틀과 현재 아이템개수를 보여주는 뱃지 뷰 구현
  • DialogFragment

  • 카드 추가, 수정시 나타날 DialogFragment 별도로 구현

  • ActionLog Message

    • ActionLog Message를 Html.fromhtml을 이용해서 html로 처리
  • 네트워크&Repository

    • 추가,삭제 api 구현
    • 액션로그 api명세 변경사항에 맞게 수정
    • toDo 아이템 조회 api 구현

kang-proto and others added 19 commits April 11, 2022 16:57
[TEAM-03][Android]  카드 추가를 위한 DialogFragment 구현,  Navigation Drawer의 close버튼 추가
-  item_count_background 리소스 정의
-  activity_todo 레이아웃 크기에 맞게 수정
-  커스텀 뷰 클래스 및 레이아웃 정의
- todoTitileView 의 추가버튼에 dialog 연결
- Id Int 로 변환
-  Next 추가
- 레파지토리 , 라피자토리 소스 에 대한 구현
- dummy 데이터 viewModel 로 이전
- add api 에 대한  DTO  생성
- add 응답 함수 구현
[Team-03][Android] Todo Add 부분 구현
[Team-03][Android]   Update를 위한 DialogFragment 추가,  롱클릭 컨텍스트 메뉴 처리
[Team-03][Android]  ActionLog 스타일적용을 위한 htmlFormatter 구현
- DTO 새롭게 정의
- strings.xml 수정
- ActionLog response 받는 객체 Response<> 로 감쌈
- 더미데이터 일부 수정
- network / repository 구현
[Team-03][Android] 네트워크 기능 구현
@nohriter nohriter added the review-android Further information is requested label Apr 13, 2022
var title: String,
var content: String,
var type: ProgressType,
var itemId: Int = -1,

Choose a reason for hiding this comment

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

itemId 의 기본값이 -1 인 이유가 있나요?

Copy link

Choose a reason for hiding this comment

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

아이템 추가하는 방식이 서버로부터 추가 요청을 보내면 id 값을 성공적으로 받아오는 경우 id 값을 부여하고 추가하는 식을 구현되어있어서 최소 생성 값은 id로 부여받을 수 없은 값인 음수로 진행했었습니다.
이유에 대해 생각하다보니 null 값이 더 적절한 것 같아 그렇게 고치겠습니다.

val nowLocation: String,
val historyDate: String
val pastLocation: Any

Choose a reason for hiding this comment

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

pastLocation 의 타입이 Any 로 되어있는데 어떤 의도였나요?

Copy link

Choose a reason for hiding this comment

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

서버의 응답이 string 이 아닌 null 이 올 수 도 있어서 Any 로 했지만
생각해보니 nullable type 으로 처리하면 되서 그렇게 수정했습니다.


// Insert
data class CardIndex(
val card_id: Int

Choose a reason for hiding this comment

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

card_id의 변수명 컨벤션 룰을 지켜주세요~

Copy link

Choose a reason for hiding this comment

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

넵 수정했습니다

Comment on lines 21 to 47
data class Todo(
val content: String,
val id: Int,
val nextId: Int,
val title: String,
val uploadDateTime: String,
val writer: String
)

data class Ing(
val content: String,
val id: Int,
val nextId: Int,
val title: String,
val uploadDateTime: String,
val writer: String
)


data class Done(
val content: String,
val id: Int,
val nextId: Int,
val title: String,
val uploadDateTime: String,
val writer: String
)

Choose a reason for hiding this comment

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

Todo, Ing, Done 데이터 클래스들의 내부 데이터들이 동일해보이네요~
그렇다면 클래스명만 바꿔서 여러 데이터 클래스들을 만들 필요가 있었을까요? ㅎㅎ

Copy link

Choose a reason for hiding this comment

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

넵 하나로 통합하여 사용하는 것으로 수정했습니다.

Comment on lines 12 to 17
suspend fun getCardIdx(
@Query("title") title: String,
@Query("content") content: String,
@Query("writer") writer: String,
@Query("current_location") current_location: String
): Response<CardIndex>

Choose a reason for hiding this comment

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

특정 카드의 인덱스를 찾기 위해 title, content, writer, current_location 정보를 전달해주도록 설계하셨네요 ?
만약 title, content, writer, current_location 이 모두 같은 경우에는 어떻게 되나요?

Copy link

Choose a reason for hiding this comment

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

위에서 언급한데로 아이템을 추가할 경우 서버로부터 id 를 요청하고 정상 응답 받을 경우 아이템을 생성하게 됩니다.
이때 서버로 id를 요청하는 위의 함수입니다.
따라서 생성에 필요한 인자들만 넘기게 되었습니다.

Comment on lines 42 to 44
binding.todoTitleViewTodo.title.text = "해야할 일"
binding.todoTitleViewInProgress.title.text = "하고있는 일"
binding.todoTitleViewTodoDone.title.text = "완료한 일"

Choose a reason for hiding this comment

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

문자열은 strings.xml 파일에서 관리하는게 유지보수할때 좋아요~
특히 UI 에서 사용하는 문자열은요 ㅎㅎ
(서비스에서 다국어를 지원한다고 상상해보세요)

Copy link

Choose a reason for hiding this comment

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

넵 이부분 수정하겠습니다.

@@ -123,22 +142,33 @@ class ToDoActivity : AppCompatActivity() {

toDoViewModel.todoList.observe(this) {
todoAdapter.submitList(it)
binding.todoTitleViewTodo.count.text = it?.size.toString()

Choose a reason for hiding this comment

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

itnull 인 경우 텍스트뷰에 어떻게 보일까요?
nullable 타입인 경우 항상 null 일 때 어떻게 처리할지 신경써주세요~

Copy link

Choose a reason for hiding this comment

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

네 null 일 경우에 대한 처리 신경쓰도록 하겠습니다,
null 일경우 0을 반환하도록 처리하려 했으나 nullable 타입이아닌 것을 확인하고 safe call 연산자를 제거했습니다.

Comment on lines 20 to 28
val todoList: LiveData<List<TodoItem>> = _todoList
val inProgressList: LiveData<List<TodoItem>> = _inProgressList
val doneList: LiveData<List<TodoItem>> = _doneList
var todoList: LiveData<List<TodoItem>> = _todoList
var inProgressList: LiveData<List<TodoItem>> = _inProgressList
var doneList: LiveData<List<TodoItem>> = _doneList

Choose a reason for hiding this comment

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

val 에서 var로 바꾼 의도가 무엇인가요?

Copy link

Choose a reason for hiding this comment

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

라이브데이터의 특징을 잠시 잊고 실수한 것 같습니다.
해당 부분 val 로 수정했습니다.

Copy link

@renovatio0424 renovatio0424 left a comment

Choose a reason for hiding this comment

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

코멘트 확인해주세요~

@shim506
Copy link

shim506 commented Apr 14, 2022

Q1. todoItem 의 add 나 remove update 가 발생했을때 데이터의 추가 삭제 수정이 뷰모델에서 일어나서는 안된다는 생각으로 이벤트가 발생할경우 뷰모델에서 레파지토리에 아이템 리스트(라이브데이터를 리스트화해서 전달)를 전달해주고 레파지토리 내부에서 해당 리스트에 추가 삭제 수정을 한뒤 다시 리스트를 반환하는 형태로 구현되어 있습니다. 해당 구현이 마음에 걸리는데 혹시 의견좀 부탁드릴 수 있을까요

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review-android Further information is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants