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

노래 재생 화면에서 플레이 리스트에 추가 #296

Merged
merged 12 commits into from
Dec 7, 2023
Merged
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.ohdodok.catchytape.core.data.api

import com.ohdodok.catchytape.core.data.model.AddMusicToPlaylistRequest
import com.ohdodok.catchytape.core.data.model.MusicResponse
import com.ohdodok.catchytape.core.data.model.PlaylistRequest
import com.ohdodok.catchytape.core.data.model.PlaylistResponse
Expand All @@ -22,4 +23,10 @@ interface PlaylistApi {
suspend fun getPlaylist(
@Path("playlistId")playlistId: Int
): List<MusicResponse>

@POST("playlists/{playlistId}")
suspend fun postMusicToPlaylist(
@Path("playlistId")playlistId: Int,
@Body music: AddMusicToPlaylistRequest,
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package com.ohdodok.catchytape.core.data.model

import kotlinx.serialization.Serializable

@Serializable
data class AddMusicToPlaylistRequest(
val musicId: String
)
Comment on lines +5 to +8
Copy link
Member

Choose a reason for hiding this comment

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

오 이거 제가 올린 PR에도 생성된 requestbody 인데 자주 쓰일 것 같아서 저는 MusicIdRequest 로 클래스를 만들었습니당! 이따 통일하는 게 좋을 것 같네요~!

Copy link
Member Author

Choose a reason for hiding this comment

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

MusicIdRequest가 더 간단해서 좋네요

Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package com.ohdodok.catchytape.core.data.repository

import com.ohdodok.catchytape.core.data.api.PlaylistApi
import com.ohdodok.catchytape.core.data.api.UserApi
import com.ohdodok.catchytape.core.data.model.AddMusicToPlaylistRequest
import com.ohdodok.catchytape.core.data.model.PlaylistRequest
import com.ohdodok.catchytape.core.data.model.toDomains
import com.ohdodok.catchytape.core.domain.model.Music
Expand Down Expand Up @@ -35,6 +36,13 @@ class PlaylistRepositoryImpl @Inject constructor(
val response = playlistApi.getPlaylist(playlistId)
emit(response.toDomains())
}

override suspend fun addMusicToPlaylist(playlistId: Int, musicId: String) {
playlistApi.postMusicToPlaylist(
playlistId = playlistId,
music = AddMusicToPlaylistRequest(musicId),
)
}
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,6 @@ interface PlaylistRepository {
fun getRecentPlaylist(): Flow<List<Music>>

fun getPlaylist(playlistId: Int): Flow<List<Music>>

suspend fun addMusicToPlaylist(playlistId: Int, musicId: String)
}
1 change: 1 addition & 0 deletions android/core/ui/build.gradle.kts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
@Suppress("DSL_SCOPE_VIOLATION") // TODO: Remove once KTIJ-19369 is fixed
plugins {
id("catchytape.android.library")
id("catchytape.android.hilt")
}

android {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
package com.ohdodok.catchytape.feature.playlist
package com.ohdodok.catchytape.core.ui

import android.view.LayoutInflater
import android.view.ViewGroup
import androidx.recyclerview.widget.DiffUtil
import androidx.recyclerview.widget.ListAdapter
import androidx.recyclerview.widget.RecyclerView
import com.ohdodok.catchytape.feature.playlist.databinding.ItemPlaylistBinding
import com.ohdodok.catchytape.feature.playlist.model.PlaylistUiModel
import com.ohdodok.catchytape.core.ui.databinding.ItemPlaylistBinding
import com.ohdodok.catchytape.core.ui.model.PlaylistUiModel

class PlaylistAdapter :
ListAdapter<PlaylistUiModel, PlaylistAdapter.PlaylistViewHolder>(PlaylistItemDiffUtil) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package com.ohdodok.catchytape.core.ui

import android.os.Bundle
import android.view.LayoutInflater
import android.view.View
import android.view.ViewGroup
import androidx.fragment.app.viewModels
import androidx.lifecycle.Lifecycle
import androidx.lifecycle.lifecycleScope
import androidx.lifecycle.repeatOnLifecycle
import androidx.navigation.fragment.findNavController
import com.google.android.material.bottomsheet.BottomSheetDialogFragment
import com.ohdodok.catchytape.core.ui.databinding.BottomSheetPlaylistBinding
import dagger.hilt.android.AndroidEntryPoint
import kotlinx.coroutines.launch

@AndroidEntryPoint
class PlaylistBottomSheet : BottomSheetDialogFragment() {
Copy link
Collaborator

@2taezeat 2taezeat Dec 7, 2023

Choose a reason for hiding this comment

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

BottomSheetDialogFragment() 을 사용해서, Bottom Player가 안보이는 등, 적어주셨던 것 처럼 UI에 문제가 생기는 것거 같은데,

Fragment를 띄우면 하단 재생 바가 자기 부른 줄 알고 같이 올라옴
이유는 모르겠으나 바텀 시트가 떠 있는 동안 재생 버튼이 사라짐
UI radius 추가하거나 디자인을 바꿔야 함

최종발표까지 시간이 얼마 안남았기에, BottomSheetDialog 나 아니면 단순하게 Dialog 등으로 하는 것도 하나의 방법일 것 같아요.
(UI 문제가 빠르게 해결되면, 지금 상태로 해도 당연히 좋습니다.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Fragment가 아닌 클래스가 있었군요
그런데 지금은 Navigation과 Fragment의 기능을 사용하고 있기 때문에, 변경해야 할 지점이 많아요. 일단 이슈를 만들어 놓고 머지할게요. 모달 바텀 시트의 배경이라 기능상 문제가 있거나 크게 신경쓰이는 부분은 아닙니다!

val viewModel: PlaylistBottomSheetViewModel by viewModels()

private var _binding: BottomSheetPlaylistBinding? = null
private val binding get() = _binding!!

override fun onCreateView(
inflater: LayoutInflater,
container: ViewGroup?,
savedInstanceState: Bundle?
): View {
_binding = BottomSheetPlaylistBinding.inflate(inflater, container, false)
binding.lifecycleOwner = viewLifecycleOwner
binding.viewModel = viewModel
binding.rvPlaylists.adapter = PlaylistAdapter()

observeEvent()

return binding.root
}

private fun observeEvent() {
viewLifecycleOwner.lifecycleScope.launch {
repeatOnLifecycle(Lifecycle.State.STARTED) {
viewModel.closeEvent.collect {
findNavController().popBackStack()
}
}
}
}

override fun onDestroyView() {
super.onDestroyView()
_binding = null
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package com.ohdodok.catchytape.core.ui

import androidx.lifecycle.SavedStateHandle
import androidx.lifecycle.ViewModel
import androidx.lifecycle.viewModelScope
import com.ohdodok.catchytape.core.domain.model.Playlist
import com.ohdodok.catchytape.core.domain.repository.PlaylistRepository
import com.ohdodok.catchytape.core.ui.model.PlaylistUiModel
import dagger.hilt.android.lifecycle.HiltViewModel
import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.SharedFlow
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.asSharedFlow
import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.launch
import javax.inject.Inject

@HiltViewModel
class PlaylistBottomSheetViewModel @Inject constructor(
savedStateHandle: SavedStateHandle,
private val playlistRepository: PlaylistRepository,
) : ViewModel() {

private val musicId: String = requireNotNull(savedStateHandle["musicId"]) {
"musicId 정보가 누락되었어요."
}

private val _playlists = MutableStateFlow(emptyList<PlaylistUiModel>())
val playlists: StateFlow<List<PlaylistUiModel>> = _playlists.asStateFlow()

private val _closeEvent = MutableSharedFlow<Unit>()
val closeEvent: SharedFlow<Unit> = _closeEvent.asSharedFlow()

init {
fetchPlaylists()
}

private fun fetchPlaylists() {
viewModelScope.launch {
_playlists.value = playlistRepository.getPlaylists().first().toUiModels()
}
}

private fun addMusicToPlaylist(playlistId: Int, musicId: String) {
viewModelScope.launch {
playlistRepository.addMusicToPlaylist(playlistId = playlistId, musicId = musicId)
_closeEvent.emit(Unit)
}
}

private fun List<Playlist>.toUiModels(): List<PlaylistUiModel> = this.map { it.toUiModel() }

private fun Playlist.toUiModel(): PlaylistUiModel {
Comment on lines +53 to +55
Copy link
Member

Choose a reason for hiding this comment

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

함수명을 toPresentation 으로 하는 것 어떨까욥? toDomain 과 맥락이 동일하게!

Copy link
Member Author

Choose a reason for hiding this comment

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

나는 조금 다르게 생각했어. UiModel은 특정 UI 컴포넌트에서만 사용하는 개념으로 봤어.
만약 presentation이라고 짓는 거라면 UI의 형태와 관계 없이 RecyclerView든 어떤 자세히보기 화면이든 관계 없이 사용할 수 있어야 할 것 같아. 이 경우엔 특정 RecyclerView에서만 사용할 수 있는 data class라고 생각해서 presentation에서 사용하는 model보다는 특정 ui에서 사용하는 model이다 라는 의미로 사용했어

Copy link
Member

Choose a reason for hiding this comment

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

toPresentation == toUiModel 은 같은 의미라고 느껴져! 적어둔 이유처럼 특정 ui컴포넌트만을 위한 것이라면 어떤 컴포넌트의 UiModel인지 함수명을 바꿔야하지 않을까? 그게 아니라면 toPresentation 도 괜찮은 것 같아

return PlaylistUiModel(
id = id,
title = title,
thumbnailUrl = thumbnailUrl,
trackSize = trackSize,
onClick = {
addMusicToPlaylist(
playlistId = id,
musicId = musicId
)
}
)
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package com.ohdodok.catchytape.feature.playlist.model
package com.ohdodok.catchytape.core.ui.model

data class PlaylistUiModel(
val id: Int,
Expand Down
59 changes: 59 additions & 0 deletions android/core/ui/src/main/res/layout/bottom_sheet_playlist.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
<?xml version="1.0" encoding="utf-8"?>
<layout xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:app="http://schemas.android.com/apk/res-auto"
xmlns:tools="http://schemas.android.com/tools">

<data>
<variable
name="viewModel"
type="com.ohdodok.catchytape.core.ui.PlaylistBottomSheetViewModel" />
</data>

<androidx.constraintlayout.widget.ConstraintLayout
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:background="@color/surface_bright">

<TextView
android:id="@+id/tv_title"
style="@style/TitleMedium"
android:layout_width="0dp"
android:layout_height="wrap_content"
android:layout_marginHorizontal="@dimen/margin_horizontal"
android:layout_marginTop="@dimen/extra_large"
android:text="@string/add_to_playlist"
android:textColor="@color/on_surface"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toTopOf="parent" />

<com.google.android.material.divider.MaterialDivider
android:id="@+id/divider"
android:layout_width="0dp"
android:layout_height="1dp"
android:layout_marginTop="@dimen/medium"
app:dividerColor="@color/outline_variant"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toBottomOf="@id/tv_title" />

<androidx.recyclerview.widget.RecyclerView
android:id="@+id/rv_playlists"
android:layout_width="0dp"
android:layout_height="wrap_content"
android:layout_marginTop="8dp"
android:layout_marginBottom="8dp"
android:maxHeight="400dp"
android:orientation="vertical"
android:paddingHorizontal="@dimen/margin_horizontal"
app:layoutManager="androidx.recyclerview.widget.LinearLayoutManager"
app:layout_constraintBottom_toBottomOf="parent"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toBottomOf="@id/divider"
app:list="@{viewModel.playlists}"
tools:itemCount="3"
tools:listitem="@layout/item_music_vertical" />

</androidx.constraintlayout.widget.ConstraintLayout>
</layout>
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

<variable
name="playlist"
type="com.ohdodok.catchytape.feature.playlist.model.PlaylistUiModel" />
type="com.ohdodok.catchytape.core.ui.model.PlaylistUiModel" />
</data>

<androidx.constraintlayout.widget.ConstraintLayout
Expand Down
3 changes: 2 additions & 1 deletion android/core/ui/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
<string name="description">설명</string>
<string name="select_thumbnail">썸네일 이미지 선택</string>


<string name="track_size">트랙 %d개</string>
Copy link
Member

Choose a reason for hiding this comment

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

이거 playlist 모듈 strings 파일에 있는데 playlist 모듈에서 삭제한번 해주세요!~

Copy link
Member Author

Choose a reason for hiding this comment

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

넹 삭제할게여


<string name="error_message_connection">"error_message_connection"</string>
<string name="error_message_ssl_hand_shake">"error_message_ssl_hand_shake"</string>
Expand All @@ -53,5 +53,6 @@
<string name="error_message_not_exist_genre">"error_message_not_exist_genre"</string>
<string name="error_message_expired_token">"error_message_expired_token"</string>
<string name="error_message_service">"error_message_service"</string>
<string name="add_to_playlist">재생 목록에 노래 추가</string>

</resources>
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@ class PlayerFragment : BaseFragment<FragmentPlayerBinding>(R.layout.fragment_pla
binding.btnPrevious.setOnClickListener {
player.movePreviousMedia()
}

binding.btnAddToPlaylist.setOnClickListener {
findNavController().showPlaylistBottomSheet()
}
}

private fun collectEvents() {
Expand All @@ -75,6 +79,13 @@ class PlayerFragment : BaseFragment<FragmentPlayerBinding>(R.layout.fragment_pla
}
}
}

private fun NavController.showPlaylistBottomSheet() {
val musicId = viewModel.uiState.value.currentMusic?.id ?: return

val action = PlayerFragmentDirections.actionPlayerFragmentToPlaylistBottomSheet(musicId = musicId)
this.navigate(action)
}
}

fun NavController.navigateToPlayer() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,21 @@
tools:layout="@layout/fragment_player">

<deepLink app:uri="android-app://com.ohdodok.catchytape/player_fragment" />

<action
android:id="@+id/action_player_fragment_to_playlist_bottom_sheet"
app:destination="@id/playlist_bottom_sheet" />

</fragment>

<dialog
android:id="@+id/playlist_bottom_sheet"
android:name="com.ohdodok.catchytape.core.ui.PlaylistBottomSheet"
android:label="playlist bottom sheet"
tools:layout="@layout/bottom_sheet_playlist">

<argument
android:name="musicId"
app:argType="string" />
</dialog>

</navigation>
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import androidx.fragment.app.viewModels
import androidx.navigation.NavController
import androidx.navigation.fragment.findNavController
import com.ohdodok.catchytape.core.ui.BaseFragment
import com.ohdodok.catchytape.core.ui.PlaylistAdapter
import com.ohdodok.catchytape.core.ui.toMessageId
import com.ohdodok.catchytape.feature.playlist.databinding.FragmentPlaylistsBinding
import dagger.hilt.android.AndroidEntryPoint
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import com.ohdodok.catchytape.core.domain.model.CtErrorType
import com.ohdodok.catchytape.core.domain.model.CtException
import com.ohdodok.catchytape.core.domain.model.Playlist
import com.ohdodok.catchytape.core.domain.repository.PlaylistRepository
import com.ohdodok.catchytape.feature.playlist.model.PlaylistUiModel
import com.ohdodok.catchytape.core.ui.model.PlaylistUiModel
import dagger.hilt.android.lifecycle.HiltViewModel
import kotlinx.coroutines.CoroutineExceptionHandler
import kotlinx.coroutines.flow.MutableSharedFlow
Expand Down