Skip to content

Conversation

@sujin9
Copy link

@sujin9 sujin9 commented Oct 15, 2023

안녕하세요 핑구 ~ 🐧❤️
벌써,, 마지막 미션 마지막 리뷰요청이라니,,, 기분이 이상하네요 😢
마지막 리뷰를 핑구에게 받게 되어 영광입니당 히히 이번 스텝도 잘 부탁드려요 ~

지난 리뷰

전체 지우기 되돌리기 기능

전체 지우는 경우, 임시 변수에 해당 내역들을 담아놓고, 뒤로가기 하면 addAll을, 새롭게 변경사항이 생기면 초기화해주도록 했어요.

선택된 색상 표시 기능

PaintColor가 isSelected 프로퍼티를 갖도록 했고, data class로 변경한 뒤 색상이 선택될 때마다 isSelected 값을 초기화해줍니다.
그리고 이러한 color list를 viewModel에서 관리하면서, 옵저빙 + ListAdapter를 이용하여 선택할 때마다 체크 표시가 다르게 보여지도록 했어요.

이와 같이 두 기능 모두 추가 구현했습니다 ~ 훨씬 편리해진게 느껴져서 기분이 좋네요! 😆

이번 미션

테블릿 가로, 세로 모드, 그리고 다크 모드까지 구현했습니다!

테블릿 세로 + 라이트 모드

테블릿 가로 + 다크 모드

폰 라이트 모드

폰 다크 모드

테블릿 세로 + 다크 모드

그리고 제 마음입니다. 💘💘💘

@sujin9 sujin9 requested a review from pingu244 October 15, 2023 10:17
@sujin9 sujin9 self-assigned this Oct 15, 2023
Copy link

@pingu244 pingu244 left a comment

Choose a reason for hiding this comment

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

안녕하세요 하티🐥❣️
마지막 미션까지 고생많으셨어요~!~!
3단계 미션 요구사항은 모두 완료하신 것 같아 어프루브하겠습니다!
몇가지 코멘트 남겼으니 확인해보세요
마지막까지 화이팅해봅시다💪

Comment on lines 34 to 38
fun setBrushColor(index: Int) {
val color = colors[index]
_brushColors.value = _brushColors.value?.map {
PaintColor(it.colorRes, it.colorRes == color.colorRes)
}

Choose a reason for hiding this comment

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

전 정말 심심하실 하티를 위해 선택사항으로 드렸는데 이렇게 멋지게 구현해주셨군요! 하티 멋져요👍👍👍


override fun onBindViewHolder(holder: ColorViewHolder, position: Int) {
holder.bind(colors[position])
holder.bind(currentList[position])

Choose a reason for hiding this comment

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

holder.bind(getItem(position)) 이렇게 할 수도 있어요!

Copy link
Author

@sujin9 sujin9 Oct 16, 2023

Choose a reason for hiding this comment

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

오,, 이런 방법도 있는 줄은 몰랐네요! 😆

Comment on lines 46 to 52
private val colors = mutableListOf(
PaintColor(R.color.red, true),
PaintColor(R.color.orange, false),
PaintColor(R.color.yellow, false),
PaintColor(R.color.green, false),
PaintColor(R.color.blue, false),
)

Choose a reason for hiding this comment

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

이 부분은 추후 유지보수를 위하여
이 뷰모델에 종속되는 것이 아닌 다른 파일로 빼내는 것이 어떨까요?

Copy link
Author

Choose a reason for hiding this comment

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

완전완전 동의합니다!! 😊

Choose a reason for hiding this comment

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

저도 이렇게 drawable-night를 만들어서 그에 해당하는 아이콘들을 모두 흰색으로 해주었는데요,
이번에 써니에게서 받은 리뷰가 아주 좋은 것 같아서 공유합니다! 테마에서 설정한 색상 값으로 설정할 수 있다고 합니다.
android:fillColor="?attr/colorOnPrimary"
써니의 리뷰

Copy link
Author

Choose a reason for hiding this comment

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

오,, 너무 좋은데요?? 매번 라이트/다크 모드 마다 두 파일로 관리해야 함이 불편하다고 생각했는데
좋은 방법인 것 같아요!! 감사합니다 😆

android:layout_height="0dp"
android:layout_margin="10dp"
android:src="@drawable/ic_check"
android:visibility="@{isSelected ? View.VISIBLE : View.INVISIBLE}"

Choose a reason for hiding this comment

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

오 이렇게도 할 수 있군요! 배워갑니다 땡큐😉

@pingu244 pingu244 merged commit 147fc5f into woowacourse:sujin9 Oct 16, 2023
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.

2 participants