-
Notifications
You must be signed in to change notification settings - Fork 0
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
#19 [ui] 데일리 루틴 뷰 구현 #34
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아자아자 화이팅!! 할 수 있다
GONE의 세계로,,
} | ||
} | ||
|
||
private var isBtnSelected = false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
변수는 항상 class의 맨 상단에 주시면 됩니다!
isBtnSelected = !isBtnSelected | ||
btnRadioEmpty.isSelected = isBtnSelected | ||
} | ||
btnRadioEmptySecond.setOnClickListener { | ||
isBtnSelected = !isBtnSelected | ||
btnRadioEmptySecond.isSelected = isBtnSelected | ||
} | ||
btnRadioEmptyThird.setOnClickListener { | ||
isBtnSelected = !isBtnSelected | ||
btnRadioEmptyThird.isSelected = isBtnSelected |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
위의 변수를 공용으로 사용하고 있네요!
btnRadioEmpty.isSelected = !btnRadioEmpty.isSelected
이런식으로 바꿔주세요~!
import com.sopetit.softie.databinding.FragmentDailyEmptyBinding | ||
import com.sopetit.softie.util.binding.BindingFragment | ||
|
||
class DailyEmptyFragment : |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Empty보다는 DailyRoutineFragment가 좀 더 어울려 보입니다!
import com.sopetit.softie.databinding.ActivityDailyEditBinding | ||
import com.sopetit.softie.util.binding.BindingActivity | ||
|
||
class DailyEditActivity : BindingActivity<ActivityDailyEditBinding>(R.layout.activity_daily_edit) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DailyRoutineEditActivity는 어떤가요!
변수명이나 class명이 더 길어도 좋으니 명확하게 설명해주는 것이 좋아보입니다!
class DailyEmptyViewModel : ViewModel() { | ||
val routineAddList = listOf<Int>(1) | ||
|
||
companion object { | ||
const val NONE = 0 | ||
const val FIRST_ROUTINE = 1 | ||
const val SECOND_ROUTINE = 2 | ||
const val THIRD_ROUTINE = 3 | ||
const val FOURTH_ROUTINE = 4 | ||
const val MAX_ROUTINE = 3 | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
empty fragment가 daily activity에 종속된 것이 아니니 viewModel을 분리해주세요!
app:layout_constraintTop_toTopOf="parent" /> | ||
|
||
<TextView | ||
android:id="@+id/tv_cancel" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
마찬가지입니다~ id 컨벤션 지켜주세요!
app:layout_constraintTop_toTopOf="@id/tv_daily_routine" /> | ||
|
||
<View | ||
android:id="@+id/v_blank" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ㅎㅎ
app:layout_constraintEnd_toEndOf="parent" | ||
android:background="@drawable/shape_red_fill_12_rect" | ||
app:layout_constraintBottom_toBottomOf="parent" /> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
공백이 두 줄이네요 한 줄 삭제해주세요~
<androidx.appcompat.widget.AppCompatButton | ||
android:id="@+id/btn_delete" | ||
android:layout_width="0dp" | ||
android:layout_height="57dp" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저는 고정 dp 보다는 height = "0dp"를 쓰고
paddingTop, paddingBottom을 이용하는 걸 더 선호하기는 합니다!
app/src/main/res/values/strings.xml
Outdated
<string name="daily_edit_delete">n개 삭제</string> | ||
<string name="daily_edit_routine_name">데일리 루틴</string> | ||
<string name="daily_edit_cancel">취소</string> | ||
<string name="daily_routine_user_add">이불 개기</string> | ||
<string name="daily_routine_ing">n번째 달성 중</string> | ||
<string name="daily_routine_fin_btn">완료하기</string> | ||
<string name="daily_routine_fin">달성완료</string> | ||
<string name="daily_edit">편집</string> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
뷰마다 분리해주세요!
string 변수명도 변경 부탁드립니다~
const val NONE = 0 | ||
const val FIRST_ROUTINE = 1 | ||
const val SECOND_ROUTINE = 2 | ||
const val THIRD_ROUTINE = 3 | ||
const val FOURTH_ROUTINE = 4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
너무 잦은 companion object는 좋지 않습니다!
companion object에서 const val는 컴파일 시에 초기화 되기 때문에 싱글톤으로 사용할 수 있는데,
위와 같은 변수들은 쓰이지 않을 가능성이 크며 다른 화면에서 필요해 보이지 않습니다! => companion으로 분류할 필요가 없어보입니다~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저도 무작정 썼는데 잘 배워갑니다!
app/src/main/res/values/strings.xml
Outdated
<!-- doll selection --> | ||
<string name="doll_selection_question">어떤 친구와 함께 할까요?</string> | ||
<string name="doll_selection_subtitle">한 번 선택한 인형은 바꿀 수 없어요</string> | ||
<string name="doll_selection_button">이 친구와 함께 할래</string> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 부분이 아마 bear selection으로 바뀌면서 잘못 merge된 거 같네요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
올라가라~~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
진짜 수고하셨습니다!!
<corners android:radius="20dp"/> | ||
<solid android:color="@color/white"/> | ||
<stroke android:color="@color/gray100" | ||
android:width="1dp"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
solid, stroke 색 다른 경우에는 파일명 shape_white_fill_gray100_stroke_20_rect
으로 바꿔주세요!
} | ||
} | ||
|
||
private fun setButtonClickListener(button: View) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
함수화 좋아요~~
📑 Work Description
🛠️ Issue
📷 Screenshot
Screen_recording_20240111_143710.mp4
💬 To Reviewers
activity_sample 만들고 DailyRoutineActivity라는 가짜 액티비티 만들어서 확인해줬습니다.
편집 뷰에서 클릭한 개수대로 버튼 n개 삭제하기 텍스트 포함해서 편집용 브랜치에다가 로직 완성하겠습니다.