UI to export bookmarks as GPX #8137
Conversation
Signed-off-by: cyber-toad <the.cyber.toad@proton.me>
Signed-off-by: cyber-toad <the.cyber.toad@proton.me>
Signed-off-by: cyber-toad <the.cyber.toad@proton.me>
Signed-off-by: cyber-toad <the.cyber.toad@proton.me>
| @@ -83,7 +83,7 @@ std::string GetFileNameForExport(BookmarkManager::KMLDataCollectionPtr::element_ | |||
| return fileName; | |||
| } | |||
|
|
|||
| BookmarkManager::SharingResult ExportSingleFile(BookmarkManager::KMLDataCollectionPtr::element_type::value_type const & kmlToShare) | |||
| BookmarkManager::SharingResult ExportSingleFileKml(BookmarkManager::KMLDataCollectionPtr::element_type::value_type const & kmlToShare) | |||
There was a problem hiding this comment.
- What is the value of copy-pasting the code into a new function? Let's extend this one instead and add switch inside.
- Let's support exporting KML, KMZ and GPX if it's easy.
There was a problem hiding this comment.
- Export to GPX doesn't have export zip and cleanup of temporary file, so about half of the code is different. What benefit we have if we put the same code into switch branches - now each function covers specific scenario?
if we merge them we need to useifseveral time to adjust logic - and there is a problem toifguard. - I had an idea to add KMZ enum value to distinguish between export of single track (Text) and export all (Kmz). But because we split export/export all by number of collections I removed it for now (until there is no 'Export all to Gpz') because it is misleading now.
There was a problem hiding this comment.
What happens with that temporary file? Is it cleaned later by the system somehow?
There was a problem hiding this comment.
What happens with that temporary file? Is it cleaned later by the system somehow?
It works this way:
- It creates file like "/data/user/0/app.organicmaps.debug/cache/Route.kml.tmp505640107344"
- File is removed by OM code and kmz file is created
- It shares file like "/data/user/0/app.organicmaps.debug/cache/Route.kmz"
- In Android studio file browser I see folder /data/data/app.organicmaps.debug/cache on my phone after sharing is done with several tracks including the latest one "Route.kmz"
In this folder I see some tracks for previous days also. I was not able to find cleanup code for this folder in OM code, based on this answer https://stackoverflow.com/questions/9942560/when-to-clear-the-cache-dir-in-android application should do some housekeeping for it. But better to check it with our Android experts, I could be wrong. I tried to check this folder for non-debug build of OM that I have on my phone, but unfortunately there is an error
There was a problem hiding this comment.
Interesting, so we have some temporary files stuck there indefinitely... @rtsisyk @Jean-BaptisteC is there a way to know when a shared file was accessed/read by some other app and can be safely deleted? What Android guidelines say about it?
P.S. It's better to test beta or web flavor for non-debug builds.
| @@ -1475,7 +1475,7 @@ UNIT_CLASS_TEST(Runner, ExportAll) | |||
| TEST(base::DeleteFileX(indexPath), ()); | |||
| TEST(base::DeleteFileX(tmpPath), ()); | |||
| }; | |||
| bmManager.PrepareFileForSharing(std::move(categories), checker); | |||
| bmManager.PrepareFileForSharing(std::move(categories), checker, KmlFileType::Text); | |||
There was a problem hiding this comment.
Does it export KML or KMZ? Previously, it was always KMZ, right?
There was a problem hiding this comment.
As described above, I removed kmz enum value and Text for multiple categories now means Put all to KMZ.
Signed-off-by: cyber-toad <the.cyber.toad@proton.me>
|
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. |
|
@Jean-BaptisteC could you please help me to reproduce the crash that you mentioned, what are the steps? |
Signed-off-by: cyber-toad <the.cyber.toad@proton.me>
There was a problem hiding this comment.
@kirylkaveryn can you please review and fix failing iOS code? And maybe add iOS buttons to export? :)
Not able to reproduce crash |
|
@cyber-toad I've updated the ios part.
|
Once code change are completed I'll create translations & generate strings.
Also iOS changes will be required.
@biodranik WDYT about the implementation?