Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit b4ddeff7 authored by Ellen Poe's avatar Ellen Poe
Browse files

fix: review comments

parent da71e7e9
Loading
Loading
Loading
Loading
Loading
+3 −4
Original line number Diff line number Diff line
@@ -99,14 +99,13 @@ fun NearbyScreenContent(viewModel: NearbyViewModel, onPlaceSelected: (Place) ->
        }

        FlowRow {
            val selectedCategories by viewModel.selectedCategories.collectAsState()
            for (chipSpec in viewModel.allCategories) {
                var isSelected by remember { mutableStateOf(false) }
                FilterChip(
                    modifier = Modifier.padding(end = 8.dp),
                    selected = isSelected,
                    selected = selectedCategories.contains(chipSpec.category),
                    onClick = {
                        isSelected = !isSelected
                        viewModel.setCategorySelection(chipSpec.category, isSelected)
                        viewModel.toggleCategorySelection(chipSpec.category)
                    },
                    label = {
                        val label = stringResource(chipSpec.labelResource)
+15 −12
Original line number Diff line number Diff line
@@ -20,6 +20,7 @@ package earth.maps.cardinal.ui.home

import android.location.Location
import android.util.Log
import androidx.annotation.VisibleForTesting
import androidx.lifecycle.ViewModel
import androidx.lifecycle.viewModelScope
import dagger.hilt.android.lifecycle.HiltViewModel
@@ -27,7 +28,6 @@ import earth.maps.cardinal.R.string
import earth.maps.cardinal.data.LocationRepository
import earth.maps.cardinal.data.Place
import earth.maps.cardinal.geocoding.GeocodingService
import kotlinx.coroutines.FlowPreview
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.asStateFlow
@@ -59,7 +59,8 @@ class NearbyViewModel @Inject constructor(

    private var lastLocation: Location? = null

    private val selectedCategories: MutableSet<String> = mutableSetOf()
    private val _selectedCategories = MutableStateFlow<Set<String>>(emptySet())
    val selectedCategories = _selectedCategories.asStateFlow()

    init {
        // Start observing location updates
@@ -77,11 +78,11 @@ class NearbyViewModel @Inject constructor(
        FilterChipSpec("accommodation", string.category_accommodation),
    )

    fun setCategorySelection(category: String, selection: Boolean) {
        if (selection) {
            selectedCategories.add(category)
    fun toggleCategorySelection(category: String) {
        if (_selectedCategories.value.contains(category)) {
            _selectedCategories.value = _selectedCategories.value.minus(category)
        } else {
            selectedCategories.remove(category)
            _selectedCategories.value = _selectedCategories.value.plus(category)
        }
        refreshData()
    }
@@ -90,17 +91,17 @@ class NearbyViewModel @Inject constructor(
     * Starts observing location updates from the LocationRepository.
     * This method is made open for testing purposes.
     */
    @OptIn(FlowPreview::class)
    @VisibleForTesting
    fun startLocationObservation() {
        viewModelScope.launch {
            locationRepository.locationFlow.distinctUntilChanged { pos1, pos2 ->
            locationRepository.locationFlow.distinctUntilChanged(areEquivalent = { pos1, pos2 ->
                // Change if either is null (*new* null values are filtered by the null check in collectLatest
                if (pos1 == null || pos2 == null) {
                    return@distinctUntilChanged false
                }
                // Only update if location changed significantly (more than 250 meters)
                pos2.distanceTo(pos1) < 250f
            }.collectLatest { location ->
                pos2.distanceTo(pos1) < 250f // true means equivalency
            }).collectLatest { location ->
                location?.let {
                    lastLocation = it
                    Log.d(TAG, "Location updated: $lastLocation")
@@ -110,15 +111,17 @@ class NearbyViewModel @Inject constructor(
        }
    }

    @VisibleForTesting
    fun getSelectedCategoriesSet(): Set<String> {
        return selectedCategories.toSet()
        return selectedCategories.value.toSet()
    }

    suspend fun fetchNearby(latitude: Double, longitude: Double) {
        _isLoading.value = true
        _error.value = null
        try {
            _nearbyResults.value = geocodingService.nearby(latitude, longitude, selectedCategories.toList())
            _nearbyResults.value =
                geocodingService.nearby(latitude, longitude, selectedCategories.value.toList())
            _isLoading.value = false
        } catch (e: Exception) {
            _error.value = e.message ?: "Error fetching nearby places"
+5 −5
Original line number Diff line number Diff line
@@ -254,14 +254,14 @@ class NearbyViewModelTest {
        assertTrue(viewModel.getSelectedCategoriesSet().isEmpty())
        
        // Add categories
        viewModel.setCategorySelection(category1, true)
        viewModel.setCategorySelection(category2, true)
        viewModel.toggleCategorySelection(category1)
        viewModel.toggleCategorySelection(category2)
        
        // Verify categories were added
        assertEquals(setOf(category1, category2), viewModel.getSelectedCategoriesSet())
        
        // Remove one category
        viewModel.setCategorySelection(category1, false)
        viewModel.toggleCategorySelection(category1)
        
        // Verify remaining category
        assertEquals(setOf(category2), viewModel.getSelectedCategoriesSet())
@@ -288,8 +288,8 @@ class NearbyViewModelTest {
        io.mockk.clearMocks(mockGeocodingService)

        // Add categories
        viewModel.setCategorySelection("food", true)
        viewModel.setCategorySelection("health", true)
        viewModel.toggleCategorySelection("food")
        viewModel.toggleCategorySelection("health")

        // Allow coroutines to complete
        advanceUntilIdle()