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

Scalebar: Refactor ViewModel #715

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
38eea4e
WIP
puneet-pdx Jan 16, 2025
c72fab4
WIP
puneet-pdx Jan 17, 2025
dad7ec5
Update tests
puneet-pdx Jan 17, 2025
317352c
update maxLength calc
puneet-pdx Jan 17, 2025
fe2ec0b
update test
puneet-pdx Jan 17, 2025
72de31b
fix bug where the scalebar was not displaying fraction values
puneet-pdx Jan 17, 2025
387b32e
optimize imports
puneet-pdx Jan 18, 2025
fd887a7
Refactor fun names
puneet-pdx Jan 18, 2025
ff0c93c
add doc
puneet-pdx Jan 18, 2025
7ed31a2
address code review comments
puneet-pdx Jan 21, 2025
ef93e8b
make measureMinSegmentWidth private
puneet-pdx Jan 21, 2025
a62a3d1
remove showScalebar fun
puneet-pdx Jan 21, 2025
824558e
optimize import
puneet-pdx Jan 21, 2025
698603c
Revert "optimize import"
puneet-pdx Jan 21, 2025
4c61fae
Revert "remove showScalebar fun"
puneet-pdx Jan 21, 2025
b589861
Revert "make measureMinSegmentWidth private"
puneet-pdx Jan 21, 2025
2a7bd00
working proto
eri9000 Jan 21, 2025
b75e62c
Merge branch 'feature-branches/scalebar' into Erick/copyOf_5189
eri9000 Jan 21, 2025
34cfa11
Update Scalebar.kt
eri9000 Jan 21, 2025
cbbc8a5
working protto 2
eri9000 Jan 22, 2025
2c4b125
updates test
eri9000 Jan 22, 2025
6858d1d
rename a few properties
eri9000 Jan 22, 2025
5d6dd45
update method signature
eri9000 Jan 22, 2025
f2317b6
update names
eri9000 Jan 22, 2025
adc6cd5
rename composable to match guidelines
eri9000 Jan 23, 2025
f6cac20
update retrieval of last label or empty if there's none
eri9000 Jan 23, 2025
783b76e
refactor names
eri9000 Jan 23, 2025
50d56d0
Merge branch 'feature-branches/scalebar' into Erick/scalebar-internal…
eri9000 Jan 23, 2025
46767d8
additional merge fixes
eri9000 Jan 23, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,9 @@ fun MainScreen(modifier: Modifier) {
mutableStateOf(
ArcGISMap(BasemapStyle.ArcGISTopographic).apply {
initialViewpoint = Viewpoint(
latitude = 39.8,
longitude = -98.6,
scale = 10e7
latitude = 33.751765,
longitude = -117.935518,
scale = 44016.65596653387
)
}
)
Expand All @@ -78,7 +78,7 @@ fun MainScreen(modifier: Modifier) {
.align(Alignment.BottomStart)
) {
Scalebar(
maxWidth = 175.0,
maxDisplayWidth = 175.0,
Copy link
Collaborator

Choose a reason for hiding this comment

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

this should just be maxWidth as the user is only concerned with what is shown on the display. From the user perspective there is only the width of the Scalebar being shown on the screen. I think this should simply be maxWidth

unitsPerDip = unitsPerDip,
viewpoint = viewpoint,
spatialReference = spatialReference,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
/*
* Copyright 2025 Esri
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.arcgismaps.toolkit.scalebar

import androidx.compose.ui.test.junit4.createComposeRule
import androidx.compose.ui.text.TextStyle
import androidx.compose.ui.unit.sp
import com.arcgismaps.geometry.Point
import com.arcgismaps.geometry.SpatialReference
import com.arcgismaps.mapping.Viewpoint
import com.arcgismaps.toolkit.scalebar.theme.LabelTypography
import com.google.common.truth.Truth.assertThat
import kotlinx.coroutines.test.runTest
import org.junit.Rule
import org.junit.Test

class ScalebarCalculationsTest {
@get:Rule
val composeTestRule = createComposeRule()

private val esriRedlands = Point(-13046081.04434825, 4036489.208008117, SpatialReference.webMercator())
private val defaultLabelTypography = LabelTypography(labelStyle = TextStyle(fontSize = 11.sp))

/**
* Given map properties and device properties
* When the properties of a line style scalebar are computed
* Then the display length and labels should be correct
*
* @since 200.7.0
*/
@Test
fun testLineStyle() {
testScalebarCalculations(
point = esriRedlands,
style = ScalebarStyle.Line,
maxWidth = 175.0,
units = ScalebarUnits.METRIC,
scale = 10000000.0,
unitsPerDip = 2645.833333330476,
labelTypography = defaultLabelTypography,
expectedDisplayLength = 171.0,
expectedLabels = listOf("375 km")
)
}

/**
* Given map properties and device properties
* When the properties of a bar scalebar are calculated
* Then the display length and labels should be correct
*
* @since 200.7.0
*/
@Test
fun testBarStyle() = runTest {
testScalebarCalculations(
point = esriRedlands,
style = ScalebarStyle.Bar,
maxWidth = 175.0,
units = ScalebarUnits.METRIC,
scale = 10000000.0,
unitsPerDip = 2645.833333330476,
labelTypography = defaultLabelTypography,
expectedDisplayLength = 171.0,
expectedLabels = listOf("375 km")
)
}


/**
* Executes the scalebar calculations and verifies the results.
*
* @since 200.7.0
*/
private fun testScalebarCalculations(
point: Point,
style: ScalebarStyle,
maxWidth: Double,
units: ScalebarUnits,
scale: Double,
unitsPerDip: Double,
labelTypography: LabelTypography,
expectedDisplayLength: Double,
expectedLabels: List<String>,
spatialReference: SpatialReference = SpatialReference.webMercator(),
useGeodeticCalculations: Boolean = true,
) {
composeTestRule.setContent {

val viewpoint = Viewpoint(
center = Point(
x = point.x,
y = point.y,
spatialReference = spatialReference
),
scale = scale
)

val scalebarProperties = calculateScalebarProperties(
minScale = 0.0,
useGeodeticCalculations = useGeodeticCalculations,
scalebarUnits = units,
spatialReference = spatialReference,
viewpoint = viewpoint,
unitsPerDip = unitsPerDip,
maxDisplayLength = maxWidth,
)
val minSegmentWidth = measureMinSegmentWidth(
scalebarLineLength = scalebarProperties?.lineLength ?: 0.0,
labelTypography = labelTypography,
)
val scalebarLabels = generateScalebarLabels(
minSegmentWidth = minSegmentWidth,
displayLength = scalebarProperties?.displayLength ?: 0.0,
scalebarLineLength = scalebarProperties?.lineLength ?: 0.0,
displayUnit = scalebarProperties?.displayUnit,
style = style,
labelTypography = labelTypography,
)

assertThat(scalebarProperties?.displayLength).isWithin(.5).of(expectedDisplayLength)
assertThat(scalebarLabels.size).isEqualTo(expectedLabels.size)
for (i in expectedLabels.indices) {
assertThat(scalebarLabels[i].label).isEqualTo(expectedLabels[i])
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ class ScalebarTests {
* @since 200.7.0
*/
@Test
fun testLineScaleBarColorChange() {
fun testLineScalebarColorChange() {
composeTestRule.setContent {
Box(
modifier = Modifier.fillMaxSize(),
Expand Down

This file was deleted.

Loading