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.

Sign up for GitHub

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

Jump to bottom

Fix scale bar text being cut #1716

Merged
merged 17 commits into from
Oct 3, 2022
Merged

Fix scale bar text being cut #1716

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
e88df19
Added dummy scale bar example
jush Oct 3, 2022
131b56d
Use scale bar gravity to align to bottom right
jush Oct 3, 2022
ef46663
Find problematic scale bar configurations
jush Oct 3, 2022
536f91a
Fix scale bar cut-off right without stroke
jush Oct 3, 2022
91e3194
Handle text cut-off when using stroke
jush Oct 3, 2022
5d0fbb7
Layout fine tuning
jush Oct 3, 2022
f89d051
Only check right margin for right-most unit bar text
jush Oct 3, 2022
19c5c7d
Cleaned up Scale Bar activity
jush Oct 3, 2022
9a457fb
Updated CHANGELOG.md
jush Oct 3, 2022
a725d18
Lint formatting
jush Oct 3, 2022
ef51c61
Apply suggestions from code review
jush Oct 3, 2022
64cdac4
Use safeX when checking if it goes beyond left margin
jush Oct 3, 2022
a65bbc6
Removed values-w600dp for scale bar
jush Oct 3, 2022
512acd5
Added missing newline
jush Oct 3, 2022
f97571a
Added 10.9.0-rc.1 changelog header entry
jush Oct 3, 2022
b964151
Moved changelog entry under 10.9.0-rc.1
jush Oct 3, 2022
d0adea0
Lint format
jush Oct 3, 2022
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
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
Mapbox welcomes participation and contributions from everyone.
# main

## Bug fixes 🐞
jush marked this conversation as resolved.
Show resolved Hide resolved
* Fix scale bar text being cut ([1716](https://github.com/mapbox/mapbox-maps-android/pull/1716))

# 10.8.1 September 30, 2022

## Bug fixes 🐞
Expand Down
12 changes: 12 additions & 0 deletions app/src/main/AndroidManifest.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1027,6 +1027,18 @@
android:name="android.support.PARENT_ACTIVITY"
android:value=".ExampleOverviewActivity" />
</activity>
<activity
android:name=".examples.ScaleBarActivity"
android:description="@string/description_scale_bar"
android:exported="true"
android:label="@string/activity_scale_bar">
<meta-data
android:name="@string/category"
android:value="@string/category_config" />
<meta-data
android:name="android.support.PARENT_ACTIVITY"
android:value=".ExampleOverviewActivity" />
</activity>
<activity
android:name=".examples.LargeGeojsonPerformanceActivity"
android:description="@string/description_large_geojson"
Expand Down
16 changes: 16 additions & 0 deletions app/src/main/java/com/mapbox/maps/testapp/examples/ScaleBarActivity.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package com.mapbox.maps.testapp.examples

import android.os.Bundle
import androidx.appcompat.app.AppCompatActivity
import com.mapbox.maps.testapp.R

/**
* Activity to showcase scale bar custom configuration using xml attributes.
*/
class ScaleBarActivity : AppCompatActivity() {
Copy link
Contributor

Choose a reason for hiding this comment

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

should we create similar activity on iOS for parity and making sure that our scalebar customizations are aligned? @mapbox/maps-ios


override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
setContentView(R.layout.activity_scale_bar)
}
}
24 changes: 24 additions & 0 deletions app/src/main/res/layout/activity_scale_bar.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<?xml version="1.0" encoding="utf-8"?>
<com.mapbox.maps.MapView xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:app="http://schemas.android.com/apk/res-auto"
android:id="@+id/mapView"
android:layout_width="match_parent"
android:layout_height="match_parent"
app:mapbox_cameraTargetLat="61.489557808394814"
app:mapbox_cameraTargetLng="23.735379149896005"
app:mapbox_cameraZoom="14.940602577615023"

app:mapbox_scaleBarBorderWidth="1dp"
app:mapbox_scaleBarHeight="5dp"
app:mapbox_scaleBarGravity="@integer/scaleBarGravity"
jush marked this conversation as resolved.
Show resolved Hide resolved
app:mapbox_scaleBarIsMetricUnits="true"
app:mapbox_scaleBarMarginBottom="@dimen/scaleBarMarginBottom"
app:mapbox_scaleBarMarginRight="@dimen/scaleBarMarginRight"
app:mapbox_scaleBarPrimaryColor="@android:color/black"
app:mapbox_scaleBarRatio="@fraction/scaleBarRatio"
app:mapbox_scaleBarSecondaryColor="@android:color/white"
app:mapbox_scaleBarShowTextBorder="true"
app:mapbox_scaleBarTextBarMargin="10dp"
app:mapbox_scaleBarTextBorderWidth="5dp"
app:mapbox_scaleBarTextColor="@android:color/black"
app:mapbox_scaleBarTextSize="10dp" />
8 changes: 8 additions & 0 deletions app/src/main/res/values-w600dp/scale_bar_config.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<?xml version="1.0" encoding="utf-8"?>
<resources>
<item name="scaleBarRatio" format="float" type="fraction">0.2</item>
<dimen name="scaleBarMarginRight">60dp</dimen>
<dimen name="scaleBarMarginBottom">8dp</dimen>
<!-- "bottom | right" See plugin-scalebar/src/main/res/values/attrs.xml -->
<integer name="scaleBarGravity">0x55</integer>
jush marked this conversation as resolved.
Show resolved Hide resolved
</resources>
1 change: 1 addition & 0 deletions app/src/main/res/values/example_descriptions.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@
<string name="description_multiple_display">Display the map on a secondary display</string>
<string name="description_map_view_customization">Customize your map view</string>
<string name="description_ornaments_margin">Update margins of ornaments when the map rotates</string>
<string name="description_scale_bar">Show scale bar at custom position</string>
<string name="description_large_geojson">Display long route as large geojson</string>
<string name="description_nine_patch">Add 9-patch image to the style</string>
<string name="description_feature_state">Create interactive hover effects with feature state</string>
Expand Down
1 change: 1 addition & 0 deletions app/src/main/res/values/example_titles.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@
<string name="activity_multiple_display">Multi display</string>
<string name="activity_map_view_customization">Creating a map view</string>
<string name="activity_ornaments">Ornaments</string>
<string name="activity_scale_bar">Scale Bar</string>
<string name="activity_large_geojson">Geojson performance</string>
<string name="activity_nine_patch">9-patch image</string>
<string name="activity_feature_state">Feature state</string>
Expand Down
9 changes: 9 additions & 0 deletions app/src/main/res/values/scale_bar_config.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<?xml version="1.0" encoding="utf-8"?>
<resources>
<item name="scaleBarRatio" format="float" type="fraction">0.8</item>
<dimen name="scaleBarMarginRight">0dp</dimen>
<dimen name="scaleBarMarginBottom">50dp</dimen>

<!-- "bottom | center_horizontal" See plugin-scalebar/src/main/res/values/attrs.xml-->
<integer name="scaleBarGravity">0x51</integer>
</resources>
39 changes: 34 additions & 5 deletions plugin-scalebar/src/main/java/com/mapbox/maps/plugin/scalebar/ScaleBarImpl.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ class ScaleBarImpl : ScaleBar, View {
strokePaint.textSize = value.textSize
scaleTable = if (value.isMetricUnits) metricTable else imperialTable
unit = if (value.isMetricUnits) METER_UNIT else FEET_UNIT
strokePaint.strokeWidth = value.textBorderWidth
strokePaint.strokeWidth = if (value.showTextBorder) value.textBorderWidth else 0F
kiryldz marked this conversation as resolved.
Show resolved Hide resolved
enable = value.enabled
if (useContinuousRendering) {
reusableCanvas = null
Expand Down Expand Up @@ -270,13 +270,20 @@ class ScaleBarImpl : ScaleBar, View {
textPaint.textAlign = Paint.Align.LEFT
strokePaint.textAlign = Paint.Align.LEFT
}

jush marked this conversation as resolved.
Show resolved Hide resolved
else -> {
textPaint.textAlign = Paint.Align.CENTER
strokePaint.textAlign = Paint.Align.CENTER
}
}

drawText(canvas, distanceText, (unitBarWidth * rectIndex), textSize)
drawText(
canvas = canvas,
text = distanceText,
x = (unitBarWidth * rectIndex),
y = textSize,
lastUnitBarText = rectIndex == rectCount
jush marked this conversation as resolved.
Show resolved Hide resolved
)

if (rectIndex != rectCount) {
canvas.drawRect(
Expand All @@ -294,11 +301,33 @@ class ScaleBarImpl : ScaleBar, View {
}
}

private fun drawText(canvas: Canvas, text: String, x: Float, y: Float) {
private fun drawText(canvas: Canvas, text: String, x: Float, y: Float, lastUnitBarText: Boolean) {
var safeX = x

// As an optimization only check if it goes beyond right view for the last unit bar text
if (lastUnitBarText) {
// Check if it goes beyond the right margin of the view
val textWidthPx = textPaint.measureText(text)
val textMaxRightPx = x + strokePaint.strokeWidth + when (textPaint.textAlign) {
jush marked this conversation as resolved.
Show resolved Hide resolved
Paint.Align.LEFT -> textWidthPx
Paint.Align.CENTER -> textWidthPx / 2
Paint.Align.RIGHT, null -> 0F
}
if (textMaxRightPx > width) {
// Move it away from right margin enough to fit
safeX -= textMaxRightPx - width
jush marked this conversation as resolved.
Show resolved Hide resolved
}
}

// Check if it goes beyond the left margin of the view
if (x - strokePaint.strokeWidth / 2 < 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

is it ok that we check x here but operate with safeX below?

Copy link
Member Author

Choose a reason for hiding this comment

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

You're right. It's better to use safeX.

Copy link
Contributor

Choose a reason for hiding this comment

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

and also for better readability I'd use if (x < trokePaint.strokeWidth / 2) similar to L316

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmmm I personally think that:

if (x < trokePaint.strokeWidth / 2)

it's a bit obscure.
I need to think more to figure out why we're comparing a position (x or safeX) with a width (strokeWidth). Somehow, to me, mine reads easier: to position safeX I remove a width and then check if it goes beyond the left minimum position (0).

But happy to change it to your proposal if others also prefer it.

safeX += strokePaint.strokeWidth / 2
}

if (settings.showTextBorder) {
canvas.drawText(text, x, y, strokePaint)
canvas.drawText(text, safeX, y, strokePaint)
}
canvas.drawText(text, x, y, textPaint)
canvas.drawText(text, safeX, y, textPaint)
}

/**
Expand Down