Skip to content
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,18 @@ package com.daedan.festabook.presentation.placeMap
import android.content.Context
import android.os.Bundle
import android.view.View
import android.widget.AdapterView
import androidx.compose.foundation.background
import androidx.compose.foundation.layout.padding
import androidx.compose.runtime.getValue
import androidx.compose.ui.Modifier
import androidx.compose.ui.platform.ViewCompositionStrategy
import androidx.compose.ui.unit.dp
import androidx.fragment.app.Fragment
import androidx.fragment.app.FragmentTransaction
import androidx.fragment.app.commit
import androidx.fragment.app.viewModels
import androidx.lifecycle.ViewModelProvider
import androidx.lifecycle.compose.collectAsStateWithLifecycle
import androidx.lifecycle.lifecycleScope
import com.daedan.festabook.R
import com.daedan.festabook.databinding.FragmentPlaceMapBinding
Expand All @@ -31,7 +37,9 @@ import com.daedan.festabook.presentation.placeMap.placeCategory.PlaceCategoryFra
import com.daedan.festabook.presentation.placeMap.placeDetailPreview.PlaceDetailPreviewFragment
import com.daedan.festabook.presentation.placeMap.placeDetailPreview.PlaceDetailPreviewSecondaryFragment
import com.daedan.festabook.presentation.placeMap.placeList.PlaceListFragment
import com.daedan.festabook.presentation.placeMap.timeTagSpinner.adapter.TimeTagSpinnerAdapter
import com.daedan.festabook.presentation.placeMap.timeTagSpinner.component.TimeTagMenu
import com.daedan.festabook.presentation.theme.FestabookColor
import com.daedan.festabook.presentation.theme.FestabookTheme
import com.naver.maps.map.MapFragment
import com.naver.maps.map.NaverMap
import com.naver.maps.map.OnMapReadyCallback
Expand Down Expand Up @@ -88,23 +96,6 @@ class PlaceMapFragment(
savedInstanceState: Bundle?,
) {
super.onViewCreated(view, savedInstanceState)
binding.spinnerSelectTimeTag.onItemSelectedListener =
object : AdapterView.OnItemSelectedListener {
override fun onItemSelected(
parent: AdapterView<*>,
view: View?,
position: Int,
id: Long,
) {
val item = parent.getItemAtPosition(position) as TimeTag

onTimeTagSelected(item)
}

override fun onNothingSelected(parent: AdapterView<*>) {
onNothingSelected()
}
}
if (savedInstanceState == null) {
childFragmentManager.commit {
addWithSimpleTag(R.id.fcv_map_container, mapFragment)
Expand All @@ -118,6 +109,7 @@ class PlaceMapFragment(
}
lifecycleScope.launch {
setUpMapManager()
setupComposeView()
setUpObserver()
}
binding.logger.log(
Expand Down Expand Up @@ -169,22 +161,35 @@ class PlaceMapFragment(
}
}

private fun setUpObserver() {
viewModel.timeTags.observe(viewLifecycleOwner) { timeTags ->
// 타임태그가 없는 경우 메뉴 GONE
binding.layoutMapMenu.visibility =
if (timeTags.isNullOrEmpty()) View.GONE else View.VISIBLE

if (binding.spinnerSelectTimeTag.adapter == null) {
val adapter = TimeTagSpinnerAdapter(requireContext(), timeTags.toMutableList())
binding.spinnerSelectTimeTag.adapter = adapter
} else {
val adapter = binding.spinnerSelectTimeTag.adapter as TimeTagSpinnerAdapter
adapter.updateItems(timeTags)
adapter.notifyDataSetChanged()
private fun setupComposeView() {
binding.cvPlaceMap.apply {
setViewCompositionStrategy(ViewCompositionStrategy.DisposeOnViewTreeLifecycleDestroyed)
setContent {
FestabookTheme {
val timeTags by viewModel.timeTags.collectAsStateWithLifecycle(
viewLifecycleOwner,
)
if (!timeTags.isEmpty()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

isNotEmpty()를 써도 좋을 것 같아용

Copy link
Contributor

@parkjiminnnn parkjiminnnn Dec 2, 2025

Choose a reason for hiding this comment

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

collectAsStateWithLifecycle(viewLifecycleOwner) 이렇게 viewLifecycleOwner를 직접 명시해주는게 메모리 누수 측면에서 더 좋은가요?
명시 안해주었을 때 어떤 상황에서 문제가 발생하나요?(단순 질문)
제 코드엔 안되어 있어서 적용하려구요!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

앗 코멘트 남기는걸 까먹었네요.
처음에는 fragment의 생명주기임을 명시하려고 했지만
viewLifecycleOwner와 LocalLifeCycleOwner.current가 동일한 인스턴스라서 그냥 제거했습니다. !

val initialTitle = timeTags.first().name
TimeTagMenu(
initialTitle = initialTitle,
timeTags = timeTags,
onTimeTagClick = {
onTimeTagSelected(it)
},
modifier =
Modifier
.background(
FestabookColor.white,
).padding(horizontal = 24.dp),
)
}
}
}
}
}

private fun setUpObserver() {
viewModel.placeGeographies.observe(viewLifecycleOwner) { placeGeographies ->
when (placeGeographies) {
is PlaceListUiState.Loading -> Unit
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ import com.daedan.festabook.presentation.placeMap.model.toUiModel
import dev.zacsweers.metro.AppScope
import dev.zacsweers.metro.ContributesIntoMap
import dev.zacsweers.metro.Inject
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.launch

@ContributesIntoMap(AppScope::class)
Expand All @@ -38,8 +41,8 @@ class PlaceMapViewModel @Inject constructor(
val placeGeographies: LiveData<PlaceListUiState<List<PlaceCoordinateUiModel>>>
get() = _placeGeographies

private val _timeTags = MutableLiveData<List<TimeTag>>()
val timeTags: LiveData<List<TimeTag>> = _timeTags
private val _timeTags = MutableStateFlow<List<TimeTag>>(emptyList())
val timeTags: StateFlow<List<TimeTag>> = _timeTags.asStateFlow()
Copy link
Contributor

Choose a reason for hiding this comment

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

👍


private val _selectedTimeTag = MutableLiveData<TimeTag>()
val selectedTimeTag: LiveData<TimeTag> = _selectedTimeTag
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,185 @@
package com.daedan.festabook.presentation.placeMap.timeTagSpinner.component

import androidx.compose.foundation.background
import androidx.compose.foundation.border
import androidx.compose.foundation.clickable
import androidx.compose.foundation.interaction.MutableInteractionSource
import androidx.compose.foundation.layout.Arrangement
import androidx.compose.foundation.layout.Row
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.height
import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.layout.width
import androidx.compose.foundation.layout.wrapContentSize
import androidx.compose.material3.DropdownMenu
import androidx.compose.material3.DropdownMenuItem
import androidx.compose.material3.ExperimentalMaterial3Api
import androidx.compose.material3.ExposedDropdownMenuBox
import androidx.compose.material3.ExposedDropdownMenuBoxScope
import androidx.compose.material3.Icon
import androidx.compose.material3.MenuAnchorType
import androidx.compose.material3.Text
import androidx.compose.material3.TopAppBarDefaults
import androidx.compose.material3.ripple
import androidx.compose.runtime.Composable
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.rememberCoroutineScope
import androidx.compose.runtime.setValue
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.layout.onGloballyPositioned
import androidx.compose.ui.platform.LocalDensity
import androidx.compose.ui.res.painterResource
import androidx.compose.ui.tooling.preview.Preview
import androidx.compose.ui.unit.DpOffset
import androidx.compose.ui.unit.IntSize
import androidx.compose.ui.unit.dp
import com.daedan.festabook.R
import com.daedan.festabook.domain.model.TimeTag
import com.daedan.festabook.presentation.theme.FestabookColor
import com.daedan.festabook.presentation.theme.FestabookTheme
import com.daedan.festabook.presentation.theme.FestabookTypography
import com.daedan.festabook.presentation.theme.festabookShapes
import com.daedan.festabook.presentation.theme.festabookSpacing
import kotlinx.coroutines.delay
import kotlinx.coroutines.launch

@OptIn(ExperimentalMaterial3Api::class)
@Composable
fun TimeTagMenu(
initialTitle: String,
timeTags: List<TimeTag>,
modifier: Modifier = Modifier,
onTimeTagClick: (TimeTag) -> Unit = {},
) {
var title by remember { mutableStateOf(initialTitle) }
Copy link
Contributor

Choose a reason for hiding this comment

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

title은 서버에서 오는 데이터기 때문에 사용자 클릭에 따라 값이 변경되면 onClick관련 메서드를 viewModel에서 만들어서 넘겨주는 것도 좋아보이는데 어떠신가요? 사소해보이긴 하는데 밀러의 의견이 궁금합니다! (진짜 궁금)

ViewModel이 한층 더 뚱뚱해진다는 단점이 있지만 stateHolder = ViewModel이라는 관점에서 봤을 때 나쁘지 않아보이고 컴포저블 함수가 StateLess하게 된다는 점도 있어보입니다!

Copy link
Contributor Author

@oungsi2000 oungsi2000 Dec 3, 2025

Choose a reason for hiding this comment

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

저는 title은 클라이언트 측에서 정해주는 데이터라고 생각했습니다.
또한 TimeTagMenu 컴포저블을 호출했을 때, 개발자가 기대하는 동작 중 하나는 내가 따로 설정하지 않아도 선택된 타임태그의 이름을 표시한다 라고 생각해서 title State를 TimeTagMenu 내부로 넣었습니다 !

하지만 곰곰이 생각해보니, timeTags를 서버에서 가져올 때, selectedTimeTag 또한 같이 정해줍니다.
그런데 TimeTagMenu에서 UI용 상태를 하나 뚫게 되면, viewModel의 selectedTimeTag와 UI의 상태가 불일치될 여지가 있네요.
단일 진실 공급원을 위반합니다.

때문에 제이의 말씀대로 viewModel에서 처리하는게 더 낫다고 생각이 들었어요.

임시 StateFlow를 생성하여 title을 외부 컴포저블로
반영했습니다 ~!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

onClick관련 메서드를 viewModel에서 만들어서 넘겨주는 것도 좋아보이는데 어떠신가요?

viewModel.onDaySelected로 통합했습니다.

var expanded by remember { mutableStateOf(false) }
var dropdownWidth by remember { mutableStateOf(IntSize.Zero) }
val density = LocalDensity.current
val scope = rememberCoroutineScope()

Row(
modifier = modifier.fillMaxWidth(),
) {
ExposedDropdownMenuBox(
modifier =
Modifier
.wrapContentSize()
.background(Color.Transparent),
expanded = expanded,
onExpandedChange = { expanded = !expanded },
) {
TimeTagButton(
title = title,
onSizeDetermined = { dropdownWidth = it },
)
DropdownMenu(
Copy link
Contributor

Choose a reason for hiding this comment

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

오 compose에서는 dropdown으로 바로 만들 수 있네요?! 👀

expanded = expanded,
onDismissRequest = { expanded = false },
offset = DpOffset(x = 0.dp, y = festabookSpacing.paddingBody2),
modifier =
Modifier
.width(
with(density) { dropdownWidth.width.toDp() },
).background(color = FestabookColor.white)
.border(
width = 2.dp,
color = FestabookColor.gray300,
shape = festabookShapes.radius2,
),
Copy link
Contributor

Choose a reason for hiding this comment

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

제가 CardBackgound함수 만들어놨는데 그거 사용해주셔도 좋을 것 같아요!!

shape = festabookShapes.radius2,
) {
timeTags.forEach { item ->
DropdownMenuItem(
text = {
Text(
text = item.name,
style = FestabookTypography.bodyLarge,
Copy link
Contributor

Choose a reason for hiding this comment

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

MaterialTheme.typography.bodyLarge로 바꾸면 적용 테마에 따라 자동으로 바꿔줄 수 있을 것 같아용

Copy link
Contributor Author

Choose a reason for hiding this comment

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

festabookTyphography 를 하나 만들어도 좋을 것 같아요 !

)
},
onClick = {
scope.launch {
title = item.name
onTimeTagClick(item)
waitForRipple {
expanded = false
}
}
},
)
}
}
}
}
}

@Composable
@OptIn(ExperimentalMaterial3Api::class)
private fun ExposedDropdownMenuBoxScope.TimeTagButton(
title: String,
onSizeDetermined: (IntSize) -> Unit,
) {
Row(
modifier =
Modifier
.width(140.dp)
.onGloballyPositioned { coordinates ->
onSizeDetermined(coordinates.size)
}.menuAnchor(
type = MenuAnchorType.PrimaryNotEditable,
enabled = true,
).height(TopAppBarDefaults.MediumAppBarCollapsedHeight) // Festabook TopAppbar Size
.background(Color.Transparent)
.clickable(
onClick = {},
interactionSource = remember { MutableInteractionSource() },
indication = ripple(),
),
verticalAlignment = Alignment.CenterVertically,
horizontalArrangement = Arrangement.SpaceBetween,
) {
Text(
text = title,
style = FestabookTypography.displaySmall,
Copy link
Contributor

Choose a reason for hiding this comment

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

여기도요!

)

Icon(
painter = painterResource(id = R.drawable.ic_chevron_down),
contentDescription = "드롭다운",
Copy link
Contributor

Choose a reason for hiding this comment

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

👀

)
}
}

private suspend inline fun waitForRipple(
timeMillis: Long = 100,
after: () -> Unit = {},
) {
delay(timeMillis)
after()
}
Comment on lines +155 to +161
Copy link
Contributor

Choose a reason for hiding this comment

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

요친구 역할을 알 수 있을까요?(진짜 모름..)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

delay를 걸어주지 않으면 TimeTag를 클릭했을 때 너무 빨리 사라져서 클릭 리플 효과가 발생하지 않더라구요 !
XML View에서는 리플 효과가 있어서 Compose에서도 반영하려고 저 코드를 넣었습니다.
(전반적으로 인터랙션이 없어서 굉장히 밋밋했어요. )


@Composable
@Preview(showBackground = true)
private fun TimeTagMenuPreview() {
val timeTags =
listOf(
TimeTag(1, "1일차 오전"),
TimeTag(2, "오후"),
)
var title by remember { mutableStateOf("1일차 오전") }
FestabookTheme {
TimeTagMenu(
initialTitle = title,
timeTags = timeTags,
modifier =
Modifier
.background(FestabookColor.white)
.padding(horizontal = festabookSpacing.paddingScreenGutter),
// Festabook Gutter
onTimeTagClick = { },
)
}
}
Loading
Loading