Conversation
| private lateinit var binding: ActivityEventDetailBinding | ||
| private val viewModel: EventDetailViewModel by viewModels { EventDetailViewModel.factory } | ||
| private val eventId: Long by lazy { | ||
| intent.getLongExtra(EVENT_ID_KEY, DEFAULT_EVENT_ID).apply { |
| INFORMATION_TAB_POSITION -> tab.text = "상세 정보" | ||
| COMMENT_TAB_POSITION -> tab.text = "댓글" | ||
| PARTICIPANT_TAB_POSITION -> tab.text = "같이가요" |
There was a problem hiding this comment.
문자열 리소스로 추출하지 않은 거 가슴이 아프네요ㅠㅠ
There was a problem hiding this comment.
나중에 한번에 하자고 하셔서... ㅠㅠ
| import com.google.android.material.tabs.TabLayoutMediator | ||
|
|
||
| class EventDetailActivity : AppCompatActivity() { | ||
| private lateinit var binding: ActivityEventDetailBinding |
There was a problem hiding this comment.
private val binding: ActivityEventDetailBinding by lazy { ActivityEventDetailBinding.inflate(layoutInflater) }
이렇게 var를 val로 바꿔도 괜찮을 것 같습니다 😄
| val eventDetail: LiveData<EventDetailUiState> | ||
| get() = _eventDetail | ||
|
|
||
| private val _participation: MutableLiveData<Boolean> = MutableLiveData() |
There was a problem hiding this comment.
타입이 Boolean이라면 participation이라는 명사보다 isParticipated 같은 동사구가 국룰 아닌가요? ❓
There was a problem hiding this comment.
여기서 필요 없는 변수라 지웠습니다~ 제가 참가자 목록이랑 함께 작업하다 보니 섞인게 많아요 ㅎㅎ
| import com.emmsale.R | ||
| import com.emmsale.presentation.utils.extension.px | ||
|
|
||
| class EventTag : AppCompatCheckBox { |
There was a problem hiding this comment.
시간이 없어서 부나의 코드를 재사용한 것은 알지만, 이벤트 태그는 텍스트뷰가 더 어울린 것 같습니다ㅠㅠ
There was a problem hiding this comment.
저도 급하게 코드를 작성하느라 CheckBox의 checkable 속성을 false로 설정해두고 사용하고 있지만,
선택할 필요가 없는 View이기 때문에 TextView로 변경하는 것이 좋을 것 같습니다 : )
There was a problem hiding this comment.
그렇겠네요 오래 안걸리면 바꿔볼게요
| android:layout_height="wrap_content" | ||
| android:layout_marginStart="17dp" | ||
| android:ems="12" | ||
| android:text="@{eventDetail.name}" |
There was a problem hiding this comment.
eventDetail은 EventDetailUiState.Success 타입인데 observable을 구현한 타입이 아니라서 변경을 관찰할 수 없습니다.
나중에 새로고침 기능을 추가한다면 문제가 생길 것 같습니다 😢
There was a problem hiding this comment.
EventDetail 을 새로고침할 일이 없을 것 같긴 해서... 이 작업은 시간이 오래걸릴 것 같고 논의 해야할 사항이라서 일단은 나중에 더 고려해보겠습니다.
| import com.emmsale.data.common.ApiResult | ||
|
|
||
| interface EventDetailRepository { | ||
| suspend fun fetchEventDetail(eventId: Long): ApiResult<EventDetail> |
There was a problem hiding this comment.
컨벤션에 따르면 반환값이 있으므로 getXxx 네이밍이 맞습니당ㅠㅠ
viewModel에서는 데이터를 불러오는 로직에 반환값이 없으므로 fetchXxx를 사용합니다.
There was a problem hiding this comment.
이전에 토마스 PR에도 남긴 리뷰입니다.
위에 남겨주신 리뷰를 조금 더 구체적으로 작성한 예시입니다.
There was a problem hiding this comment.
이 부분도 사실 다시 고려해 봐야할 것 같아요. viewModel 에서만 fetch를 쓰고 서버에서는 get 을 쓰는게 좀 어색합니다. fetch 가 좀더 서버에서 쓰이는 언어같은데... 일단은 놔두고 다음 회의 때 한번 더 정했으면 좋겠습니다.
| EventDetailUiState.from( | ||
| result.data, | ||
| ), |
#️⃣연관된 이슈
#111
📝작업 내용
행사 상세정보 화면 구현했어요.
스크린샷 (선택)
예상 소요 시간 및 실제 소요 시간
💬리뷰 요구사항(선택)
EventDetail 부분만 봐주세요. 너무 아니다 싶은 부분만 리뷰 달아주세요 머지가 시급합니다.
presentation 리뷰 커밋
data 리뷰 커밋