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

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

Merged
merged 9 commits into from
Jun 10, 2022
Merged
Changes from 4 commits
Commits
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
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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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>
Original file line number Diff line number Diff line change
@@ -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.mapCopyrightTextView.isVisible = false
return false
}

@@ -100,6 +101,7 @@ abstract class AbsMessageLocationItem<H : AbsMessageLocationItem.Holder> : AbsMe
holder.staticMapPinImageView.setImageDrawable(pinDrawable)
}
holder.staticMapErrorTextView.isVisible = false
holder.mapCopyrightTextView.isVisible = true
return false
}
})
@@ -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 mapCopyrightTextView by bind<TextView>(R.id.mapCopyrightTextView)
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we follow the same convention as for other ids by starting by staticMap prefix?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

}
}
Original file line number Diff line number Diff line change
@@ -18,7 +18,6 @@ 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
@@ -63,10 +62,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=0")
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 use attribution=false if it works: the intention may be clearer with a boolean.

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, works, done.

}
}
}
Original file line number Diff line number Diff line change
@@ -40,6 +40,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
@@ -58,6 +59,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()

@@ -94,6 +96,12 @@ class LocationLiveMapViewFragment @Inject constructor() : VectorBaseFragment<Fra
private fun setupMap() {
val mapFragment = getOrCreateSupportMapFragment()
mapFragment.getMapAsync { mapboxMap ->
mapboxMap.uiSettings.apply {
// Place copyright above the user list bottom sheet
setLogoMargins(dimensionConverter.dpToPx(8), 0, 0, dimensionConverter.dpToPx(208))
setAttributionMargins(dimensionConverter.dpToPx(96), 0, 0, dimensionConverter.dpToPx(208))
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 use the size of the view that these values ​​depend on instead of using static values ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, done. Although it is not possible to calculate the width of the "mapbox" logo.

}

lifecycleScope.launch {
mapboxMap.setStyle(urlMapProvider.getMapUrl()) { style ->
mapStyle = style
Original file line number Diff line number Diff line change
@@ -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
10 changes: 10 additions & 0 deletions vector/src/main/res/layout/item_timeline_event_location_stub.xml
Original file line number Diff line number Diff line change
@@ -62,4 +62,14 @@
android:orientation="horizontal"
app:layout_constraintGuide_percent="0.5" />

<TextView
android:id="@+id/mapCopyrightTextView"
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>
1 change: 1 addition & 0 deletions vector/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
@@ -3036,6 +3036,7 @@
<string name="labs_enable_live_location_summary">Temporary implementation: locations persist in room history</string>
<string name="live_location_bottom_sheet_stop_sharing">Stop sharing</string>
<string name="live_location_bottom_sheet_last_updated_at">Updated %1$s ago</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.

Do we really want this string to be translated? I think this is not necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exactly, moved.


<string name="message_bubbles">Show Message bubbles</string>