Fixes #3766, Added OPENSTREET attribution #3889

ashishkumar468 · 2020-07-29T11:47:55Z

Description (required)

Fixes #3766

**What changes did you make and why?

Added custom attribution to open street map

Tests performed (required)

Tested betaDebug with API 27

Screenshots (for UI changes only)

* Added OPENSTREET attribution in nearby

tests-checker

Could you please add tests to make sure this change works as expected?

app/src/main/res/layout/fragment_nearby_parent.xml

sivaraam · 2020-07-29T15:12:54Z

Just so I understand, we're thinking of just attributing "OpenStreetMap" attribution visible while also leaving the mapbox attribution as-is?

Wouldn't it be nice if we just give all the full attribution via linked text like "© Mapbox © OpenStreetMap"? This is similar to the attribution in https://foursquare.com/explore which looks nice. Of course, we would also have to add an option to opt-out of Mapbox telemetry in our app settings. I'm hoping full text only would look nice and consistent.

misaochan · 2020-07-29T18:26:14Z

The problem is that we can't modify the attribution through the Mapbox SDK, so we are using the workaround of simply adding a UI element of our own, hence why the "Mapbox" and "OpenStreetMap" are not all in the same TextView.

Wouldn't it be nice if we just give all the full attribution via linked text like "© Mapbox © OpenStreetMap"? This is similar to the attribution in https://foursquare.com/explore which looks nice.

I am not sure if we are allowed to convert the Mapbox logo to plaintext, but that could be an option if we are. Do you know if the Mapbox Android SDK docs state if we are allowed to? The Foursquare web app is not using the Android SDK, AFAIK.

Edit: Ah, I just saw https://docs.mapbox.com/help/how-mapbox-works/attribution/#mapbox-maps-sdk-for-android that you linked on the issue. I guess then it comes down to how complex the implementation of the telemetry opt out will be. If it adds substantial complexity, it may be worth just going with the standard Mapbox logo and trying to tweak the OSM TextView to make it look less out-of-place. Making the text © OpenStreetMap could be a start, and also we can tweak the color/font.

sivaraam · 2020-07-30T17:56:20Z

Edit: Ah, I just saw https://docs.mapbox.com/help/how-mapbox-works/attribution/#mapbox-maps-sdk-for-android that you linked on the issue. I guess then it comes down to how complex the implementation of the telemetry opt out will be.

Yep. ~~It seems straightforward when I look at the corresponding Telemetry class~~ (scratch that. It looks like a class in a very old version).

sivaraam · 2020-07-30T18:12:06Z

~~It seems straightforward when I look at the corresponding Telemetry class~~ (scratch that. It looks like a class in a very old version).

From this code in the Android SDK, it does appear fairly straightforward.

ashishkumar468 · 2020-07-31T17:52:38Z

Seems like the telemetry opt-out options is not that difficult to implement, let me combine these two attributions and get back

misaochan · 2020-08-03T09:53:28Z

Thanks a lot @ashishkumar468 , let me know how it goes. :)

ashishkumar468 · 2020-08-03T11:18:47Z

@misaochan, just to confirm, it will be like @mapbox @openstreetmap with clicks redirecting to corresponding links and an option in the settings to opt-out of telemetry?

misaochan · 2020-08-03T15:08:45Z

Right. From https://docs.mapbox.com/help/how-mapbox-works/attribution/#mapbox-maps-sdk-for-android :

Mapbox includes this built-in information button for your convenience. If you decide not to use it, you must include attribution on the map in a text format. The attribution must include © Mapbox as a link to https://www.mapbox.com/about/maps/, "© OpenStreetMap" as a link to http://www.openstreetmap.org/copyright, and "Improve this map" as a link to https://www.mapbox.com/map-feedback/

ashishkumar468 · 2020-08-03T15:37:20Z

The docs say this

The attribution must include © Mapbox as a link to https://www.mapbox.com/about/maps/, "© OpenStreetMap" as a link to http://www.openstreetmap.org/copyright, and "Improve this map" as a link to https://www.mapbox.com/map-feedback/.

Are we also including "Improve this map", this would be a long line then

sivaraam · 2020-08-03T17:15:43Z

The docs say this

The attribution must include © Mapbox as a link to mapbox.com/about/maps, "© OpenStreetMap" as a link to http://www.openstreetmap.org/copyright, and "Improve this map" as a link to mapbox.com/map-feedback.

Are we also including "Improve this map", this would be a long line then

Yeah, they do say that. I wonder why they didn't mention this in that support ticket response 🤔

Given that we can then remove the Mapbox attribution if we add the text attribution, I'm hoping it should look fine. I'm not sure, though.

ashishkumar468 · 2020-08-04T04:08:53Z

@sivaraam Just to confirm, you mean, we just show "@openstreetmap"?

sivaraam · 2020-08-04T11:01:48Z

@sivaraam Just to confirm, you mean, we just show "@openstreetmap"?

No. I was hoping that the full text attribution as suggested by the doc, which is:

The attribution must include © Mapbox as a link to https://www.mapbox.com/about/maps/, "© OpenStreetMap" as a link to http://www.openstreetmap.org/copyright, and "Improve this map" as a link to https://www.mapbox.com/map-feedback/

... might look good given that Mapbox's in-built attribution can be removed.

ashishkumar468 · 2020-08-04T12:27:30Z

So it will be like "@mapbox @openstreetmap Improve this map"

sivaraam · 2020-08-04T12:36:12Z

So it will be like "@mapbox @openstreetmap Improve this map"

Yeah. That's what the document asks us to do. So, we don't have a choice. We'll have to see how that looks to be sure which one of the following would look nice:

Mapbox built-in, Text only Openstreetmap attribution
Full text only attribution

neslihanturan · 2020-08-05T11:51:54Z

I think we better go with option 1, so that we can give built in attribution at least for Mapbox.

sivaraam · 2020-08-05T13:15:04Z

I think we better go with option 1, so that we can give built in attribution at least for Mapbox.

The problem is its inconsistent to have text attribution along-side Mapbox attribution. Also, given that Mapbox allows us to give full text-only attribution it would be better to do that in case it looks nice.

misaochan · 2020-08-07T09:43:43Z

Looks good to me, thanks a lot @ashishkumar468 . :) @sivaraam , do you know if it's legal for us to make the default telemetry setting "off" instead of on? I think having a default off would be better for our users than on, if it were possible.

sivaraam

I believe this still needs some changes.

Here's the screenshot of my stab at a linked text-only attribution (see review comments for details):

app/src/main/res/values/strings.xml

app/src/main/res/xml/preferences.xml

app/src/main/res/layout/fragment_nearby_parent.xml

app/src/main/java/fr/free/nrw/commons/settings/SettingsFragment.java

app/src/main/java/fr/free/nrw/commons/nearby/fragments/NearbyParentFragment.java

sivaraam · 2020-08-07T19:15:13Z

do you know if it's legal for us to make the default telemetry setting "off" instead of on? I think having a default off would be better for our users than on, if it were possible.

As much as I would love that, that doesn't seem feasible :(

In the docs related to this, they explicitly mention "opt-out" and not "opt-in". The iOS counterpart for text attribution is even more explicit about this:

Integrate the setting directly into your app. Hook a UISwitch control up to the MGLMapboxMetricsEnabled Boolean user default, which should be YES by default. Then set MGLMapboxMetricsEnabledSettingShownInApp to YES in your app’s Info.plist file.

Ref: Mapbox Maps SDK for iOS

misaochan · 2020-08-10T10:30:21Z

The screenshot looks good to me and the suggested changes make sense. Thanks @sivaraam . :)

ashishkumar468 · 2020-08-11T11:10:22Z

Thankyou for suggesting the changes, @sivaraam, I have made the suggested changes, Please have a look :)

misaochan

Looks good to me, happy to merge pending 1 final change. Thanks @ashishkumar468 !

app/src/main/res/values/strings.xml

sivaraam

This looks good! I wonder whether the text looks good in Dark mode 🤔 I'm facing weird issues with building the app so couldn't confirm this myself.

* #3624 DateTimeFormat wrong - match pattern returned from servers (#3625) * Revert "Fixes: #3179 Make category search non case-sensitive (#3326)" (#3636) Simply lower casing the name of the category sent to the server doesn't result in the server doing a case insensitive category search. In fact, it reduces the category search space as only categories that has a lower case character is searched even if the search text contains upper case characters. The test case did not catch this issue as the first character of the title is case insensitive[1]. So, revert the changes done in commit afdeaae. See further disucssion in the issue thread of #3179 starting from [2]. [1]: https://www.mediawiki.org/wiki/Manual:Page_title [2]: #3179 (comment) * Bugfix/security exception (#3627) * Fixes #3626 * Check is file is actually created before writing to file, file picker android * Handle Security exception * Fixes #3436 and #2881: Media Detail design Overhaul (#3505) * ic_map_dark_24dp: map icon for white background * ic_info_outline_dark_24dp: info icon for dark background * MediaDetailFragment: update the spacer as per image aspect ratio * fragment_media_detail: design overhaul * fragment_media_detail: remove redundant background color statements * make requested changes * add dark mode support * minor ui tweak * white map icon in dark mode * make rquested changes * make requested changes to layout * fix misalignment of category list * subtle amendments * convert comments to javadocs * minor amendments * minor changes * add styles for media detail * Media detail fragment refactored * make suggested changes * minor name fix * fix the delete button border * Fixes #3639 (Fix Save State implementation of CheckBoxTriState ) (#3686) * Add #3723 and #3721 to 2.13 release, fix conflicts Conflicts were caused by merging #3723 before #3721 , so I just rolled both into one commit. * Fix NullPointer when clicking on image in MediaDetailFragment (#3730)… (#3739) * Update changelog.md * Versioning for v2.13 * Fixes #3705 (Crash when viewing pic I just uploaded) (#3782) * Fixes #3705 * Let the MediaDetailPager fragment know when the contributions have been updated * Handle NPE, null check on adapter in MediaDetailPagerFragment * Fixed BookmarkLocationsDao DB migration (#3793) * Fixes #3725 (#3795) * Downgraded okhttp version to support Api 19 devices * Handled null CompoundDrawable[2] in etTitle-> UploadMediaDetailsFragment (#3828) * DownSample Upload image to be shown in UploadMediaDetailFragment to handle OOM, Bitmap Too large exception (#3830) * Fixes #3829 * DownSample Upload image to be shown in UploadMediaDetailFragment to handle OOM, Bitmap Too large exception * removed unused imports, handled possible exceptions * Let Fresco handle the downsampling of image * invalidate in onTransformEnd * Expose an interface TransformationListener in ZoomableDraweeView to listen to transformation change end * removed photoView dependency * removed unused imports in ZoomableActivity * Bugfix, expand/collapse * changed functio name * Bugfix/p18 uploads (#3869) * updated gradle plugin version * BugFix #3856 * Do not use preference for deciding acceptable lat long for nearby uploads, instead save the corresponding location in the Contribution via UploadItem * Marshall contribution's hasInvalidLocation * reset un-related changes * Fixed test cases * Minor code formatting and docs * Fixes #3882 (#3883) * Make hasInvalidLocation non-null integer with default value 0 Co-authored-by: Ashish Kumar <ashish@Ashishs-MacBook-Air.local> * Fixes #3766, Added OPENSTREET attribution (#3889) * Fixes #3766 * Added OPENSTREET attribution in nearby * Added custom text attribution in Nearby * Deleted unused class CustomBorderTextView * review suggested changes * modified telemetry summary string * Versioning and changelog for v2.13.1 (#3908) * Update changelog.md * Versioning for v2.13.1 * Fixes #3914 (#3915) * Verify user login before setting upload count * fixed compile-time error * fix erros * delete emptied files * remove empty file CategoriesModel.java Co-authored-by: Seán Mac Gillicuddy <seantheappdev@gmail.com> Co-authored-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com> Co-authored-by: Kshitij Bhardwaj <44129798+kbhardwaj123@users.noreply.github.com> Co-authored-by: Vitaly V. Pinchuk <vetal.978@gmail.com> Co-authored-by: Josephine Lim <josephinelim86@gmail.com> Co-authored-by: Ashish Kumar <ashish@Ashishs-MacBook-Air.local>

* commons-app#3624 DateTimeFormat wrong - match pattern returned from servers (commons-app#3625) * Revert "Fixes: commons-app#3179 Make category search non case-sensitive (commons-app#3326)" (commons-app#3636) Simply lower casing the name of the category sent to the server doesn't result in the server doing a case insensitive category search. In fact, it reduces the category search space as only categories that has a lower case character is searched even if the search text contains upper case characters. The test case did not catch this issue as the first character of the title is case insensitive[1]. So, revert the changes done in commit afdeaae. See further disucssion in the issue thread of commons-app#3179 starting from [2]. [1]: https://www.mediawiki.org/wiki/Manual:Page_title [2]: commons-app#3179 (comment) * Bugfix/security exception (commons-app#3627) * Fixes commons-app#3626 * Check is file is actually created before writing to file, file picker android * Handle Security exception * Fixes commons-app#3436 and commons-app#2881: Media Detail design Overhaul (commons-app#3505) * ic_map_dark_24dp: map icon for white background * ic_info_outline_dark_24dp: info icon for dark background * MediaDetailFragment: update the spacer as per image aspect ratio * fragment_media_detail: design overhaul * fragment_media_detail: remove redundant background color statements * make requested changes * add dark mode support * minor ui tweak * white map icon in dark mode * make rquested changes * make requested changes to layout * fix misalignment of category list * subtle amendments * convert comments to javadocs * minor amendments * minor changes * add styles for media detail * Media detail fragment refactored * make suggested changes * minor name fix * fix the delete button border * Fixes commons-app#3639 (Fix Save State implementation of CheckBoxTriState ) (commons-app#3686) * Add commons-app#3723 and commons-app#3721 to 2.13 release, fix conflicts Conflicts were caused by merging commons-app#3723 before commons-app#3721 , so I just rolled both into one commit. * Fix NullPointer when clicking on image in MediaDetailFragment (commons-app#3730)… (commons-app#3739) * Update changelog.md * Versioning for v2.13 * Fixes commons-app#3705 (Crash when viewing pic I just uploaded) (commons-app#3782) * Fixes commons-app#3705 * Let the MediaDetailPager fragment know when the contributions have been updated * Handle NPE, null check on adapter in MediaDetailPagerFragment * Fixed BookmarkLocationsDao DB migration (commons-app#3793) * Fixes commons-app#3725 (commons-app#3795) * Downgraded okhttp version to support Api 19 devices * Handled null CompoundDrawable[2] in etTitle-> UploadMediaDetailsFragment (commons-app#3828) * DownSample Upload image to be shown in UploadMediaDetailFragment to handle OOM, Bitmap Too large exception (commons-app#3830) * Fixes commons-app#3829 * DownSample Upload image to be shown in UploadMediaDetailFragment to handle OOM, Bitmap Too large exception * removed unused imports, handled possible exceptions * Let Fresco handle the downsampling of image * invalidate in onTransformEnd * Expose an interface TransformationListener in ZoomableDraweeView to listen to transformation change end * removed photoView dependency * removed unused imports in ZoomableActivity * Bugfix, expand/collapse * changed functio name * Bugfix/p18 uploads (commons-app#3869) * updated gradle plugin version * BugFix commons-app#3856 * Do not use preference for deciding acceptable lat long for nearby uploads, instead save the corresponding location in the Contribution via UploadItem * Marshall contribution's hasInvalidLocation * reset un-related changes * Fixed test cases * Minor code formatting and docs * Fixes commons-app#3882 (commons-app#3883) * Make hasInvalidLocation non-null integer with default value 0 Co-authored-by: Ashish Kumar <ashish@Ashishs-MacBook-Air.local> * Fixes commons-app#3766, Added OPENSTREET attribution (commons-app#3889) * Fixes commons-app#3766 * Added OPENSTREET attribution in nearby * Added custom text attribution in Nearby * Deleted unused class CustomBorderTextView * review suggested changes * modified telemetry summary string * Versioning and changelog for v2.13.1 (commons-app#3908) * Update changelog.md * Versioning for v2.13.1 * Fixes commons-app#3914 (commons-app#3915) * Verify user login before setting upload count * fixed compile-time error * fix erros * delete emptied files * remove empty file CategoriesModel.java Co-authored-by: Seán Mac Gillicuddy <seantheappdev@gmail.com> Co-authored-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com> Co-authored-by: Kshitij Bhardwaj <44129798+kbhardwaj123@users.noreply.github.com> Co-authored-by: Vitaly V. Pinchuk <vetal.978@gmail.com> Co-authored-by: Josephine Lim <josephinelim86@gmail.com> Co-authored-by: Ashish Kumar <ashish@Ashishs-MacBook-Air.local>

* Fixes commons-app#3766 * Added OPENSTREET attribution in nearby * Added custom text attribution in Nearby * Deleted unused class CustomBorderTextView * review suggested changes * modified telemetry summary string

Fixes commons-app#3766

1f0fcdb

* Added OPENSTREET attribution in nearby

tests-checker bot suggested changes Jul 29, 2020

View reviewed changes

misaochan reviewed Jul 29, 2020

View reviewed changes

app/src/main/res/layout/fragment_nearby_parent.xml Outdated Show resolved Hide resolved

ashishkumar468 added 2 commits August 6, 2020 18:14

Added custom text attribution in Nearby

cf1f44b

Deleted unused class CustomBorderTextView

0505fae

misaochan approved these changes Aug 7, 2020

View reviewed changes

sivaraam requested changes Aug 7, 2020

View reviewed changes

review suggested changes

81a3fdb

misaochan reviewed Aug 11, 2020

View reviewed changes

app/src/main/res/values/strings.xml Outdated Show resolved Hide resolved

modified telemetry summary string

0cf4083

sivaraam approved these changes Aug 11, 2020

View reviewed changes

misaochan merged commit fcc3053 into commons-app:2.13-release Aug 11, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes #3766, Added OPENSTREET attribution #3889

Fixes #3766, Added OPENSTREET attribution #3889

ashishkumar468 commented Jul 29, 2020 •

edited

Loading

tests-checker bot left a comment

sivaraam commented Jul 29, 2020 •

edited

Loading

misaochan commented Jul 29, 2020 •

edited

Loading

sivaraam commented Jul 30, 2020 •

edited

Loading

sivaraam commented Jul 30, 2020

ashishkumar468 commented Jul 31, 2020

misaochan commented Aug 3, 2020

ashishkumar468 commented Aug 3, 2020

misaochan commented Aug 3, 2020 •

edited

Loading

ashishkumar468 commented Aug 3, 2020

sivaraam commented Aug 3, 2020

ashishkumar468 commented Aug 4, 2020

sivaraam commented Aug 4, 2020

ashishkumar468 commented Aug 4, 2020

sivaraam commented Aug 4, 2020 •

edited

Loading

neslihanturan commented Aug 5, 2020

sivaraam commented Aug 5, 2020

misaochan commented Aug 7, 2020

sivaraam left a comment

sivaraam commented Aug 7, 2020

misaochan commented Aug 10, 2020

ashishkumar468 commented Aug 11, 2020

misaochan left a comment

sivaraam left a comment •

edited

Loading

Fixes #3766, Added OPENSTREET attribution #3889

Fixes #3766, Added OPENSTREET attribution #3889

Conversation

ashishkumar468 commented Jul 29, 2020 • edited Loading

tests-checker bot left a comment

Choose a reason for hiding this comment

sivaraam commented Jul 29, 2020 • edited Loading

misaochan commented Jul 29, 2020 • edited Loading

sivaraam commented Jul 30, 2020 • edited Loading

sivaraam commented Jul 30, 2020

ashishkumar468 commented Jul 31, 2020

misaochan commented Aug 3, 2020

ashishkumar468 commented Aug 3, 2020

misaochan commented Aug 3, 2020 • edited Loading

ashishkumar468 commented Aug 3, 2020

sivaraam commented Aug 3, 2020

ashishkumar468 commented Aug 4, 2020

sivaraam commented Aug 4, 2020

ashishkumar468 commented Aug 4, 2020

sivaraam commented Aug 4, 2020 • edited Loading

neslihanturan commented Aug 5, 2020

sivaraam commented Aug 5, 2020

misaochan commented Aug 7, 2020

sivaraam left a comment

Choose a reason for hiding this comment

sivaraam commented Aug 7, 2020

misaochan commented Aug 10, 2020

ashishkumar468 commented Aug 11, 2020

misaochan left a comment

Choose a reason for hiding this comment

sivaraam left a comment • edited Loading

Choose a reason for hiding this comment

ashishkumar468 commented Jul 29, 2020 •

edited

Loading

sivaraam commented Jul 29, 2020 •

edited

Loading

misaochan commented Jul 29, 2020 •

edited

Loading

sivaraam commented Jul 30, 2020 •

edited

Loading

misaochan commented Aug 3, 2020 •

edited

Loading

sivaraam commented Aug 4, 2020 •

edited

Loading

sivaraam left a comment •

edited

Loading