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

[ISSUE-43] 홈 화면 UI 구현 #73

Merged
merged 35 commits into from
Jul 5, 2022
Merged

Conversation

hoyahozz
Copy link
Member

@hoyahozz hoyahozz commented Jun 29, 2022

ISSUE


작업 내용

  • 홈 화면의 UI 구현
  • 공통 컴포넌트로 사용할 캘린더 UI 구현
  • 바텀 네비게이션이 화면을 가리는 오류 해결
  • MaterialCalendarView 의존성 추가
  • Compose ConstraintLayout 의존성 추가

실행 화면

Screen Shot GIF
Screenshot_1656503730 Home

Check List

  • PR 제목은 [ISSUE-{N}] PR 제목으로 작성
  • CI/CD 통과 여부
  • 1명 이상의 Approve 확인 후 Merge
  • 관련 이슈 연결

@hoyahozz hoyahozz self-assigned this Jun 29, 2022
Copy link
Member

@jihee-dev jihee-dev 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 1 to 19
<vector xmlns:android="http://schemas.android.com/apk/res/android"
android:width="20dp"
android:height="20dp"
android:viewportWidth="20"
android:viewportHeight="20">
<path
android:pathData="M10.833,13.333L7.5,10L10.833,6.667"
android:strokeLineJoin="round"
android:strokeWidth="0.875"
android:fillColor="#00000000"
android:strokeColor="#9CA3AD"
android:strokeLineCap="round"/>
<path
android:pathData="M7,0.306L13,0.306A6.694,6.694 0,0 1,19.694 7L19.694,13A6.694,6.694 0,0 1,13 19.694L7,19.694A6.694,6.694 0,0 1,0.306 13L0.306,7A6.694,6.694 0,0 1,7 0.306z"
android:strokeAlpha="0.3"
android:strokeWidth="0.6125"
android:fillColor="#00000000"
android:strokeColor="#6671F6"/>
</vector>
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
Member Author

Choose a reason for hiding this comment

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

84c4547
🫠🫠🫠🫠🫠

class CalendarDecorator {
class TodayDecorator(context: Context) : DayViewDecorator {
private var date = CalendarDay.today()
private val boldSpan: StyleSpan = StyleSpan(Typeface.BOLD)
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
Member Author

Choose a reason for hiding this comment

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

f797009
🫠🫠🫠🫠🫠

Comment on lines 37 to 39
view?.addSpan(object : ForegroundColorSpan(Color.White.toArgb()) {})
view?.setSelectionDrawable(drawable)
view?.addSpan(boldSpan)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
view?.addSpan(object : ForegroundColorSpan(Color.White.toArgb()) {})
view?.setSelectionDrawable(drawable)
view?.addSpan(boldSpan)
view?.let {
it.addSpan(object : ForegroundColorSpan(Color.White.toArgb()) {})
it.setSelectionDrawable(drawable)
it.addSpan(boldSpan)
}

묶어줘도 좋을 것 같아요!

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
Member Author

Choose a reason for hiding this comment

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

f797009
🫠🫠🫠🫠🫠

private val drawable = ContextCompat.getDrawable(context, R.drawable.bg_calendar_selection)

override fun shouldDecorate(day: CalendarDay?): Boolean {
val calendar = day!!.calendar
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
Member Author

Choose a reason for hiding this comment

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

addDecorator를 할 때 null이면 데코레이팅 자체를 해주지 않는 로직이 있어서, CalendarDay?에서 ?를 빼줘도 될 것 같네요! 지적해주셔서 감사합니다 :)

Copy link
Member Author

Choose a reason for hiding this comment

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

f797009
🫠🫠🫠🫠🫠

// TODO : 약속 수 들어가는 로직 넣기 (정호)
@Composable
fun HomeIsLoginBox() {
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
Member Author

Choose a reason for hiding this comment

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

34e1dd6
🫠🫠🫠🫠🫠

fun HomeTopBox(loginState: HomeContract.LoginState) {
Copy link
Member

Choose a reason for hiding this comment

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

TopBox라는 네이밍이 조금 모호해서.. 같이 고민해서 이름을 변경해 봤으면 좋겠어요 ㅠㅠ 어떤 화면인가요??

Copy link
Member Author

Choose a reason for hiding this comment

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

크게 기준을 앱바 - 상단 박스 - 하단 박스 요렇게 세 단계로 분류를 잡아서 분리했습니다..!
홈 화면 외에서 활용되지 않을 것 같아서 기준을 이런식으로 잡았어여..!

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
Member Author

Choose a reason for hiding this comment

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

34e1dd6
🫠🫠🫠🫠🫠


@Composable
fun HomeIsNotLoginBox() {
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
Member Author

Choose a reason for hiding this comment

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

34e1dd6
🫠🫠🫠🫠🫠

Copy link
Member

@jihee-dev jihee-dev left a comment

Choose a reason for hiding this comment

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

추가로 지금 크기들이 피그마에 소수점 단위로 보이는 건 알지만.. 유도리있게 되도록이면 4의 배수 혹은 못해도 짝수로는 맞춰주시면 더 좋을 것 같아요!

userName: String,
onUserIconClick: () -> Unit,
) {
ConstraintLayout(
Copy link
Member

Choose a reason for hiding this comment

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

이거 그냥 Row로 해도 좋을 것 같은데 ConstraintLayout으로 사용한 이유가 있을까요?

Copy link
Member Author

Choose a reason for hiding this comment

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

디자인에서는 텍스트가 그림의 중앙을 기준으로 수평을 유지하도록 구성되어 있는데, Row로 구성해보니 텍스트가 묘하게 중앙정렬이 안돼서 우선 ConstraintLayout으로 설정했습니다ㅠ

한번 더 Row로 시도해보겠습니다~!

Copy link
Member

@jihee-dev jihee-dev Jun 29, 2022

Choose a reason for hiding this comment

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

아하 이거 그 아이콘 높이랑 텍스트 높이가 달라서 텍스트가 세로로 중앙 정렬이 되지 않는 문제라면
row modifier에 verticalAlignment = Alignment.CenterVertically 적용해 보시면 해결할 수 있을 것 같아요!

Copy link
Member Author

@hoyahozz hoyahozz Jun 29, 2022

Choose a reason for hiding this comment

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

오 그거 넣고 했었는데 뭐라해야되지... 암튼 똑같이 텍스트가 묘하게 아래로 내려가있어서..............ㅎㅎ;;;;;;;;;;;;;;;;
움 한번 더 시도해볼게요! 생각해보니 바텀에 마진주는 식으로 해결할 수도 있을 것 같아여!

Copy link
Member

Choose a reason for hiding this comment

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

넵 좋아용

@jihee-dev
Copy link
Member

jihee-dev commented Jun 29, 2022

다른 화면 구현하다가 발견해서 아까 이해하지 못한 코드 이해하게 됐는데,
그 PlanText? 이게 그 월별 약속 리스트의 아이템인 것 같더라고요!
네이밍 추천하자면 TodayPlans - TodayPlanItem / Month(ly)Plans - Month(ly)PlanItem 어떠신가요
변경했으면 좋겠다고 리뷰만 드리고 더 좋은 네이밍을 생각해내지 못해서 찜찜했는데 생각난 김에 의견 남깁니당

class HomeViewModel @Inject constructor(
) : BaseViewModel<HomeViewState, HomeSideEffect, HomeEvent>(HomeViewState()) {

// TODO : 로그인 여부 체크(정호)
Copy link
Member

Choose a reason for hiding this comment

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

SplashViewModel 의 로그인 체크하는 부분 참고해주세요~

@jihee-dev
Copy link
Member

캘린더 밑 Padding이랑 그림자 추가하시고 머지 부탁드려용

@hoyahozz hoyahozz merged commit 90761d2 into develop Jul 5, 2022
@hoyahozz hoyahozz deleted the feature/issue-043-home-ui branch July 5, 2022 08:44
@KwonDae KwonDae modified the milestones: Sprint #5, Sprint #4 Jul 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants