UI to export bookmarks as GPX #8137

cyber-toad · 2024-05-10T19:39:32Z

Once code change are completed I'll create translations & generate strings.
Also iOS changes will be required.
@biodranik WDYT about the implementation?

Closes #5271

map/bookmark_manager.cpp

map/map_tests/bookmarks_test.cpp

Jean-BaptisteC · 2024-05-12T17:54:45Z

FIY, app crash when you press button to export bookmarks :)

cyber-toad · 2024-05-12T19:54:30Z

FIY, app crash when you press button to export bookmarks :)

Do you mean "Export all" button? I tried different exports and was not able to reproduce, is there a log available? Also do you build from latest source, there was an issue with accidental file removal, but I fixed it couple of days ago.

cyber-toad · 2024-05-15T18:39:42Z

@Jean-BaptisteC could you please help me to reproduce the crash that you mentioned, what are the steps?

biodranik

@kirylkaveryn can you please review and fix failing iOS code? And maybe add iOS buttons to export? :)

Jean-BaptisteC · 2024-05-19T20:48:02Z

Do you mean "Export all" button? I tried different exports and was not able to reproduce, is there a log available? Also do you build from latest source, there was an issue with accidental file removal, but I fixed it couple of days ago.

Not able to reproduce crash

kirylkaveryn · 2024-05-20T15:07:06Z

@cyber-toad I've updated the ios part.
Please, grab the last commit from here.
And add the ios tag for the generation of the string.
https://github.com/organicmaps/organicmaps/tree/ios/gpx-file-export-for-ios

cyber-toad · 2024-05-26T21:00:09Z

@biodranik please have a look at this PR. I included ios changes, added translations (without regeneration of string files for now). Let me know if something else except translation verification & string regeneration is required. It is not clear what should be done with #8137 (comment), other items are implemented.

biodranik

Some nitpicks. A menu item on ios is not translated, does the key matches the android one?

@rtsisyk can you please help and release an alpha today with all PRs marked in the milestone including this one?

@cyber-toad unrelated question: what's missing in the core to export/import timestamps for each point of a track for both kml and gpx?

Any other data that should be saved for recorded tracks?

map/bookmark_manager.cpp

map/map_tests/bookmarks_test.cpp

biodranik · 2024-05-26T23:13:27Z

map/map_tests/bookmarks_test.cpp

+  auto checker = [](BookmarkManager::SharingResult const & result)
+  {
+    auto fileName = result.m_sharingPath;
+    TEST(fileName.find("Some random route.gpx") != std::string::npos, ());


TEST_EQUAL 0?

I didn't get what should be compared with 0? fileName is "/tmp/Some random route.gpx" in this test

I see, that's confusing, then it should be renamed to filePath.

map/map_tests/bookmarks_test.cpp

kirylkaveryn · 2024-05-27T06:01:05Z

A menu item on ios is not translated, does the key matches the android one?

I've made the screen records before the string was translated and regenerated.

I will make an additional recordings soon and will add here.

biodranik · 2024-05-27T06:15:50Z

Thanks! These two strings should be replaced:

biodranik · 2024-05-27T06:53:41Z

@kirylkaveryn another issue I saw in iOS simulator when saving a GPX to Files, do we need to somehow specify/add a definition for the saved/exported .gpx file extension/mime type?

2024-05-27 08:52:12.309440+0200 Organic Maps (Debug)[8222:3200376] [NSExtension] Extension request contains input items but the extension point does not specify a set of allowed payload classes. The extension point's NSExtensionContext subclass must implement `+_allowedItemPayloadClasses`. This must return the set of allowed NSExtensionItem payload classes. In future, this request will fail with an error. Extension: <EXConcreteExtension: 0x600003347c00> {id = com.apple.DocumentManagerUICore.SaveToFiles} Items: (
    "<NSExtensionItem: 0x6000004fe070> - userInfo: {\n    NSExtensionItemAttachmentsKey =     (\n        \"<NSItemProvider: 0x600002d72680> {types = (\\n    \\\"public.plain-text\\\"\\n)}\",\n        \"<NSItemProvider: 0x600002d7a530> {types = (\\n    \\\"com.topografix.gpx\\\",\\n    \\\"public.file-url\\\"\\n)}\"\n    );\n    \"com.apple.UIKit.NSExtensionItemUserInfoIsContentManagedKey\" = 0;\n}"
)

kirylkaveryn · 2024-05-27T08:21:23Z

Thanks! These two strings should be replaced:

The strings were added correctly into the ios (check the export_file_gpx localized strings). But they are still not generated.

(after regen)

kirylkaveryn · 2024-05-27T08:45:47Z

@kirylkaveryn another issue I saw in iOS simulator when saving a GPX to Files, do we need to somehow specify/add a definition for the saved/exported .gpx file extension/mime type?

2024-05-27 08:52:12.309440+0200 Organic Maps (Debug)[8222:3200376] [NSExtension] Extension request contains input items but the extension point does not specify a set of allowed payload classes. The extension point's NSExtensionContext subclass must implement `+_allowedItemPayloadClasses`. This must return the set of allowed NSExtensionItem payload classes. In future, this request will fail with an error. Extension: <EXConcreteExtension: 0x600003347c00> {id = com.apple.DocumentManagerUICore.SaveToFiles} Items: (
    "<NSExtensionItem: 0x6000004fe070> - userInfo: {\n    NSExtensionItemAttachmentsKey =     (\n        \"<NSItemProvider: 0x600002d72680> {types = (\\n    \\\"public.plain-text\\\"\\n)}\",\n        \"<NSItemProvider: 0x600002d7a530> {types = (\\n    \\\"com.topografix.gpx\\\",\\n    \\\"public.file-url\\\"\\n)}\"\n    );\n    \"com.apple.UIKit.NSExtensionItemUserInfoIsContentManagedKey\" = 0;\n}"
)

Hmm... I can't catch such warnings on either the device and the simulator. And we haven't any extensions.
There are some other internal warnings but they do not affect the functionality.

And we already have all related type...

cyber-toad · 2024-05-27T18:47:31Z

@cyber-toad unrelated question: what's missing in the core to export/import timestamps for each point of a track for both kml and gpx?

kml::MultiGeometry that is used in TrackData uses PointWithAltitude that is (x, y, h) without timestamp. Once there is a timestamp there we can update parsers to populate.

biodranik

Thanks! @cyber-toad can you please squash all commits, and make a separate strings regeneration commit, that will also include iOS strings?

biodranik · 2024-05-27T20:06:50Z

map/map_tests/bookmarks_test.cpp

+  auto checker = [](BookmarkManager::SharingResult const & result)
+  {
+    auto fileName = result.m_sharingPath;
+    TEST(fileName.find("Some random route.gpx") != std::string::npos, ());


I see, that's confusing, then it should be renamed to filePath.

cyber-toad · 2024-05-28T07:20:20Z

Thanks! @cyber-toad can you please squash all commits, and make a separate strings regeneration commit, that will also include iOS strings?

Done

biodranik

Thanks! A few nits before merge.

biodranik · 2024-05-28T12:04:35Z

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

@@ -662,7 +662,8 @@
 	<string name="bookmarks_empty_list_title">This list is empty</string>
 	<string name="bookmarks_empty_list_message">To add a bookmark, tap a place on the map and then tap the star icon</string>
 	<string name="category_desc_more">…more</string>
-	<string name="export_file">Export file</string>
+	<string name="export_file">Export KML/KMZ file</string>


This should be in the regenerated commit to avoid conflicts.

Looks like strings.xml is still modified here. Can it be moved to another commit?

data/strings/strings.txt

map/bookmark_manager.cpp

biodranik

Looks like the changes were not pushed yet...

biodranik · 2024-05-28T18:41:11Z

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

@@ -662,7 +662,8 @@
 	<string name="bookmarks_empty_list_title">This list is empty</string>
 	<string name="bookmarks_empty_list_message">To add a bookmark, tap a place on the map and then tap the star icon</string>
 	<string name="category_desc_more">…more</string>
-	<string name="export_file">Export file</string>
+	<string name="export_file">Export KML/KMZ file</string>


Looks like strings.xml is still modified here. Can it be moved to another commit?

data/strings/strings.txt

cyber-toad · 2024-05-28T18:48:58Z

Looks like the changes were not pushed yet...

yep, I replied during applying changes should be pushed now

biodranik

Thanks, and a few (hopefully final!) nits.

map/bookmark_manager.cpp

android/app/src/main/java/app/organicmaps/bookmarks/BookmarkCategoriesFragment.java

map/bookmark_manager.cpp

Signed-off-by: cyber-toad <the.cyber.toad@proton.me>

biodranik

Thank you for your patience and for finishing a new feature!

rtsisyk · 2024-05-30T06:11:38Z

I was in the middle of testing this feature... Have you got at least one review from anybody who works on Android?

biodranik · 2024-05-30T06:21:41Z

The author tested it on Android, JB has tested it on Android, and I have tested it on Android.

Any discovered issues can be fixed.

rtsisyk · 2024-05-30T07:01:17Z

The author tested it on Android, JB has tested it on Android, and I have tested it on Android.

#8332

biodranik reviewed May 10, 2024

View reviewed changes

map/bookmark_manager.cpp Show resolved Hide resolved

map/map_tests/bookmarks_test.cpp Show resolved Hide resolved

biodranik reviewed May 16, 2024

View reviewed changes

rtsisyk requested review from rtsisyk and kavikhalique May 26, 2024 18:51

cyber-toad marked this pull request as ready for review May 26, 2024 21:01

biodranik reviewed May 26, 2024

View reviewed changes

biodranik added this to the Needs alpha/beta testing milestone May 26, 2024

biodranik reviewed May 27, 2024

View reviewed changes

cyber-toad force-pushed the gpx-export-ui branch from c8a1757 to 11f815f Compare May 27, 2024 21:14

biodranik reviewed May 28, 2024

View reviewed changes

cyber-toad force-pushed the gpx-export-ui branch from 3e051a9 to 35af4b3 Compare May 28, 2024 18:47

biodranik reviewed May 28, 2024

View reviewed changes

map/bookmark_manager.cpp Outdated Show resolved Hide resolved

android/app/src/main/java/app/organicmaps/bookmarks/BookmarkCategoriesFragment.java Outdated Show resolved Hide resolved

map/bookmark_manager.cpp Outdated Show resolved Hide resolved

cyber-toad added 2 commits May 29, 2024 00:13

[gpx] Add export buttons in Android and iOS

a4783ed

Signed-off-by: cyber-toad <the.cyber.toad@proton.me>

[strings] Regenerated

9895c93

Signed-off-by: cyber-toad <the.cyber.toad@proton.me>

cyber-toad force-pushed the gpx-export-ui branch from 35af4b3 to 9895c93 Compare May 28, 2024 22:13

biodranik approved these changes May 28, 2024

View reviewed changes

biodranik merged commit 9af27a3 into organicmaps:master May 30, 2024
14 of 16 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

UI to export bookmarks as GPX #8137

UI to export bookmarks as GPX #8137

cyber-toad commented May 10, 2024 •

edited

Jean-BaptisteC commented May 12, 2024

cyber-toad commented May 12, 2024 •

edited

cyber-toad commented May 15, 2024

biodranik left a comment

Jean-BaptisteC commented May 19, 2024

kirylkaveryn commented May 20, 2024

cyber-toad commented May 26, 2024 •

edited

biodranik left a comment

biodranik May 26, 2024

cyber-toad May 27, 2024

biodranik May 27, 2024

kirylkaveryn commented May 27, 2024

biodranik commented May 27, 2024

biodranik commented May 27, 2024 •

edited

kirylkaveryn commented May 27, 2024 •

edited

kirylkaveryn commented May 27, 2024 •

edited

cyber-toad commented May 27, 2024

biodranik left a comment

biodranik May 27, 2024

cyber-toad commented May 28, 2024

biodranik left a comment

biodranik May 28, 2024

cyber-toad May 28, 2024

biodranik May 28, 2024

biodranik left a comment

biodranik May 28, 2024

cyber-toad commented May 28, 2024

biodranik left a comment

biodranik left a comment

rtsisyk commented May 30, 2024

biodranik commented May 30, 2024

rtsisyk commented May 30, 2024

UI to export bookmarks as GPX #8137

UI to export bookmarks as GPX #8137

Conversation

cyber-toad commented May 10, 2024 • edited

Jean-BaptisteC commented May 12, 2024

cyber-toad commented May 12, 2024 • edited

cyber-toad commented May 15, 2024

biodranik left a comment

Choose a reason for hiding this comment

Jean-BaptisteC commented May 19, 2024

kirylkaveryn commented May 20, 2024

cyber-toad commented May 26, 2024 • edited

biodranik left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kirylkaveryn commented May 27, 2024

biodranik commented May 27, 2024

biodranik commented May 27, 2024 • edited

kirylkaveryn commented May 27, 2024 • edited

kirylkaveryn commented May 27, 2024 • edited

cyber-toad commented May 27, 2024

biodranik left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cyber-toad commented May 28, 2024

biodranik left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

biodranik left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cyber-toad commented May 28, 2024

biodranik left a comment

Choose a reason for hiding this comment

biodranik left a comment

Choose a reason for hiding this comment

rtsisyk commented May 30, 2024

biodranik commented May 30, 2024

rtsisyk commented May 30, 2024

cyber-toad commented May 10, 2024 •

edited

cyber-toad commented May 12, 2024 •

edited

cyber-toad commented May 26, 2024 •

edited

biodranik commented May 27, 2024 •

edited

kirylkaveryn commented May 27, 2024 •

edited

kirylkaveryn commented May 27, 2024 •

edited