Skip to content

Fix copyright attributions of map views [PSF-1058] - [PSF-1072] #6247

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

Merged
merged 9 commits into from
Jun 10, 2022
Merged

Fix copyright attributions of map views [PSF-1058] - [PSF-1072] #6247

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
b455bf0
Create new styles for map view copyright text.
onurays Jun 7, 2022
b284b92
Use custom TextView for map view copyright.
onurays Jun 7, 2022
ec1eb49
Fix position of the copyright on maximized map.
onurays Jun 7, 2022
4b50058
Changelog added.
onurays Jun 7, 2022
68c6e52
Use dynamic height to calculate the position of the copyright attribu…
onurays Jun 7, 2022
adde210
Code review fixes.
onurays Jun 8, 2022
d0ab6af
Remove unused import.
onurays Jun 8, 2022
9b22880
Set translatable as false for the copyright text.
onurays Jun 8, 2022
f9f1b3e
Code review fix.
onurays Jun 9, 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
1 change: 1 addition & 0 deletions changelog.d/6247.bugfix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix copyright attributions of map views
3 changes: 2 additions & 1 deletion library/ui-styles/src/main/res/values/dimens_font.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
<dimen name="text_size_body">14sp</dimen>
<dimen name="text_size_caption">12sp</dimen>
<dimen name="text_size_micro">10sp</dimen>
<dimen name="text_size_nano">8sp</dimen>

<dimen name="text_size_button">14sp</dimen>

</resources>
</resources>
5 changes: 5 additions & 0 deletions library/ui-styles/src/main/res/values/styles_location.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,9 @@
<item name="android:gravity">center</item>
</style>

<style name="Widget.Vector.TextView.Nano.Copyright">
<!-- Static map view is always light in both light and dark theme. So we need to use a static dark color -->
<item name="android:textColor">@color/element_content_primary_light</item>
</style>

</resources>
7 changes: 6 additions & 1 deletion library/ui-styles/src/main/res/values/styles_text_view.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,4 +48,9 @@
<item name="lineHeight">16sp</item>
</style>

</resources>
<style name="Widget.Vector.TextView.Nano">
<item name="android:textAppearance">@style/TextAppearance.Vector.Nano</item>
<item name="lineHeight">16sp</item>
</style>

</resources>
7 changes: 7 additions & 0 deletions library/ui-styles/src/main/res/values/text_appearances.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,4 +85,11 @@
<item name="android:letterSpacing">0.02</item>
</style>

<style name="TextAppearance.Vector.Nano" parent="TextAppearance.MaterialComponents.Caption">
<item name="fontFamily">sans-serif</item>
<item name="android:fontFamily">sans-serif</item>
<item name="android:textSize">@dimen/text_size_nano</item>
<item name="android:letterSpacing">0</item>
</style>

</resources>
3 changes: 3 additions & 0 deletions ...main/java/im/vector/app/features/home/room/detail/timeline/item/AbsMessageLocationItem.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ abstract class AbsMessageLocationItem<H : AbsMessageLocationItem.Holder> : AbsMe
): Boolean {
holder.staticMapPinImageView.setImageResource(R.drawable.ic_location_pin_failed)
holder.staticMapErrorTextView.isVisible = true
holder.staticMapCopyrightTextView.isVisible = false
return false
}

Expand All @@ -100,6 +101,7 @@ abstract class AbsMessageLocationItem<H : AbsMessageLocationItem.Holder> : AbsMe
holder.staticMapPinImageView.setImageDrawable(pinDrawable)
}
holder.staticMapErrorTextView.isVisible = false
holder.staticMapCopyrightTextView.isVisible = true
return false
}
})
Expand All @@ -111,5 +113,6 @@ abstract class AbsMessageLocationItem<H : AbsMessageLocationItem.Holder> : AbsMe
val staticMapImageView by bind<ImageView>(R.id.staticMapImageView)
val staticMapPinImageView by bind<ImageView>(R.id.staticMapPinImageView)
val staticMapErrorTextView by bind<TextView>(R.id.staticMapErrorTextView)
val staticMapCopyrightTextView by bind<TextView>(R.id.staticMapCopyrightTextView)
}
}
8 changes: 2 additions & 6 deletions vector/src/main/java/im/vector/app/features/location/UrlMapProvider.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,13 @@ package im.vector.app.features.location

import im.vector.app.BuildConfig
import im.vector.app.core.resources.LocaleProvider
import im.vector.app.core.resources.isRTL
import im.vector.app.features.raw.wellknown.getElementWellknown
import org.matrix.android.sdk.api.extensions.tryOrNull
import org.matrix.android.sdk.api.raw.RawService
import org.matrix.android.sdk.api.session.Session
import javax.inject.Inject

class UrlMapProvider @Inject constructor(
private val localeProvider: LocaleProvider,
private val session: Session,
private val rawService: RawService
) {
Expand Down Expand Up @@ -63,10 +61,8 @@ class UrlMapProvider @Inject constructor(
append(height)
append(".png")
append(keyParam)
if (!localeProvider.isRTL()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The localeProvider dependency can be removed from this class since it is no longer used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, done.

// On LTR languages we want the legal mentions to be displayed on the bottom left of the image
append("&attribution=bottomleft")
}
// Since the default copyright font is too small we put a custom one on map
append("&attribution=false")
}
}
}
10 changes: 10 additions & 0 deletions vector/src/main/java/im/vector/app/features/location/live/map/LocationLiveMapViewFragment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import androidx.core.graphics.drawable.toBitmap
import androidx.lifecycle.lifecycleScope
import com.airbnb.mvrx.fragmentViewModel
import com.airbnb.mvrx.withState
import com.google.android.material.bottomsheet.BottomSheetBehavior
import com.mapbox.mapboxsdk.geometry.LatLng
import com.mapbox.mapboxsdk.geometry.LatLngBounds
import com.mapbox.mapboxsdk.maps.MapView
Expand All @@ -40,6 +41,7 @@ import im.vector.app.R
import im.vector.app.core.extensions.addChildFragment
import im.vector.app.core.extensions.configureWith
import im.vector.app.core.platform.VectorBaseFragment
import im.vector.app.core.utils.DimensionConverter
import im.vector.app.databinding.FragmentLocationLiveMapViewBinding
import im.vector.app.features.location.UrlMapProvider
import im.vector.app.features.location.zoomToBounds
Expand All @@ -58,6 +60,7 @@ class LocationLiveMapViewFragment @Inject constructor() : VectorBaseFragment<Fra

@Inject lateinit var urlMapProvider: UrlMapProvider
@Inject lateinit var bottomSheetController: LiveLocationBottomSheetController
@Inject lateinit var dimensionConverter: DimensionConverter

private val viewModel: LocationLiveMapViewModel by fragmentViewModel()

Expand Down Expand Up @@ -94,6 +97,13 @@ class LocationLiveMapViewFragment @Inject constructor() : VectorBaseFragment<Fra
private fun setupMap() {
val mapFragment = getOrCreateSupportMapFragment()
mapFragment.getMapAsync { mapboxMap ->
val bottomSheetHeight = BottomSheetBehavior.from(views.bottomSheet).peekHeight
mapboxMap.uiSettings.apply {
// Place copyright above the user list bottom sheet
setLogoMargins(dimensionConverter.dpToPx(8), 0, 0, bottomSheetHeight + dimensionConverter.dpToPx(8))
setAttributionMargins(dimensionConverter.dpToPx(96), 0, 0, bottomSheetHeight + dimensionConverter.dpToPx(8))
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you also add this setAttributionMargins in MapTilerMapView? In this view, we only call setLogoMargins.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is working well with default margins. Since I moved the position of attribution here I also needed re-set all the margins.

Copy link
Contributor

Choose a reason for hiding this comment

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

What I meant is what you have done is great (we see the logo + information icon). But I have seen in MapTilerMapView, we only call the setLogoMargins implying we only see the logo but not the information icon. I know it is out of the scope of this PR but we could align this behaviour and add the call to setAttributionMargins in MapTilerMapView so that we can see the information icon.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We actually see both logo and the info icon in MapTilerMapView. You can see it in maximized static location sharing.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the info icon is missing in the screen where user selects location sharing option. It is hidden under the options view. This is due to missing call to setAttributionMargins.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I missed this screen!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice catch, done!

}

lifecycleScope.launch {
mapboxMap.setStyle(urlMapProvider.getMapUrl()) { style ->
mapStyle = style
Expand Down
2 changes: 1 addition & 1 deletion vector/src/main/res/layout/fragment_location_live_map_view.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
android:layout_width="match_parent"
android:layout_height="match_parent"
android:background="@drawable/bg_live_location_users_bottom_sheet"
app:behavior_peekHeight="200dp"
app:behavior_hideable="false"
app:behavior_peekHeight="200dp"
app:layout_behavior="com.google.android.material.bottomsheet.BottomSheetBehavior">

<View
Expand Down
10 changes: 10 additions & 0 deletions vector/src/main/res/layout/item_timeline_event_location_stub.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,4 +62,14 @@
android:orientation="horizontal"
app:layout_constraintGuide_percent="0.5" />

<TextView
android:id="@+id/staticMapCopyrightTextView"
style="@style/Widget.Vector.TextView.Nano.Copyright"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_margin="4dp"
android:text="@string/location_map_view_copyright"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toTopOf="parent" />

</androidx.constraintlayout.widget.ConstraintLayout>
2 changes: 2 additions & 0 deletions vector/src/main/res/values/donottranslate.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,6 @@
<string name="ftue_auth_email_verification_subtitle">To confirm your email address, tap the button in the email we just sent to %s</string>
<string name="ftue_auth_email_verification_footer">Did not receive an email?</string>
<string name="ftue_auth_email_resend_email">Resend email</string>

<string name="location_map_view_copyright">© MapTiler © OpenStreetMap contributors</string>
Copy link
Member

Choose a reason for hiding this comment

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

Can you add translatable="false" please?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't know that, done.

</resources>