organicmaps/organicmaps-tmp
Archived
13
Fork 1
Code Issues 2.7k Pull requests 182 Projects Releases 65 Wiki Activity Actions

[Android][iOS] Social pages editor fix #3248

Merged
root merged 5 commits from issue-2787-android-social-pages-editor into master 2023-02-20 14:53:48 +00:00
Conversation 11 Commits 5 Files changed 27 +412 -115
strump commented 2022-08-24 11:02:55 +00:00
Member
Copy link

Closes #2787, closes #3089

The problem is some URLs after normalization are not valid because domain name is removed. I fixed the issue by adding domain name to native code called from UI. Also domain name is added to OSM changeset now.

  • Facebook:

    • OSM data or User input: https://facebook.com/pages/Daikichi-Japans-Restaurant/118163444928138
    • After normalization: pages/Daikichi-Japans-Restaurant/118163444928138
    • On PlacePage UI:
      • Old: https://facebook.com/pages/Daikichi-Japans-Restaurant/118163444928138
      • New: pages/Daikichi-Japans-Restaurant/118163444928138
    • On upload to OSM:
      • Old: pages/Daikichi-Japans-Restaurant/118163444928138
      • New: https://facebook.com/pages/Daikichi-Japans-Restaurant/118163444928138

  • Instagram:

    • OSM data or User input: https://www.instagram.com/p/BvkgKZNDbqN/?ghid=UwPchX7B
    • After normalization: p/BvkgKZNDbqN
    • On PlacePage UI:
      • Old: https://instagram.com/p/BvkgKZNDbqN
      • New: p/BvkgKZNDbqN
    • On upload to OSM:
      • Old: p/BvkgKZNDbqN
      • New: https://instagram.com/p/BvkgKZNDbqN

  • Facebook 2:

    • OSM data or User input: www.facebook.com/OpenStreetMap
    • After normalization: OpenStreetMap
    • On PlacePage UI:
      • Old: @OpenStreetMap
      • New: OpenStreetMap
    • On upload to OSM: OpenStreetMap

Also fixed iPhone UI. This is my first commit to Swift and Objective-C code. Please verify!!!

Before After
iphone-url-old iphone-url-new

Doesn't require map data change (in contrast to PR #3212).

Update 2023-01-23

  • Simplified social contacts URLs on PlacePage. Only part of URL is shown.
  • Don't generate mobile URLs for social networks. Only default URLs.
  • [iOS] Added Line social network to iPhone PlacePage and Editor.
  • Formatting and comments updates.

social-contacts-editor-fix

Update 2023-01-31

  • Social contacts on PlacePage doesn't have @ symbol now
  • Introduced constexpr variables in C++ code
  • Tests improvements

social-contacts-editor-fix-2

Closes #2787, closes #3089 The problem is some URLs after normalization are not valid because domain name is removed. I fixed the issue by adding domain name to native code called from UI. Also domain name is added to OSM changeset now. * **Facebook**: * **OSM data or User input**: `https://facebook.com/pages/Daikichi-Japans-Restaurant/118163444928138` * **After normalization**: `pages/Daikichi-Japans-Restaurant/118163444928138` * **On PlacePage UI**: * Old: `https://facebook.com/pages/Daikichi-Japans-Restaurant/118163444928138` * New: `pages/Daikichi-Japans-Restaurant/118163444928138` * **On upload to OSM**: * Old: `pages/Daikichi-Japans-Restaurant/118163444928138` * New: `https://facebook.com/pages/Daikichi-Japans-Restaurant/118163444928138` * **Instagram**: * **OSM data or User input**: `https://www.instagram.com/p/BvkgKZNDbqN/?ghid=UwPchX7B` * **After normalization**: `p/BvkgKZNDbqN` * **On PlacePage UI**: * Old: `https://instagram.com/p/BvkgKZNDbqN` * New: `p/BvkgKZNDbqN` * **On upload to OSM**: * Old: `p/BvkgKZNDbqN` * New: `https://instagram.com/p/BvkgKZNDbqN` * **Facebook 2**: * **OSM data or User input**: `www.facebook.com/OpenStreetMap` * **After normalization**: `OpenStreetMap` * **On PlacePage UI**: * Old: `@OpenStreetMap` * New: `OpenStreetMap` * **On upload to OSM**: `OpenStreetMap` Also fixed iPhone UI. **This is my first commit to Swift and Objective-C code.** Please verify!!! | Before | After | | --------------- | --------------- | | ![iphone-url-old](https://user-images.githubusercontent.com/720808/195790416-ec83d95e-d181-4cba-813b-06aa5e80eacd.jpg) | ![iphone-url-new](https://user-images.githubusercontent.com/720808/195790466-92af2283-f566-4e2b-9528-7168936f9d56.jpg) | Doesn't require map data change (in contrast to PR #3212). ## Update 2023-01-23 * Simplified social contacts URLs on PlacePage. Only part of URL is shown. * Don't generate mobile URLs for social networks. Only default URLs. * [iOS] Added Line social network to iPhone PlacePage and Editor. * Formatting and comments updates. ![social-contacts-editor-fix](https://user-images.githubusercontent.com/720808/214022489-940e83be-fd57-4148-b5fe-0b88d4af4fc3.png) ## Update 2023-01-31 * Social contacts on PlacePage doesn't have `@` symbol now * Introduced `constexpr` variables in C++ code * Tests improvements ![social-contacts-editor-fix-2](https://user-images.githubusercontent.com/720808/215830034-be55116c-3f60-4c0a-86a0-dca6f9b3a50f.png)
strump reviewed 2022-09-23 15:27:42 +00:00
indexer/validate_and_format_contacts.cpp
strump commented 2022-09-23 15:27:41 +00:00
Author
Member

This function converts string_view value to string even if no changes are made. Don't know how to improve it.

This function converts `string_view value` to `string` even if no changes are made. Don't know how to improve it.
biodranik commented 2022-09-24 16:31:58 +00:00 (Migrated from github.com)
Copy link

Actually, I have a better idea. To avoid duplicating the code in both iOS and Android, let's do normalization in the core before features are uploaded to OSM. WDYT?

Also, are there some other checks which we can do in the core once instead of on both platforms?

Actually, I have a better idea. To avoid duplicating the code in both iOS and Android, let's do normalization in the core before features are uploaded to OSM. WDYT? Also, are there some other checks which we can do in the core once instead of on both platforms?
strump commented 2022-09-26 08:33:58 +00:00
Author
Member
Copy link

@biodranik let me answer you comment here.
Here're analytics of contact:facebook data for the whole Planet:

  • count - 141 882 - total object in OSM database;
  • source_length - 5 949 920 - Total length of contact:facebook values;
  • normalized_length - 2 498 146 - Total length of normalized values;
  • url_count - 8 302 - number of normalized values with '/' symbol (e.g. pg/LiveGalleryHairDesign); it's 5.8% of all objects;
  • denormalized_length - 2 672 488 - Total length of normalized values with https://facebook.com domain (if it's needed); nicknames are not affected.

So if we add https://facebook.com/ explicitly to all URLs (not nicknames) it will increase map size by 2 672 488 - 2 498 146 = 174 342 bytes.

For all social contacts:

tag count source_length normalized_length url_count denormalized_length
contact:facebook 141 882 5 949 920 2 498 146 8302 (5.8%) 2 672 488
contact:instagram 69 168 2 590 348 895 059 255 (0.3%) 900 669
contact:twitter 34 020 996 019 353 032 142 (0.4%) 355 872
contact:vk 53 760 1 361 593 568 377 15 (0.03%) 568 602

N.B.: I used Google BigQuery to get numbers. Here are SQLs osm_bigquery_facebook_stats.sql, osm_bigquery_instagram_stats.sql, osm_bigquery_twitter_stats.sql, osm_bigquery_vk_stats.sql.

@biodranik let me answer you [comment](https://git.omaps.dev/organicmaps/organicmaps/pulls/3212#issuecomment-1257014842) here. Here're analytics of `contact:facebook` data for the whole Planet: * count - `141 882` - total object in OSM database; * source_length - `5 949 920` - Total length of `contact:facebook` values; * normalized_length - `2 498 146` - Total length of normalized values; * url_count - `8 302` - number of normalized values with '/' symbol (e.g. `pg/LiveGalleryHairDesign`); it's 5.8% of all objects; * denormalized_length - `2 672 488` - Total length of normalized values with `https://facebook.com` domain (if it's needed); nicknames are not affected. So if we add `https://facebook.com/` explicitly to all URLs (not nicknames) it will increase map size by `2 672 488 - 2 498 146 = 174 342` bytes. For all social contacts: | tag | count | source_length | normalized_length | url_count | denormalized_length | | --------------- | --------------: | --------------: | --------------: | --------------: | --------------: | | `contact:facebook` | `141 882` | `5 949 920` | `2 498 146` | `8302` (5.8%) | `2 672 488` | | `contact:instagram` | `69 168` | `2 590 348` | `895 059` | `255` (0.3%) | `900 669` | | `contact:twitter` | `34 020` | `996 019` | `353 032` | `142` (0.4%) | `355 872` | | `contact:vk` | `53 760` | `1 361 593` | `568 377` | `15` (0.03%) | `568 602` | N.B.: I used [Google BigQuery](https://wiki.openstreetmap.org/wiki/BigQuery_dataset) to get numbers. Here are SQLs [osm_bigquery_facebook_stats.sql](https://gist.github.com/strump/315a4e08fdd4ed4a1dce8550f1ce26a8), [osm_bigquery_instagram_stats.sql](https://gist.github.com/strump/3834def764aa8a784d54e8804ce7c29e), [osm_bigquery_twitter_stats.sql](https://gist.github.com/strump/48a4227213b88f33ab19bcd37595706d), [osm_bigquery_vk_stats.sql](https://gist.github.com/strump/9fdb4f59297a57f5e24a6847b1d2f735).
biodranik commented 2022-09-30 22:38:36 +00:00 (Migrated from github.com)
Copy link

Thanks for the stats. There is no need to increase entropy by copypasting thousands of lines of code into the OSM DB and our maps. When tens of millions of users start using OM, the number of these strings will grow quickly. Also, don't forget about the general rule: the more compact map data, the faster the rendering.

Thanks for the stats. There is no need to increase entropy by copypasting thousands of lines of code into the OSM DB and our maps. When tens of millions of users start using OM, the number of these strings will grow quickly. Also, don't forget about the general rule: the more compact map data, the faster the rendering.
biodranik commented 2022-10-07 20:41:36 +00:00 (Migrated from github.com)
Copy link

Can we solve that issue even easier? For example:

  1. Make a list of prefixes to strip by checking values on taginfo or even better by getting them from the dump and find common patterns, e.g.
  2. Strip all these prefixes at the map generation stage (and save space!)
  3. Where do we need the full prefix? Only when the user clicks on the link or copies it. For simplicity, it can be hard-coded in each platform, or a full link can be retrieved via some C++ getter.
  4. If a user adds social link, we can show https://facebook.com/ label before the input, so the user should type only the normalized part of the URL. But if someone inserts a full URL - no problem! Our code will send it to OSM as is, either a short form of the link, or a full one.
  5. If a user edits an existing address, here the previous value in OSM (possibly with a full prefix) may be replaced by a shorter version without the prefix. But it is allowed by OSM!

What do you think about this approach?

Can we solve that issue even easier? For example: 1. Make a list of prefixes to strip by checking values on taginfo or even better by getting them from the dump and find common patterns, e.g. - https://www.facebook.com/ - https:https://www.facebook.com/ - https://facebook.com/ - https://es-es.facebook.com/ - @page - page - https://www.facebook.com/pages/category/Restaurant/Au-poincaré-182381015649241 Here we see that we can strip anything, including @ symbol, and then restore the full link by adding https://www.facebook,com/ without breaking it (the link will remain valid). 2. Strip all these prefixes at the map generation stage (and save space!) 3. Where do we need the full prefix? Only when the user clicks on the link or copies it. For simplicity, it can be hard-coded in each platform, or a full link can be retrieved via some C++ getter. 4. If a user adds social link, we can show `https://facebook.com/` label before the input, so the user should type only the normalized part of the URL. But if someone inserts a full URL - no problem! Our code will send it to OSM as is, either a short form of the link, or a full one. 5. If a user edits an existing address, here the previous value in OSM (possibly with a full prefix) may be replaced by a shorter version without the prefix. But it is allowed by OSM! What do you think about this approach?
strump commented 2022-10-11 13:02:18 +00:00
Author
Member
Copy link

@biodranik your description looks rational. It mostly works in such way on master branch.
95% of cases social contact field has only username or pagename. OM works fine with such values

  1. @username is shown on PlacePage ✅
  2. Full URL is visible only when user copies contact ✅
  3. In Editor user can change contact username ✅
  4. Username is uploaded to OSM without domain name (which is correct) ✅

But there are 5% of cases when social contact field has no username but full URL. We strip domain name leaving short part
https://www.facebook.com/pages/category/Restaurant/Au-poincaré-182381015649241 -> pages/category/Restaurant/Au-poincaré-182381015649241

  1. Full URL is shown on PlacePage (UI code prepends domain) ✅
  2. Full URL is visible when user copies contact (UI code prepends domain) ✅
  3. In Editor user can change full URL 🚫 (domain name is stripper and user needs to add it manually #2787 #3089)
  4. Full URL is uploaded to OSM 🚫 (domain name is stripped)

With this PR I'm fixing cases 3. and 4.

And I'm thinking that short URL to full URL conversion could be moved to Native code to simplify UI code.

@biodranik your description looks rational. It mostly works in such way on master branch. 95% of cases social contact field has only username or pagename. OM works fine with such values 1. `@username` is shown on PlacePage ✅ 2. Full URL is visible only when user copies contact ✅ 3. In Editor user can change contact username ✅ 4. Username is uploaded to OSM without domain name (which is correct) ✅ But there are 5% of cases when social contact field has no username but full URL. We strip domain name leaving short part `https://www.facebook.com/pages/category/Restaurant/Au-poincaré-182381015649241` -> `pages/category/Restaurant/Au-poincaré-182381015649241` 1. Full URL is shown on PlacePage (UI code prepends domain) ✅ 2. Full URL is visible when user copies contact (UI code prepends domain) ✅ 3. In Editor user can change full URL 🚫 (domain name is stripper and user needs to add it manually [#2787](https://git.omaps.dev/organicmaps/organicmaps/issues/2787) [#3089](https://git.omaps.dev/organicmaps/organicmaps/issues/3089)) 4. Full URL is uploaded to OSM 🚫 (domain name is stripped) With this PR I'm fixing cases 3. and 4. And I'm thinking that short URL to full URL conversion could be moved to Native code to simplify UI code.
biodranik commented 2022-10-11 14:05:21 +00:00 (Migrated from github.com)
Copy link
  1. What stops us from always showing a full url in the editor by always appending it?
  2. What stops us by always stripping full url when uploading it to the OpenStreetMap.org? Or, alternatively, always upload full urls if they were edited/changed by the user? (No need to rewrite the same unmodified url, of course, by comparing a stripped part only, for example).
1. What stops us from always showing a full url in the editor by always appending it? 2. What stops us by always stripping full url when uploading it to the OpenStreetMap.org? Or, alternatively, always upload full urls if they were edited/changed by the user? (No need to rewrite the same unmodified url, of course, by comparing a stripped part only, for example).
strump commented 2022-10-13 08:29:46 +00:00
Author
Member
Copy link

@biodranik

  1. We can show full URL on editor UI. For user it could look odd.
  2. We can upload full URL to OSM API.

But from code perspective it's not hard to keep short pagename and full URL in parallel. See my code for a simple check.

In this PR social contacts work in such way:

  1. Map data stores nickname/pagename or short URL (no domain name). No updates here.
  2. When PlacePage UI calls native method to get POI details it gets either nickname/pagename or full URL (with domain name)
  3. When Editor UI calls native method to get social contacts it gets either nickname/pagename or full URL (with domain name)
  4. OSM Changeset contains either nickname/pagename or full URL (with domain name).

I need to look iOS code if it needs modifications. Will install xCode and learn Swift.

@biodranik 1. We can show full URL on editor UI. For user it could look odd. 2. We can upload full URL to OSM API. But from code perspective it's not hard to keep short pagename and full URL in parallel. See my [code](https://git.omaps.dev/organicmaps/organicmaps/pulls/3248/files#diff-1875d95de404ef18d91166583b07278ec0468b31f6da4603e3e69e41b394e7ceR429) for a simple check. In this PR social contacts work in such way: 1. Map data stores nickname/pagename or short URL (no domain name). No updates here. 2. When PlacePage UI calls native method to get POI details it gets either nickname/pagename or full URL (with domain name) 3. When Editor UI calls native method to get social contacts it gets either nickname/pagename or full URL (with domain name) 4. OSM Changeset contains either nickname/pagename or full URL (with domain name). I need to look iOS code if it needs modifications. Will install xCode and learn Swift.
biodranik (Migrated from github.com) requested changes 2022-10-19 09:54:23 +00:00
biodranik (Migrated from github.com) left a comment

Thanks for your efforts! I'm worrying that our code complexity grows with all these copy-pasted approaches, and it increases the time to support it. Can it be optimized?

Thanks for your efforts! I'm worrying that our code complexity grows with all these copy-pasted approaches, and it increases the time to support it. Can it be optimized?
android/src/com/mapswithme/maps/editor/EditorFragment.java
biodranik (Migrated from github.com) commented 2022-09-24 16:28:54 +00:00

What about Line?

What about Line?
android/src/com/mapswithme/maps/widget/placepage/PlacePageView.java
biodranik (Migrated from github.com) commented 2022-10-19 09:53:19 +00:00

Can we optimize and implement these similar checks only once, in the C++?

Can we optimize and implement these similar checks only once, in the C++?
editor/editor_tests/xml_feature_test.cpp
biodranik (Migrated from github.com) commented 2022-10-19 09:52:32 +00:00

Why do you need it?

Why do you need it?
indexer/validate_and_format_contacts.cpp
biodranik (Migrated from github.com) commented 2022-10-19 09:47:38 +00:00 
bool isSocialContactTag(string_view tag)
```suggestion bool isSocialContactTag(string_view tag) ```
biodranik (Migrated from github.com) commented 2022-10-19 09:47:55 +00:00 
  return tag == "contact:instagram" || tag == "contact:facebook" || tag == "contact:twitter"
          || tag == "contact:vk" || tag == "contact:line";
```suggestion return tag == "contact:instagram" || tag == "contact:facebook" || tag == "contact:twitter" || tag == "contact:vk" || tag == "contact:line"; ```
biodranik (Migrated from github.com) commented 2022-10-19 09:48:10 +00:00 
  return metaID == MapObject::MetadataID::FMD_CONTACT_INSTAGRAM ||
          metaID == MapObject::MetadataID::FMD_CONTACT_FACEBOOK ||
          metaID == MapObject::MetadataID::FMD_CONTACT_TWITTER ||
          metaID == MapObject::MetadataID::FMD_CONTACT_VK ||
          metaID == MapObject::MetadataID::FMD_CONTACT_LINE;
```suggestion return metaID == MapObject::MetadataID::FMD_CONTACT_INSTAGRAM || metaID == MapObject::MetadataID::FMD_CONTACT_FACEBOOK || metaID == MapObject::MetadataID::FMD_CONTACT_TWITTER || metaID == MapObject::MetadataID::FMD_CONTACT_VK || metaID == MapObject::MetadataID::FMD_CONTACT_LINE; ```
biodranik (Migrated from github.com) commented 2022-10-19 09:48:52 +00:00 
    return {value};  // 'value' is not an URL but username. Don't add the domain name.

Or even better return value; if it compiles.

```suggestion return {value}; // 'value' is not an URL but username. Don't add the domain name. ``` Or even better `return value;` if it compiles.
biodranik (Migrated from github.com) commented 2022-10-19 09:49:44 +00:00

ditto

ditto
biodranik (Migrated from github.com) commented 2022-10-19 09:50:04 +00:00

switch is more appropriate here.

switch is more appropriate here.
indexer/validate_and_format_contacts.hpp
biodranik (Migrated from github.com) commented 2022-10-19 09:51:02 +00:00 
}  // namespace osm
```suggestion } // namespace osm ```
biodranik (Migrated from github.com) commented 2022-10-19 09:51:13 +00:00 
bool isSocialContactTag(std::string_view tag);
```suggestion bool isSocialContactTag(std::string_view tag); ```
iphone/CoreApi/CoreApi/PlacePageData/Common/PlacePageInfoData.mm
biodranik (Migrated from github.com) commented 2022-10-19 09:46:44 +00:00

It would be great to avoid hardcoding these URL parts several times everywhere in the code.

It would be great to avoid hardcoding these URL parts several times everywhere in the code.
iphone/Maps/UI/PlacePage/Components/PlacePageInfoViewController.swift
biodranik (Migrated from github.com) commented 2022-10-19 09:45:55 +00:00

It looks like a lot of duplicating code in the UI part. Can it be improved by querying the properly formatted string from the C++ core? E.g. GetShortSocialLink(metadataId)?

It looks like a lot of duplicating code in the UI part. Can it be improved by querying the properly formatted string from the C++ core? E.g. `GetShortSocialLink(metadataId)`?
iphone/Maps/UI/PlacePage/PlacePageManager/MWMPlacePageManager.mm
@ -240,2 +243,3 @@
- (void)openFacebook:(PlacePageData *)data {
[self.ownerViewController openUrl:[NSString stringWithFormat:@"https://m.facebook.com/%@", data.infoData.facebook]];
std::string const fullUrl = osm::socialContactToURL(osm::MapObject::MetadataID::FMD_CONTACT_FACEBOOK, [data.infoData.facebook UTF8String]);
[self.ownerViewController openUrl:ToNSString(fullUrl)];
biodranik (Migrated from github.com) commented 2022-10-19 09:41:17 +00:00

Can this code be optimized, so we implement the cross-platform logic only once, instead of duplicating it on each platform every time when we add some new social network support? Something like that:

Android or iOS parts have a generic method OpenLink, that internally calls some C++ Feature::FullSocialUrl(metadataId) returning an URL in a string (and throwing an exception/crashing if an invalid metadataId was queried), and then opening the browser for it.

Can this code be optimized, so we implement the cross-platform logic only once, instead of duplicating it on each platform every time when we add some new social network support? Something like that: Android or iOS parts have a generic method OpenLink, that internally calls some C++ `Feature::FullSocialUrl(metadataId)` returning an URL in a string (and throwing an exception/crashing if an invalid metadataId was queried), and then opening the browser for it.
strump reviewed 2022-11-28 14:06:24 +00:00
indexer/validate_and_format_contacts.cpp
strump commented 2022-11-28 14:06:23 +00:00
Author
Member

Removed const.

Removed `const`.
strump reviewed 2022-11-28 14:09:45 +00:00
indexer/validate_and_format_contacts.cpp
strump commented 2022-11-28 14:09:45 +00:00
Author
Member

Done!

Done!
strump reviewed 2022-11-28 14:09:57 +00:00
indexer/validate_and_format_contacts.cpp
strump commented 2022-11-28 14:09:56 +00:00
Author
Member

Removed parentheses

Removed parentheses
strump reviewed 2022-11-28 14:10:16 +00:00
indexer/validate_and_format_contacts.cpp
strump commented 2022-11-28 14:10:16 +00:00
Author
Member

Replaced with switch

Replaced with `switch`
strump reviewed 2022-11-28 14:31:12 +00:00
indexer/validate_and_format_contacts.hpp
strump commented 2022-11-28 14:31:11 +00:00
Author
Member

Done

Done
strump reviewed 2022-11-28 14:31:52 +00:00
indexer/validate_and_format_contacts.cpp
strump commented 2022-11-28 14:31:52 +00:00
Author
Member

return {value}; doesn't work. Fails with
chosen constructor is explicit in copy-initialization

`return {value};` doesn't work. Fails with `chosen constructor is explicit in copy-initialization`
strump reviewed 2022-11-28 14:34:05 +00:00
editor/editor_tests/xml_feature_test.cpp
strump commented 2022-11-28 14:34:05 +00:00
Author
Member

This test verifies that OSM XML contains full URL https://facebook.com/pages/Stereo-Plaza/118100041593935 instead of pages/Stereo-Plaza/118100041593935. Current implementation uploads latter value.

This test verifies that OSM XML contains full URL `https://facebook.com/pages/Stereo-Plaza/118100041593935` instead of `pages/Stereo-Plaza/118100041593935`. Current implementation uploads latter value.
strump commented 2022-11-29 08:06:51 +00:00
Author
Member
Copy link

@biodranik sorry for being offline with this PR. So this is another version of my fix:

  1. Implemented socialContactToURL(...) function to convert any social contant to full URL. See validate_and_format_contacts.cpp for details. All string constants with domain names are hardcoded there.
  2. On UI every time we need to show full URI we use socialContactToURL(...):
    2.1. On Place page if social contact is not pagename/username but a link
    2.2. On Place page when user taps social contant and we build URL to open in browser/app
    2.3. On Place page when user long tap on social contact
    2.4. On Editor UI if social contact is not pagename/username but a link
  3. When we build changeset to upload to OSM and social contact is a link
  4. On iOS Place page we don't add @ symbol if social contact is an URL

N.B. Every call to socialContactToURL(...) isn't cheap because of string convertions and native call:

jstring -> std::string -> socialContactToURL(...) -> std::string -> jstring

or

NSString -> std::string -> socialContactToURL(...) -> std::string -> NSString

But UI calls this method only once per social network and only on user interaction: open Place Page view, tap, long tap, open Editor.

@biodranik sorry for being offline with this PR. So this is another version of my fix: 1. Implemented `socialContactToURL(...)` function to convert any social contant to full URL. See `validate_and_format_contacts.cpp` for details. All string constants with domain names are hardcoded there. 2. On UI every time we need to show full URI we use `socialContactToURL(...)`: 2.1. On Place page if social contact is not pagename/username but a link 2.2. On Place page when user taps social contant and we build URL to open in browser/app 2.3. On Place page when user long tap on social contact 2.4. On Editor UI if social contact is not pagename/username but a link 3. When we build changeset to upload to OSM and social contact is a link 4. On iOS Place page we don't add `@` symbol if social contact is an URL **N.B.** Every call to `socialContactToURL(...)` isn't cheap because of string convertions and native call: `jstring -> std::string -> socialContactToURL(...) -> std::string -> jstring` or `NSString -> std::string -> socialContactToURL(...) -> std::string -> NSString` But UI calls this method only once per social network and only on user interaction: open Place Page view, tap, long tap, open Editor.
biodranik commented 2022-11-29 14:55:27 +00:00 (Migrated from github.com)
Copy link

Is there a way to get a properly formatted URL for UI without the overhead? E.g. to query it by id, and receive the final, properly formatted one?

Is there a way to get a properly formatted URL for UI without the overhead? E.g. to query it by id, and receive the final, properly formatted one?
strump commented 2022-11-30 09:51:29 +00:00
Author
Member
Copy link

Is there a way to get a properly formatted URL for UI without the overhead? E.g. to query it by id, and receive the final, properly formatted one?

Is there any way I can read selected mapObject from native code? For editor there's g_editableMapObject variable in native code but not for PlacePage.

> Is there a way to get a properly formatted URL for UI without the overhead? E.g. to query it by id, and receive the final, properly formatted one? Is there any way I can read selected `mapObject` from native code? For editor there's `g_editableMapObject` variable in native code but not for PlacePage.
strump commented 2023-01-19 07:48:53 +00:00
Author
Member
Copy link

I rebased this PR to the latest 'master'. And fixed case of contact:facebook=facebook.com/profile.php?id=34496388.

I rebased this PR to the latest 'master'. And fixed case of `contact:facebook=facebook.com/profile.php?id=34496388`.
👍 1
biodranik (Migrated from github.com) reviewed 2023-01-19 14:43:00 +00:00
biodranik (Migrated from github.com) left a comment

Thank you very much for your work! I know that it already took a lot of time and energy for everyone, so there is even bigger motivation to finish it in the best way possible :)

Let's focus on how we can simplify everything.

Thank you very much for your work! I know that it already took a lot of time and energy for everyone, so there is even bigger motivation to finish it in the best way possible :) Let's focus on how we can simplify everything.
android/jni/app/organicmaps/editor/Editor.cpp
biodranik (Migrated from github.com) commented 2023-01-19 13:37:40 +00:00 
  if (osm::isSocialContactTag(metaID))
  {
    auto const value = g_editableMapObject.GetMetadata(metaID);
```suggestion if (osm::isSocialContactTag(metaID)) { auto const value = g_editableMapObject.GetMetadata(metaID); ```
biodranik (Migrated from github.com) commented 2023-01-19 13:41:57 +00:00

Our shortened values can contain slashes without URLs, right? Like page/something.

Do we really need to convert social values into full URLs for iOS and Android? Can this conversion be avoided completely? For example:

  1. UI always shows values without hosts (we can even remove the @ symbol logic; it is unnecessary and will simplify the code).
  2. If the user pastes a value with a hostname in the editor, then our C++ code should properly validate it, and strip it before sending it to OSM/storing internally.
  3. If the user clicks on the social link, it should be opened in a browser. Only in this moment, we can call some C++ conversion method to receive a fully valid URL, and immediately open it in Android or iOS.

What do you think?

Our shortened values can contain slashes without URLs, right? Like `page/something`. Do we really need to convert social values into full URLs for iOS and Android? Can this conversion be avoided completely? For example: 1. UI always shows values without hosts (we can even remove the @ symbol logic; it is unnecessary and will simplify the code). 2. If the user pastes a value with a hostname in the editor, then our C++ code should properly validate it, and strip it before sending it to OSM/storing internally. 3. If the user clicks on the social link, it should be opened in a browser. Only in this moment, we can call some C++ conversion method to receive a fully valid URL, and immediately open it in Android or iOS. What do you think?
biodranik (Migrated from github.com) commented 2023-01-19 13:42:16 +00:00 
  return jni::ToJavaString(env, g_editableMapObject.GetMetadata(metaID));
```suggestion return jni::ToJavaString(env, g_editableMapObject.GetMetadata(metaID)); ```
android/jni/app/organicmaps/editor/Editor.cpp
biodranik (Migrated from github.com) commented 2023-01-19 13:44:52 +00:00 
  auto const metaID = static_cast<osm::MapObject::MetadataID>(id);
  if (osm::isSocialContactTag(metaID))
    return jni::ToJavaString(env, osm::socialContactToURL(metaID, jni::ToNativeString(env, value)));
  return value;
```suggestion auto const metaID = static_cast<osm::MapObject::MetadataID>(id); if (osm::isSocialContactTag(metaID)) return jni::ToJavaString(env, osm::socialContactToURL(metaID, jni::ToNativeString(env, value))); return value; ```
biodranik (Migrated from github.com) commented 2023-01-19 13:48:00 +00:00

Is a mobile URL really required? Aren't desktop URLs automatically redirected to their mobile versions when opened? Removing this method simplifies the implementation.

Is a mobile URL really required? Aren't desktop URLs automatically redirected to their mobile versions when opened? Removing this method simplifies the implementation.
biodranik (Migrated from github.com) commented 2023-01-19 13:48:37 +00:00 
  auto const metaID = static_cast<osm::MapObject::MetadataID>(id);
  if (osm::isSocialContactTag(metaID))
    return jni::ToJavaString(env, osm::socialContactToMobileURL(metaID, jni::ToNativeString(env, value)));
  return value;
```suggestion auto const metaID = static_cast<osm::MapObject::MetadataID>(id); if (osm::isSocialContactTag(metaID)) return jni::ToJavaString(env, osm::socialContactToMobileURL(metaID, jni::ToNativeString(env, value))); return value; ```
android/src/app/organicmaps/editor/Editor.java
biodranik (Migrated from github.com) commented 2023-01-19 13:49:18 +00:00 
  // Convert normalized contact value to full URL.
  public static native String nativeGetFullUrl(int id, String value);
```suggestion // Convert normalized contact value to full URL. public static native String nativeGetFullUrl(int id, String value); ```
android/src/app/organicmaps/widget/placepage/PlacePageView.java
biodranik (Migrated from github.com) commented 2023-01-19 13:51:31 +00:00

This method is not needed if we don't show the full URL in the UI, right? Nobody needs to see a long full URL...

This method is not needed if we don't show the full URL in the UI, right? Nobody needs to see a long full URL...
android/src/app/organicmaps/widget/placepage/PlacePageView.java
biodranik (Migrated from github.com) commented 2023-01-19 13:51:47 +00:00

ditto

ditto
android/src/app/organicmaps/widget/placepage/PlacePageView.java
biodranik (Migrated from github.com) commented 2023-01-19 13:54:56 +00:00

All these lines can be replaced by

       Utils.openUrl(getContext(), mMapObject.getFullUrl(Metadata.MetadataType.FMD_CONTACT_FACEBOOK));

Will storing the full URL in the map object simplify the code? Or storing it in the Editor is better? If we need full URL only when a link is clicked, then it should be enough, no?

All these lines can be replaced by ```suggestion Utils.openUrl(getContext(), mMapObject.getFullUrl(Metadata.MetadataType.FMD_CONTACT_FACEBOOK)); ``` Will storing the full URL in the map object simplify the code? Or storing it in the Editor is better? If we need full URL only when a link is clicked, then it should be enough, no?
android/src/app/organicmaps/widget/placepage/PlacePageView.java
biodranik (Migrated from github.com) commented 2023-01-19 13:56:51 +00:00

Showing full URL in UI doesn't look great.

Showing full URL in UI doesn't look great.
biodranik (Migrated from github.com) commented 2023-01-19 13:57:44 +00:00

The UI code on all platforms will be greatly simplified if UI just shows whatever is in the data, without any conditions and formatting.

The UI code on all platforms will be greatly simplified if UI just shows whatever is in the data, without any conditions and formatting.
editor/xml_feature.cpp
biodranik (Migrated from github.com) commented 2023-01-19 14:00:31 +00:00

Would it be easier to avoid any logic here, and store it as-is? But apply the stripping logic (in C++ core) when a user presses "Save" in the editor window?

Would it be easier to avoid any logic here, and store it as-is? But apply the stripping logic (in C++ core) when a user presses "Save" in the editor window?
indexer/indexer_tests/validate_and_format_contacts_test.cpp
@ -10,3 +10,4 @@
TEST_EQUAL(osm::ValidateAndFormat_facebook("facebook.com/OpenStreetMap"), "OpenStreetMap", ());
TEST_EQUAL(osm::ValidateAndFormat_facebook("www.facebook.com/OpenStreetMap"), "OpenStreetMap", ());
TEST_EQUAL(osm::ValidateAndFormat_facebook("www.facebook.fr/OpenStreetMap"), "OpenStreetMap", ());
TEST_EQUAL(osm::ValidateAndFormat_facebook("http://facebook.com/OpenStreetMap"), "OpenStreetMap", ());
biodranik (Migrated from github.com) commented 2023-01-19 14:04:07 +00:00

This link is not valid anymore.

This link is not valid anymore.
indexer/validate_and_format_contacts.cpp
@ -61,3 +89,3 @@
{
if (!v.empty() && !IsProtocolSpecified(v))
return kWebsiteProtocols[kWebsiteProtocolDefaultIndex] + v;
return string{kHttp}.append(v);
biodranik (Migrated from github.com) commented 2023-01-19 14:08:26 +00:00 
      return *valuePtr;
```suggestion return *valuePtr; ```
biodranik (Migrated from github.com) commented 2023-01-19 14:08:38 +00:00 
      auto const valuePtr = url.GetParamValue("id");
```suggestion auto const valuePtr = url.GetParamValue("id"); ```
indexer/validate_and_format_contacts.cpp
biodranik (Migrated from github.com) commented 2023-01-19 14:14:28 +00:00

A faster version of this code is possible, but if we don't use it at all, it would be even better (see my other comment).

A faster version of this code is possible, but if we don't use it at all, it would be even better (see my other comment).
biodranik (Migrated from github.com) commented 2023-01-19 14:17:25 +00:00 
  if (value.empty())

Is it a normal case? Maybe ASSERT(!value.empty(), ()); could be more helpful?

```suggestion if (value.empty()) ``` Is it a normal case? Maybe `ASSERT(!value.empty(), ());` could be more helpful?
biodranik (Migrated from github.com) commented 2023-01-19 14:23:57 +00:00

string_view objects are faster to compare than raw C strings. It makes sense to use local constexpr constants like this:

constexpr std::string_view kInstagram{"contact:instagram"};

if (tag == kInstagram) ...

string_view objects are faster to compare than raw C strings. It makes sense to use local constexpr constants like this: constexpr std::string_view kInstagram{"contact:instagram"}; if (tag == kInstagram) ...
biodranik (Migrated from github.com) commented 2023-01-19 14:25:04 +00:00

{} is a safer constructor version than () here and above.

  return string{value};
{} is a safer constructor version than () here and above. ```suggestion return string{value}; ```
biodranik (Migrated from github.com) commented 2023-01-19 14:26:26 +00:00 
  if (value.empty())
```suggestion if (value.empty()) ```
biodranik (Migrated from github.com) commented 2023-01-19 14:27:17 +00:00

As hostnames are copy-pasted here, they can be stored as global static constexpr constants and reused in different methods.

As hostnames are copy-pasted here, they can be stored as global static constexpr constants and reused in different methods.
biodranik (Migrated from github.com) commented 2023-01-19 14:29:02 +00:00

Isn't Line normalized? Why do we store URLs for it?

Actually, it can be safe to assume that all URLs are normalized in the new release (after adding force-normalization into the generator) because now we don't allow users to edit the map if a new maps data update is available.

Isn't Line normalized? Why do we store URLs for it? Actually, it can be safe to assume that all URLs are normalized in the new release (after adding force-normalization into the generator) because now we don't allow users to edit the map if a new maps data update is available.
biodranik (Migrated from github.com) commented 2023-01-19 14:29:29 +00:00

Do we really need a mobile url version?

Do we really need a mobile url version?
indexer/validate_and_format_contacts.cpp
biodranik (Migrated from github.com) commented 2023-01-19 14:15:37 +00:00

nit: would the switch be better/faster?

nit: would the `switch` be better/faster?
indexer/validate_and_format_contacts.hpp
biodranik (Migrated from github.com) commented 2023-01-19 14:30:32 +00:00 
#include "map_object.hpp"

#include <string>
```suggestion #include "map_object.hpp" #include <string> ```
iphone/CoreApi/CoreApi/PlacePageData/Common/PlacePageInfoData.mm
biodranik (Migrated from github.com) commented 2023-01-19 14:31:32 +00:00

Can we avoid using full urls here?

Can we avoid using full urls here?
iphone/Maps/UI/Editor/MWMEditorViewController.mm
@ -423,3 +428,4 @@
text:value
placeholder:name
keyboardType:UIKeyboardTypeDefault
capitalization:UITextAutocapitalizationTypeSentences];
biodranik (Migrated from github.com) commented 2023-01-19 14:32:46 +00:00

Is this conversion really necessary?

Is this conversion really necessary?
iphone/Maps/UI/PlacePage/Components/PlacePageInfoViewController.swift
biodranik (Migrated from github.com) commented 2023-01-19 14:33:31 +00:00

Will removing @ simplify the code?

Will removing @ simplify the code?
iphone/Maps/UI/PlacePage/PlacePageManager/MWMPlacePageManager.mm
@ -250,2 +254,4 @@
std::string const fullUrl = osm::socialContactToURL(osm::MapObject::MetadataID::FMD_CONTACT_TWITTER, [data.infoData.twitter UTF8String]);
[self.ownerViewController openUrl:ToNSString(fullUrl)];
}
biodranik (Migrated from github.com) commented 2023-01-19 14:41:51 +00:00

As I mentioned before, this code can be rewritten something like this:

[self.ownerViewController openUrl:socialContactToURL(data.infoData.facebook)]];

(Need to think about where the actual method will be and how it will be called).

Currently selected Place Page can always be accessed from C++ code like this:

static place_page::Info & rawData() { return GetFramework().GetCurrentPlacePageInfo(); }

It may make sense to implement full URL conversion into the place page Info object, if it is not needed anywhere else.

As I mentioned before, this code can be rewritten something like this: ``` [self.ownerViewController openUrl:socialContactToURL(data.infoData.facebook)]]; ``` (Need to think about where the actual method will be and how it will be called). Currently selected Place Page can always be accessed from C++ code like this: ``` static place_page::Info & rawData() { return GetFramework().GetCurrentPlacePageInfo(); } ``` It may make sense to implement full URL conversion into the place page Info object, if it is not needed anywhere else.
strump reviewed 2023-01-20 14:37:32 +00:00
android/jni/app/organicmaps/editor/Editor.cpp
strump commented 2023-01-20 14:37:32 +00:00
Author
Member

If I understand you correctly, you propose to get rid of social network domain on PlacePage UI. This is how it will look (Android). I added options when URL has no "https://" and with "instagram" instead of "instagram.com". I also added one more option with leading '/' symbol.

Which of those options do you prefer?

If I understand you correctly, you propose to get rid of social network domain on PlacePage UI. This is how it will look (Android). I added options when URL has no "https://" and with "instagram" instead of "instagram.com". I also added one more option with leading `'/'` symbol. <img src="https://user-images.githubusercontent.com/720808/213764866-ae5b50a5-8125-413c-b267-c0a7ed580434.png" width="300px"/> Which of those options do you prefer?
biodranik (Migrated from github.com) reviewed 2023-01-20 21:48:31 +00:00
android/jni/app/organicmaps/editor/Editor.cpp
biodranik (Migrated from github.com) commented 2023-01-20 21:48:31 +00:00

Without a slash is ok. It is not critical at all, because a better solution will be to show icons without the text. It will be obvious, and won't take much space, thus easier to use.

Without a slash is ok. It is not critical at all, because a better solution will be to show icons without the text. It will be obvious, and won't take much space, thus easier to use.
biodranik (Migrated from github.com) reviewed 2023-01-20 21:49:06 +00:00
android/jni/app/organicmaps/editor/Editor.cpp
biodranik (Migrated from github.com) commented 2023-01-20 21:49:06 +00:00

We can finish with this PR, create an issue about redesigning icons in Place Page, and do it later separately.

We can finish with this PR, create an issue about redesigning icons in Place Page, and do it later separately.
strump reviewed 2023-01-23 10:19:50 +00:00
android/jni/app/organicmaps/editor/Editor.cpp
strump commented 2023-01-23 10:19:49 +00:00
Author
Member

Fixed formatting.

Fixed formatting.
strump reviewed 2023-01-23 10:58:05 +00:00
android/jni/app/organicmaps/editor/Editor.cpp
strump commented 2023-01-23 10:58:04 +00:00
Author
Member

Ok, I did as we agreed. Place page shows only part of URL. See screenshots in PR description.

Ok, I did as we agreed. Place page shows only part of URL. See screenshots in PR description.
strump reviewed 2023-01-23 10:58:19 +00:00
android/jni/app/organicmaps/editor/Editor.cpp
strump commented 2023-01-23 10:58:19 +00:00
Author
Member

Removed else.

Removed `else`.
strump reviewed 2023-01-23 10:58:35 +00:00
android/jni/app/organicmaps/editor/Editor.cpp
strump commented 2023-01-23 10:58:35 +00:00
Author
Member

Fixed formatting.

Fixed formatting.
strump reviewed 2023-01-23 10:59:11 +00:00
android/jni/app/organicmaps/editor/Editor.cpp
strump commented 2023-01-23 10:59:11 +00:00
Author
Member

Removed mobile URL generation all over the code. Only full URL is used.

Removed mobile URL generation all over the code. Only full URL is used.
strump reviewed 2023-01-23 10:59:24 +00:00
android/jni/app/organicmaps/editor/Editor.cpp
strump commented 2023-01-23 10:59:23 +00:00
Author
Member

Fixed formatting.

Fixed formatting.
strump reviewed 2023-01-23 11:00:42 +00:00
android/src/app/organicmaps/widget/placepage/PlacePageView.java
strump commented 2023-01-23 11:00:42 +00:00
Author
Member

Simplified PlacePageView.refreshSocialLinks(...) method. Do not show full URL on PlacePage.

Simplified `PlacePageView.refreshSocialLinks(...)` method. Do not show full URL on PlacePage.
strump reviewed 2023-01-23 11:00:55 +00:00
android/src/app/organicmaps/widget/placepage/PlacePageView.java
strump commented 2023-01-23 11:00:55 +00:00
Author
Member

Done!

Done!
strump reviewed 2023-01-23 11:03:05 +00:00
android/src/app/organicmaps/widget/placepage/PlacePageView.java
strump commented 2023-01-23 11:03:05 +00:00
Author
Member

Added method MapObject.getContactUrl(Metadata.MetadataType) to simplify the code.
Storing short link is better. We show it on UI and can restore full URL any time (on click or in Editor).

Added method `MapObject.getContactUrl(Metadata.MetadataType)` to simplify the code. Storing short link is better. We show it on UI and can restore full URL any time (on click or in Editor).
strump reviewed 2023-01-23 11:04:24 +00:00
android/src/app/organicmaps/widget/placepage/PlacePageView.java
strump commented 2023-01-23 11:04:24 +00:00
Author
Member

Simplified with MapObject.getContactUrl(Metadata.MetadataType) call

Simplified with `MapObject.getContactUrl(Metadata.MetadataType)` call
strump reviewed 2023-01-23 11:05:29 +00:00
android/src/app/organicmaps/widget/placepage/PlacePageView.java
strump commented 2023-01-23 11:05:29 +00:00
Author
Member

Yes. PlacePage now shows social contacts with no modification. Only @ symbol is added if needed.

Yes. PlacePage now shows social contacts with no modification. Only `@` symbol is added if needed.
strump reviewed 2023-01-23 11:10:47 +00:00
editor/xml_feature.cpp
strump commented 2023-01-23 11:10:47 +00:00
Author
Member

We do strip full URL in Editor. We can't send to OSM partial URLs like contact:instagram=p/BvkgKZNDbqN. Wiki says that either username of full URL is expected. This part of code converts all shortened URLs to full URLs before sending to OSM API.

We do strip full URL in Editor. We can't send to OSM partial URLs like `contact:instagram=p/BvkgKZNDbqN`. [Wiki](https://wiki.openstreetmap.org/wiki/Key:contact:*) says that either username of full URL is expected. This part of code converts all shortened URLs to full URLs before sending to OSM API.
strump reviewed 2023-01-23 11:11:05 +00:00
indexer/indexer_tests/validate_and_format_contacts_test.cpp
@ -10,3 +10,4 @@
TEST_EQUAL(osm::ValidateAndFormat_facebook("facebook.com/OpenStreetMap"), "OpenStreetMap", ());
TEST_EQUAL(osm::ValidateAndFormat_facebook("www.facebook.com/OpenStreetMap"), "OpenStreetMap", ());
TEST_EQUAL(osm::ValidateAndFormat_facebook("www.facebook.fr/OpenStreetMap"), "OpenStreetMap", ());
TEST_EQUAL(osm::ValidateAndFormat_facebook("http://facebook.com/OpenStreetMap"), "OpenStreetMap", ());
strump commented 2023-01-23 11:11:05 +00:00
Author
Member

Fixed ID in the link.

Fixed ID in the link.
strump reviewed 2023-01-23 11:11:58 +00:00
indexer/validate_and_format_contacts.cpp
@ -61,3 +89,3 @@
{
if (!v.empty() && !IsProtocolSpecified(v))
return kWebsiteProtocols[kWebsiteProtocolDefaultIndex] + v;
return string{kHttp}.append(v);
strump commented 2023-01-23 11:11:58 +00:00
Author
Member

Fixed.

Fixed.
strump reviewed 2023-01-23 11:13:39 +00:00
indexer/validate_and_format_contacts.cpp
strump commented 2023-01-23 11:13:39 +00:00
Author
Member

This code is used from editor/xml_feature.cpp to convert short URLs to full URLs for OSM API. I simplified it with constexpr as you proposed.

This code is used from `editor/xml_feature.cpp` to convert short URLs to full URLs for OSM API. I simplified it with `constexpr` as you proposed.
strump reviewed 2023-01-23 11:16:17 +00:00
indexer/validate_and_format_contacts.cpp
strump commented 2023-01-23 11:16:17 +00:00
Author
Member

You mean replace:

  return metaID == MapObject::MetadataID::FMD_CONTACT_INSTAGRAM ||
         metaID == MapObject::MetadataID::FMD_CONTACT_FACEBOOK ||
         metaID == MapObject::MetadataID::FMD_CONTACT_TWITTER ||
         metaID == MapObject::MetadataID::FMD_CONTACT_VK ||
         metaID == MapObject::MetadataID::FMD_CONTACT_LINE;

with

switch (metaID)
{
  case MapObject::MetadataID::FMD_CONTACT_INSTAGRAM:
  case MapObject::MetadataID::FMD_CONTACT_FACEBOOK:
  case MapObject::MetadataID::FMD_CONTACT_TWITTER:
  case MapObject::MetadataID::FMD_CONTACT_VK:
  case MapObject::MetadataID::FMD_CONTACT_LINE:
   return true;
  default:
    return false;
}

? I don't know how it would affect performance. We can try and use switch.

You mean replace: ``` return metaID == MapObject::MetadataID::FMD_CONTACT_INSTAGRAM || metaID == MapObject::MetadataID::FMD_CONTACT_FACEBOOK || metaID == MapObject::MetadataID::FMD_CONTACT_TWITTER || metaID == MapObject::MetadataID::FMD_CONTACT_VK || metaID == MapObject::MetadataID::FMD_CONTACT_LINE; ``` with ``` switch (metaID) { case MapObject::MetadataID::FMD_CONTACT_INSTAGRAM: case MapObject::MetadataID::FMD_CONTACT_FACEBOOK: case MapObject::MetadataID::FMD_CONTACT_TWITTER: case MapObject::MetadataID::FMD_CONTACT_VK: case MapObject::MetadataID::FMD_CONTACT_LINE: return true; default: return false; } ``` ? I don't know how it would affect performance. We can try and use `switch`.
strump reviewed 2023-01-23 11:17:32 +00:00
indexer/validate_and_format_contacts.cpp
strump commented 2023-01-23 11:17:32 +00:00
Author
Member

Added constexpr for all "contact:*" strings in validate_and_format_contacts.cpp.

Added `constexpr` for all `"contact:*"` strings in `validate_and_format_contacts.cpp`.
strump reviewed 2023-01-23 11:20:40 +00:00
indexer/validate_and_format_contacts.cpp
strump commented 2023-01-23 11:20:39 +00:00
Author
Member

Fixed.

Fixed.
strump reviewed 2023-01-23 11:20:47 +00:00
indexer/validate_and_format_contacts.cpp
strump commented 2023-01-23 11:20:47 +00:00
Author
Member

Fixed.

Fixed.
strump reviewed 2023-01-23 11:34:38 +00:00
indexer/validate_and_format_contacts.cpp
strump commented 2023-01-23 11:34:38 +00:00
Author
Member

Line normalization is tricky. You can find some examples in validate_and_format_contacts_test.cpp file. E.g.

  • https://line.me/R/ti/p/gdltt7s380 → can extract username gdltt7s380
  • https://line.me/R/home/public/profile?id=r90ck7n1rq → can extract username r90ck7n1rq
  • https://liff.line.me/1645278921-kWRPP32q/?accountId=673watcr → no username and different domain
  • https://page.line.me/?accountId=673watcr → different domain and accountId is not the same thing as username.

So we store domain (no HTTP(S) protocol) for contact:line.

Line normalization is tricky. You can find some examples in `validate_and_format_contacts_test.cpp` file. E.g. * `https://line.me/R/ti/p/gdltt7s380` → can extract username `gdltt7s380` * `https://line.me/R/home/public/profile?id=r90ck7n1rq` → can extract username `r90ck7n1rq` * `https://liff.line.me/1645278921-kWRPP32q/?accountId=673watcr` → no username and different domain * `https://page.line.me/?accountId=673watcr` → different domain and `accountId` is not the same thing as username. So we store domain (no HTTP(S) protocol) for `contact:line`.
strump reviewed 2023-01-23 11:34:53 +00:00
indexer/validate_and_format_contacts.cpp
strump commented 2023-01-23 11:34:53 +00:00
Author
Member

Removed mobile URL code

Removed mobile URL code
strump reviewed 2023-01-23 11:35:04 +00:00
indexer/validate_and_format_contacts.hpp
strump commented 2023-01-23 11:35:04 +00:00
Author
Member

Fixed formatting.

Fixed formatting.
strump reviewed 2023-01-23 11:36:49 +00:00
iphone/CoreApi/CoreApi/PlacePageData/Common/PlacePageInfoData.mm
strump commented 2023-01-23 11:36:48 +00:00
Author
Member

Reverted this code. Copy raw data here.

Reverted this code. Copy raw data here.
strump reviewed 2023-01-23 12:34:27 +00:00
iphone/Maps/UI/Editor/MWMEditorViewController.mm
@ -423,3 +428,4 @@
text:value
placeholder:name
keyboardType:UIKeyboardTypeDefault
capitalization:UITextAutocapitalizationTypeSentences];
strump commented 2023-01-23 12:34:26 +00:00
Author
Member

As we do with Android, short URLs should be converted to full URLs in editor. This is what issue #2787 is about.

Editor validates those values and stores short URLs.

As we do with Android, short URLs should be converted to full URLs in editor. This is what issue #2787 is about. Editor validates those values and stores short URLs.
strump reviewed 2023-01-23 12:45:54 +00:00
iphone/Maps/UI/PlacePage/Components/PlacePageInfoViewController.swift
strump commented 2023-01-23 12:45:54 +00:00
Author
Member

Yes, removing @ symbol would make code more simple. But for consistency with Android I'd rather keep it.

I simplified this code by introducing func prependAtIfNeed(_ contact: String) -> String

Yes, removing `@` symbol would make code more simple. But for consistency with Android I'd rather keep it. I simplified this code by introducing `func prependAtIfNeed(_ contact: String) -> String`
strump reviewed 2023-01-23 13:14:35 +00:00
android/src/app/organicmaps/editor/Editor.java
strump commented 2023-01-23 13:14:35 +00:00
Author
Member

Fixed

Fixed
strump reviewed 2023-01-23 13:15:33 +00:00
indexer/validate_and_format_contacts.cpp
@ -61,3 +89,3 @@
{
if (!v.empty() && !IsProtocolSpecified(v))
return kWebsiteProtocols[kWebsiteProtocolDefaultIndex] + v;
return string{kHttp}.append(v);
strump commented 2023-01-23 13:15:33 +00:00
Author
Member

Fixed

Fixed
strump reviewed 2023-01-23 13:32:10 +00:00
indexer/validate_and_format_contacts.cpp
strump commented 2023-01-23 13:32:09 +00:00
Author
Member

I added ASSERT() and added @NonNull to the argument of Editor.nativeGetFullUrl(int id, @NonNull String value)

I added `ASSERT()` and added @NonNull to the argument of `Editor.nativeGetFullUrl(int id, @NonNull String value)`
biodranik (Migrated from github.com) reviewed 2023-01-28 16:07:12 +00:00
android/jni/app/organicmaps/DownloadResourcesLegacyActivity.cpp
biodranik (Migrated from github.com) commented 2023-01-28 14:26:35 +00:00

I don't think that Platform is needed below. The previous include was correct.

I know that you want to fix a warning, but it's not a trivial one. We likely need to rename the Android header with a different name. But let's leave it for a separate PR.

I don't think that Platform is needed below. The previous include was correct. I know that you want to fix a warning, but it's not a trivial one. We likely need to rename the Android header with a different name. But let's leave it for a separate PR.
android/jni/app/organicmaps/Framework.cpp
biodranik (Migrated from github.com) commented 2023-01-28 14:27:31 +00:00

Are you sure here? Let's leave it for another PR.

Are you sure here? Let's leave it for another PR.
android/jni/app/organicmaps/editor/Editor.cpp
biodranik (Migrated from github.com) commented 2023-01-28 14:30:24 +00:00

Do we really need to pass value from the Java here? Why g_editableMapObject.GetMetadata(metaID); can't be used to get it from the C++ code?

Do we really need to pass value from the Java here? Why `g_editableMapObject.GetMetadata(metaID);` can't be used to get it from the C++ code?
android/src/app/organicmaps/bookmarks/data/MapObject.java
biodranik (Migrated from github.com) commented 2023-01-28 14:32:12 +00:00

Is it a valid scenario? If not, let's log some warning here.

Is it a valid scenario? If not, let's log some warning here.
biodranik (Migrated from github.com) commented 2023-01-28 14:33:46 +00:00

Actually, the previous code did not check for null. It would be better to crash here and fix the bug, instead of debugging strange behavior later, right?

Actually, the previous code _did not check_ for null. It would be better to crash here and fix the bug, instead of debugging strange behavior later, right?
biodranik (Migrated from github.com) commented 2023-01-28 14:34:45 +00:00

res can be extracted in nativeGetFullUrl, right?

res can be extracted in nativeGetFullUrl, right?
android/src/app/organicmaps/widget/placepage/PlacePageView.java
biodranik (Migrated from github.com) commented 2023-01-28 14:38:05 +00:00

As we discussed before, appending @ may confuse users. Don't use any @ symbols can make the code and UI cleaner. We can leave it for another PR.

As we discussed before, appending @ may confuse users. Don't use any @ symbols can make the code and UI cleaner. We can leave it for another PR.
android/src/app/organicmaps/widget/placepage/PlacePageView.java
biodranik (Migrated from github.com) commented 2023-01-28 14:36:40 +00:00

Can Editor.nativeGetFullUrl(Metadata.MetadataType.FMD_CONTACT_FACEBOOK); be used here directly? What are the benefits of mMapObject proxy?

Can `Editor.nativeGetFullUrl(Metadata.MetadataType.FMD_CONTACT_FACEBOOK);` be used here directly? What are the benefits of mMapObject proxy?
android/src/app/organicmaps/widget/placepage/PlacePageView.java
biodranik (Migrated from github.com) commented 2023-01-28 14:39:15 +00:00

Do you find @ symbol useful? It takes important horizontal space )

Do you find @ symbol useful? It takes important horizontal space )
editor/editor_tests/xml_feature_test.cpp
biodranik (Migrated from github.com) commented 2023-01-28 14:40:02 +00:00

Does it make sense to add tests for other social networks too? Especially for strange Line cases?

Does it make sense to add tests for other social networks too? Especially for strange Line cases?
indexer/validate_and_format_contacts.cpp
biodranik (Migrated from github.com) commented 2023-01-28 14:41:48 +00:00

Moving "profile.php" to constexpr string_view may make some comparisons faster.

Moving "profile.php" to constexpr string_view may make some comparisons faster.
indexer/validate_and_format_contacts.cpp
biodranik (Migrated from github.com) commented 2023-01-28 16:05:53 +00:00

Let's avoid the copy-paste by using constexpr string_view for hosts.

Let's avoid the copy-paste by using constexpr string_view for hosts.
indexer/validate_and_format_contacts.cpp
biodranik (Migrated from github.com) commented 2023-01-28 16:03:11 +00:00

That's not important. It's nit )

That's not important. It's nit )
strump reviewed 2023-01-31 08:26:36 +00:00
indexer/validate_and_format_contacts.cpp
strump commented 2023-01-31 08:26:36 +00:00
Author
Member

What's the best way to concat two string_view values? Stackoverflow proposes convertion to string.

constexpr string_view urlInstagram{"https://instagram.com/"};
string socialContactToURL(string_view tag, string_view value)
{
    return urlInstagram + value;
}

gives error: invalid operands to binary expression ('const std::string_view' (aka 'const basic_string_view<char>') and 'std::string_view' (aka 'basic_string_view<char>'))

What's the best way to concat two `string_view` values? [Stackoverflow](https://stackoverflow.com/q/73478427) proposes convertion to `string`. ``` constexpr string_view urlInstagram{"https://instagram.com/"}; string socialContactToURL(string_view tag, string_view value) { return urlInstagram + value; } ``` gives error: `invalid operands to binary expression ('const std::string_view' (aka 'const basic_string_view<char>') and 'std::string_view' (aka 'basic_string_view<char>'))`
biodranik (Migrated from github.com) reviewed 2023-01-31 12:53:33 +00:00
indexer/validate_and_format_contacts.cpp
biodranik (Migrated from github.com) commented 2023-01-31 12:53:32 +00:00

string{urlInstagram} + value should help. You can't join views, they just point to some other data.

`string{urlInstagram} + value` should help. You can't join views, they just point to some other data.
strump reviewed 2023-01-31 16:58:58 +00:00
indexer/validate_and_format_contacts.cpp
strump commented 2023-01-31 16:58:57 +00:00
Author
Member

Introduced constexpr variables for "profile.php", domain names, URLs etc.

Introduced `constexpr` variables for "profile.php", domain names, URLs etc.
strump reviewed 2023-01-31 16:59:45 +00:00
android/src/app/organicmaps/widget/placepage/PlacePageView.java
strump commented 2023-01-31 16:59:44 +00:00
Author
Member

Removed mMapObject.getContactUrl() method. Introduced Framework.nativeGetPoiContactUrl(id) to simplify code.

Removed `mMapObject.getContactUrl()` method. Introduced `Framework.nativeGetPoiContactUrl(id)` to simplify code.
strump reviewed 2023-01-31 17:00:03 +00:00
android/src/app/organicmaps/bookmarks/data/MapObject.java
strump commented 2023-01-31 17:00:03 +00:00
Author
Member

Introduced Framework.nativeGetPoiContactUrl() function instead

Introduced `Framework.nativeGetPoiContactUrl()` function instead
strump reviewed 2023-01-31 17:00:54 +00:00
android/jni/app/organicmaps/DownloadResourcesLegacyActivity.cpp
strump commented 2023-01-31 17:00:54 +00:00
Author
Member

I saw compiler warnings and changed include statement.
Reverting it back.

I saw compiler warnings and changed include statement. Reverting it back.
strump reviewed 2023-01-31 17:01:04 +00:00
android/jni/app/organicmaps/Framework.cpp
strump commented 2023-01-31 17:01:04 +00:00
Author
Member

Ok, reverting

Ok, reverting
strump reviewed 2023-01-31 17:01:28 +00:00
android/src/app/organicmaps/widget/placepage/PlacePageView.java
strump commented 2023-01-31 17:01:28 +00:00
Author
Member

Removed @ symbol from Android and iOS place page UI

Removed `@` symbol from Android and iOS place page UI
strump reviewed 2023-01-31 17:11:49 +00:00
editor/editor_tests/xml_feature_test.cpp
strump commented 2023-01-31 17:11:49 +00:00
Author
Member

We have tests for normalization code: instagram.com/some_caffe_uk -> some_caffe_uk (for all social networks)
And I added tests for denormalization code: some_caffe_uk -> https://instagram.com/some_caffe_uk (for all social networks)
This test verifies that normalization was applied (reading from OSM XML) and then denormalization (writing to OSM XML). I think we don't need to add all cases here. Just to make sure that input data is normalized and output is denormalized back.

N.B. Output values not always equal to input when we talk about OSM XML.

We have tests for normalization code: `instagram.com/some_caffe_uk` -> `some_caffe_uk` (for all social networks) And I added tests for denormalization code: `some_caffe_uk` -> `https://instagram.com/some_caffe_uk` (for all social networks) This test verifies that normalization was applied (reading from OSM XML) and then denormalization (writing to OSM XML). I think we don't need to add all cases here. Just to make sure that input data is normalized and output is denormalized back. N.B. Output values not always equal to input when we talk about OSM XML.
biodranik (Migrated from github.com) reviewed 2023-01-31 18:26:19 +00:00
biodranik (Migrated from github.com) left a comment

Almost done!

@Jean-BaptisteC can you please test this PR?

Almost done! @Jean-BaptisteC can you please test this PR?
base/string_utils.cpp
biodranik (Migrated from github.com) commented 2023-01-31 18:25:53 +00:00

Is there a unit-test for this method?

Is there a unit-test for this method?
indexer/validate_and_format_contacts.cpp
biodranik (Migrated from github.com) commented 2023-01-31 18:22:54 +00:00

According to our docs/CPP_STYLE.md:

  • Compile-time constants must be named in camelCase, starting with a lower-case k, e.g. kCompileTimeConstant and marked as constexpr when possible.

So all constants should be named like kConstant.

According to our docs/CPP_STYLE.md: > - Compile-time constants must be named in camelCase, starting with a lower-case `k`, e.g. `kCompileTimeConstant` and marked as `constexpr` when possible. So all constants should be named like kConstant.
indexer/validate_and_format_contacts.cpp
biodranik (Migrated from github.com) commented 2023-01-31 18:15:34 +00:00

It is not critical, but it is better to avoid unnecessary memory allocation for a temporary string object. The optimal way to do it (here and in other places):

    return string{urlInstagram}.append(value);
It is not critical, but it is better to avoid unnecessary memory allocation for a temporary string object. The optimal way to do it (here and in other places): ```suggestion return string{urlInstagram}.append(value); ```
strump reviewed 2023-01-31 19:33:06 +00:00
indexer/validate_and_format_contacts.cpp
strump commented 2023-01-31 19:33:06 +00:00
Author
Member

Replaced + with .append(...) in the file.

Replaced `+` with `.append(...)` in the file.
strump reviewed 2023-01-31 19:33:32 +00:00
indexer/validate_and_format_contacts.cpp
strump commented 2023-01-31 19:33:32 +00:00
Author
Member

Ok, didn't see style guide.
Renamed constants in the file.

Ok, didn't see style guide. Renamed constants in the file.
strump reviewed 2023-01-31 19:33:41 +00:00
base/string_utils.cpp
strump commented 2023-01-31 19:33:40 +00:00
Author
Member

Added tests for EndsWith(string, string_view)

Added tests for EndsWith(string, string_view)
biodranik (Migrated from github.com) approved these changes 2023-02-01 14:30:32 +00:00
biodranik (Migrated from github.com) left a comment

You've made it! 🎉

You've made it! 🎉
android/jni/app/organicmaps/Framework.cpp
biodranik (Migrated from github.com) commented 2023-02-01 13:59:25 +00:00

nit: if the variable is not modified, and is not optimized by RVO (returned from the function), then we always mark it as const.

nit: if the variable is not modified, and is not optimized by RVO (returned from the function), then we always mark it as const.
strump reviewed 2023-02-01 14:42:07 +00:00
android/jni/app/organicmaps/Framework.cpp
strump commented 2023-02-01 14:42:07 +00:00
Author
Member

string_view const or const string_view ? I'm confused with C++ const semantics 😊

`string_view const` or `const string_view` ? I'm confused with C++ const semantics 😊
biodranik (Migrated from github.com) reviewed 2023-02-01 15:49:04 +00:00
android/jni/app/organicmaps/Framework.cpp
biodranik (Migrated from github.com) commented 2023-02-01 15:49:04 +00:00

It is the same :) But we use the right-to-left notation (check our CPP_STYLE.md for details).

It is the same :) But we use the right-to-left notation (check our CPP_STYLE.md for details).
biodranik (Migrated from github.com) reviewed 2023-02-01 15:49:42 +00:00
android/jni/app/organicmaps/Framework.cpp
biodranik (Migrated from github.com) commented 2023-02-01 15:49:41 +00:00

For example: string_view const * const reads as const pointer to a const string_view.

For example: `string_view const * const` reads as const pointer to a const string_view.
strump reviewed 2023-02-01 16:19:09 +00:00
android/jni/app/organicmaps/Framework.cpp
strump commented 2023-02-01 16:19:09 +00:00
Author
Member

Done!

Done!
Jean-BaptisteC (Migrated from github.com) approved these changes 2023-02-04 09:46:39 +00:00
Jean-BaptisteC (Migrated from github.com) left a comment

Pixel 6 - Android 13 ✅
Facebook:

Instagram:

Twitter:

VK:

LINE:

Long press on Place page works (Copy just ID and copy full URL) ✅
We can't add instagram link in Facebook field (Tested on all socials fields) ✅

I have create account on master dev osm and add in OM debug, but changes are not upload

Pixel 6 - Android 13 ✅ Facebook: - ID + Full URL Tested with https://www.openstreetmap.org/node/2864998473 Instagram: - ID + Full URL Tested with https://www.openstreetmap.org/node/2864998473 Twitter: - ID + Full URL https://www.openstreetmap.org/node/9722726898 VK: - ID + Full URL https://www.openstreetmap.org/node/3199685169 LINE: - ID + Full URL https://www.openstreetmap.org/node/4007245809 Long press on Place page works (Copy just ID and copy full URL) ✅ We can't add instagram link in Facebook field (Tested on all socials fields) ✅ I have create account on master dev osm and add in OM debug, but changes are not upload
vng (Migrated from github.com) requested changes 2023-02-19 20:50:04 +00:00
vng (Migrated from github.com) left a comment

I suspect that updating kothic submodule is not needed in this PR.
Don't commit it, just make git submodule update ...

UPD: And twine is also not needed here.

I suspect that updating kothic submodule is not needed in this PR. Don't commit it, just make git submodule update ... UPD: And twine is also not needed here.
base/string_utils.cpp
@ -332,0 +333,4 @@
return EndsWith(s1, std::string_view(s2));
}
bool EndsWith(std::string const & s1, std::string_view s2)
vng (Migrated from github.com) commented 2023-02-19 20:41:11 +00:00

Now you can simply call EndsWith(s1, std::string_view(s2)) here

Now you can simply call EndsWith(s1, std::string_view(s2)) here
editor/editor_tests/xml_feature_test.cpp
vng (Migrated from github.com) commented 2023-02-19 20:43:37 +00:00

Here is the shorter way to construct string from view:
std::string contactFacebook(view);

Here is the shorter way to construct string from view: std::string contactFacebook(view);
indexer/indexer_tests/validate_and_format_contacts_test.cpp
vng (Migrated from github.com) commented 2023-02-19 20:44:49 +00:00

{ from new line

{ from new line
strump reviewed 2023-02-19 21:26:59 +00:00
strump left a comment
Author
Member

Looks like this PR updates submodules kothic and twine. It’s by accident probably after rebase. Need to revert submodule changes…
Update: Done!

Looks like this PR updates submodules kothic and twine. It’s by accident probably after rebase. Need to revert submodule changes… Update: Done!
strump reviewed 2023-02-20 09:19:53 +00:00
base/string_utils.cpp
@ -332,0 +333,4 @@
return EndsWith(s1, std::string_view(s2));
}
bool EndsWith(std::string const & s1, std::string_view s2)
strump commented 2023-02-20 09:19:53 +00:00
Author
Member

Fixed

Fixed
strump reviewed 2023-02-20 09:20:03 +00:00
editor/editor_tests/xml_feature_test.cpp
strump commented 2023-02-20 09:20:03 +00:00
Author
Member

Fixed

Fixed
strump reviewed 2023-02-20 09:20:12 +00:00
indexer/indexer_tests/validate_and_format_contacts_test.cpp
strump commented 2023-02-20 09:20:12 +00:00
Author
Member

Fixed formatting

Fixed formatting
vng (Migrated from github.com) approved these changes 2023-02-20 12:38:32 +00:00
vng (Migrated from github.com) reviewed 2023-02-20 12:39:59 +00:00
base/string_utils.cpp
@ -332,2 +335,4 @@
bool EndsWith(std::string const & s1, std::string_view s2)
{
size_t const n = s1.size();
vng (Migrated from github.com) commented 2023-02-20 12:39:58 +00:00

Replace all the function's body here :)

bool EndsWith(std::string const & s1, char const * s2)
{
  return EndsWith(s1, std::string_view(s2));
}
Replace all the function's body here :) ``` bool EndsWith(std::string const & s1, char const * s2) { return EndsWith(s1, std::string_view(s2)); } ```
strump reviewed 2023-02-20 13:52:32 +00:00
base/string_utils.cpp
@ -332,2 +335,4 @@
bool EndsWith(std::string const & s1, std::string_view s2)
{
size_t const n = s1.size();
strump commented 2023-02-20 13:52:32 +00:00
Author
Member

Replaced body 😊

Replaced body 😊
This repo is archived. You cannot comment on pull requests.
Reviewers
No reviewers
biodranik
Jean-BaptisteC
vng
Labels
Clear labels   
Accessibility

issues affecting users with specific needs

  
Accessibility

issues affecting users with specific needs

  
Address

Issues with addresses, post codes, city/town/village borders

  
Address

Issues with addresses, post codes, city/town/village borders

  
Android

Android development

  
Android

Android development

  
Android Auto

Android Auto

  
Android Auto

Android Auto

  
Android Automotive (AAOS)

   
Android Automotive (AAOS)

   
API

Short Links, deep links and url schemes, including ge0 and om://

  
API

Short Links, deep links and url schemes, including ge0 and om://

  
AppGallery

Huawei AppGallery

  
AppGallery

Huawei AppGallery

  
AppStore

Apple AppStore

  
AppStore

Apple AppStore

  
Battery and Performance

Performance and battery consumption-related issues and optimizations

  
Battery and Performance

Performance and battery consumption-related issues and optimizations

  
Blocker

An issue blocking the release

  
Blocker

An issue blocking the release

  
Bookmarks and Tracks

Bookmarks, imported tracks, and KML, KMZ, KMB, GPX, GPZ import or export

  
Bookmarks and Tracks

Bookmarks, imported tracks, and KML, KMZ, KMB, GPX, GPZ import or export

  
Borders

Country borders and polygons

  
Borders

Country borders and polygons

  
Bug

Something isn't working

  
Bug

Something isn't working

  
Build

Compilation issue

  
Build

Compilation issue

  
CarPlay

CarPlay Integration

  
CarPlay

CarPlay Integration

  
Classificator

Classify object by types

  
Classificator

Classify object by types

  
Community

Community relations

  
Community

Community relations

  
Core

Cross-platform C++ libraries with a core functionality

  
Core

Cross-platform C++ libraries with a core functionality

  
CrashReports

Crash Reporting

  
CrashReports

Crash Reporting

  
Cycling

Bike, cycle, biking and related

  
Cycling

Bike, cycle, biking and related

  
Desktop

Desktop application

  
Desktop

Desktop application

  
DevEx

Developer Experience

  
DevEx

Developer Experience

  
DevOps

Automation and pipelines

  
DevOps

Automation and pipelines

  
dev_sandbox

A desktop app to debug rendering

  
dev_sandbox

A desktop app to debug rendering

  
Directions

Issues with next turns, labels, streets, voice instructions

  
Directions

Issues with next turns, labels, streets, voice instructions

  
Documentation

Improvements or additions to documentation

  
Documentation

Improvements or additions to documentation

  
Downloader

Map dowloader

  
Downloader

Map dowloader

  
Drape

Drape OpenGL, Vulkan and Metal graphics rendering engine

  
Drape

Drape OpenGL, Vulkan and Metal graphics rendering engine

  
Driving

Drive a car related issues

  
Driving

Drive a car related issues

  
Duplicate

This issue or pull request already exists

  
Duplicate

This issue or pull request already exists

  
Editor

OSM Editor

  
Editor

OSM Editor

  
Elevation

Isolines and altitude

  
Elevation

Isolines and altitude

  
Enhancement

New feature or request, an improvement of some existing feature

  
Enhancement

New feature or request, an improvement of some existing feature

  
Epic

A large body work

  
Epic

A large body work

  
External Map Datasets

TIGER, Ordnance Survey, ASTER, SRTM, etc.

  
External Map Datasets

TIGER, Ordnance Survey, ASTER, SRTM, etc.

  
F-Droid

F-Droid build and distribution

  
F-Droid

F-Droid build and distribution

  
Fonts

Font and text rendering

  
Fonts

Font and text rendering

  
Frequently User Reported

Most annoying UX and other issues for users reported by many people

  
Frequently User Reported

Most annoying UX and other issues for users reported by many people

  
Fund

   
Fund

   
Generator

Related to generator tool.

  
Generator

Related to generator tool.

  
Good first issue

Good for newcomers

  
Good first issue

Good for newcomers

  
Google Play

Google Play

  
Google Play

Google Play

  
GPS

Location and positioning issues

  
GPS

Location and positioning issues

  
GSoC

Ideas for Google Summer of Code

  
GSoC

Ideas for Google Summer of Code

  
iCloud

   
iCloud

   
Icons

Map and app icons

  
Icons

Map and app icons

  
iOS

iOS development

  
iOS

iOS development

  
Legal

Legal aspects

  
Legal

Legal aspects

  
Linux Desktop

Ubuntu, Gentoo, Red Hat, Debian, Cent OS, Fedora, etc.

  
Linux Desktop

Ubuntu, Gentoo, Red Hat, Debian, Cent OS, Fedora, etc.

  
Linux packaging

   
Linux packaging

   
Linux Phone

LibreM, PinePhone, Wayland and other mobile Linux versions

  
Linux Phone

LibreM, PinePhone, Wayland and other mobile Linux versions

  
Mac OS

Desktop issues related to Mac OS X, OM running on M1, M2, M3 Apple silicon, or Qt version

  
Mac OS

Desktop issues related to Mac OS X, OM running on M1, M2, M3 Apple silicon, or Qt version

  
Map Data

OpenStreetMap data related issues

  
Map Data

OpenStreetMap data related issues

  
Metro

Subway, Tube, light-rail public transport issues

  
Metro

Subway, Tube, light-rail public transport issues

  
Navigation

Issues related to the active Navigator / Navigation mode

  
Navigation

Issues related to the active Navigator / Navigation mode

  
Need Feedback

Further information is requested

  
Need Feedback

Further information is requested

  
Night Mode

Night or dark mode feature

  
Night Mode

Night or dark mode feature

  
NLnet 2024-06-281

Tasks related to NLnet 2024-06-281 project.

  
NLnet 2024-06-281

Tasks related to NLnet 2024-06-281 project.

  
No Feature Parity

A platform needs this to be implemented to have feature parity with other platforms

  
No Feature Parity

A platform needs this to be implemented to have feature parity with other platforms

  
Opening Hours

Business Hours, off time, holidays, seasonal

  
Opening Hours

Business Hours, off time, holidays, seasonal

  
Outdoors

Hiking, mtb, 4x4...

  
Outdoors

Hiking, mtb, 4x4...

  
POI Info

Feature metadata, OSM tags that are displayed in Place Page

  
POI Info

Feature metadata, OSM tags that are displayed in Place Page

  
Privacy

Privacy

  
Privacy

Privacy

  
Public Transport

Bus, trolleybus, train, tram, metro, subway, light rail

  
Public Transport

Bus, trolleybus, train, tram, metro, subway, light rail

  
Raw Idea

An idea that needs to be elaborated

  
Raw Idea

An idea that needs to be elaborated

  
Refactoring

   
Refactoring

   
Regional

This issue is different depending on the country or region

  
Regional

This issue is different depending on the country or region

  
Regression

A regression bug

  
Regression

A regression bug

  
Releases

Meta-tickets for tracking the release procedure

  
Releases

Meta-tickets for tracking the release procedure

  
RoboTest

Robo / Monkey Auto Tests

  
RoboTest

Robo / Monkey Auto Tests

  
Route Planning

Preview and plan your track

  
Route Planning

Preview and plan your track

  
Routing

Route building issues, e.g. valid route, valid ETA

  
Routing

Route building issues, e.g. valid route, valid ETA

  
Ruler

Helicopter routing, "as a crow flies"

  
Ruler

Helicopter routing, "as a crow flies"

  
Search

Search

  
Search

Search

  
Security

Security improvements

  
Security

Security improvements

  
Styles

Map drawing styles

  
Styles

Map drawing styles

  
Tests

   
Tests

   
Track Recording

Track Recording

  
Track Recording

Track Recording

  
Translations

Localization and translations issues

  
Translations

Localization and translations issues

  
TTS

Text-to-Speech

  
TTS

Text-to-Speech

  
UI

User interface issues

  
UI

User interface issues

  
UX

User eXperience, an issue with usability

  
UX

User eXperience, an issue with usability

  
Walk Navigation

Foot pedestrian mode

  
Walk Navigation

Foot pedestrian mode

  
Watches

Android Watches or Apple Watches support

  
Watches

Android Watches or Apple Watches support

  
Web

https://organicmaps.app/

  
Web

https://organicmaps.app/

  
Wikipedia

Issues related to Wiki articles embedded in OM data files.

  
Wikipedia

Issues related to Wiki articles embedded in OM data files.

  
Windows

Windows

  
Windows

Windows

  
Won't fix

Unfeasible or impossible to resolve

  
Won't fix

Unfeasible or impossible to resolve

  
World Map

Issues about improving the base map of the whole world that is bundled with the app

  
World Map

Issues about improving the base map of the whole world that is bundled with the app

No labels
Accessibility
Accessibility
Address
Address
Android
Android
Android Auto
Android Auto
Android Automotive (AAOS)
Android Automotive (AAOS)
API
API
AppGallery
AppGallery
AppStore
AppStore
Battery and Performance
Battery and Performance
Blocker
Blocker
Bookmarks and Tracks
Bookmarks and Tracks
Borders
Borders
Bug
Bug
Build
Build
CarPlay
CarPlay
Classificator
Classificator
Community
Community
Core
Core
CrashReports
CrashReports
Cycling
Cycling
Desktop
Desktop
DevEx
DevEx
DevOps
DevOps
dev_sandbox
dev_sandbox
Directions
Directions
Documentation
Documentation
Downloader
Downloader
Drape
Drape
Driving
Driving
Duplicate
Duplicate
Editor
Editor
Elevation
Elevation
Enhancement
Enhancement
Epic
Epic
External Map Datasets
External Map Datasets
F-Droid
F-Droid
Fonts
Fonts
Frequently User Reported
Frequently User Reported
Fund
Fund
Generator
Generator
Good first issue
Good first issue
Google Play
Google Play
GPS
GPS
GSoC
GSoC
iCloud
iCloud
Icons
Icons
iOS
iOS
Legal
Legal
Linux Desktop
Linux Desktop
Linux packaging
Linux packaging
Linux Phone
Linux Phone
Mac OS
Mac OS
Map Data
Map Data
Metro
Metro
Navigation
Navigation
Need Feedback
Need Feedback
Night Mode
Night Mode
NLnet 2024-06-281
NLnet 2024-06-281
No Feature Parity
No Feature Parity
Opening Hours
Opening Hours
Outdoors
Outdoors
POI Info
POI Info
Privacy
Privacy
Public Transport
Public Transport
Raw Idea
Raw Idea
Refactoring
Refactoring
Regional
Regional
Regression
Regression
Releases
Releases
RoboTest
RoboTest
Route Planning
Route Planning
Routing
Routing
Ruler
Ruler
Search
Search
Security
Security
Styles
Styles
Tests
Tests
Track Recording
Track Recording
Translations
Translations
TTS
TTS
UI
UI
UX
UX
Walk Navigation
Walk Navigation
Watches
Watches
Web
Web
Wikipedia
Wikipedia
Windows
Windows
Won't fix
Won't fix
World Map
World Map
Milestone
Clear milestone
No items
No milestone
Next Release
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: organicmaps/organicmaps-tmp#3248
No description provided.
Delete branch "issue-2787-android-social-pages-editor"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?