Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add experimental map #1912

Merged
merged 3 commits into from
Apr 21, 2024
Merged

add experimental map #1912

merged 3 commits into from
Apr 21, 2024

Conversation

r10s
Copy link
Member

@r10s r10s commented Aug 2, 2023

this PR adds a map to show locations to iOS.

the PR makes use of the new "Webxdc Integration" feature of deltachat/deltachat-core-rust#5461 (which need to be merged first).

therefore, review should concentrate on the buttons etc. added to iOS - the map itself is totally independent and can then be improved independently of iOS.

still, some map impressions

image_2023-06-03_13-18-23 image_2023-06-03_13-19-06
image_2023-06-03_13-18-35 image_2023-06-03_13-19-19

(the third image is a result from a location-streaming-bug some years ago (iirc, all ppl-locations became POIs :), however, how we have nice test-data by that :)

the current mapx.xdc uses leaflet, and the API defined at webxdc/maps_integration.rs in core. some random thoughts:

  • mapbox was a a source of all kind of issues in the last (telemetry hard to disable, being flagged as "bad" on fdroid and things like that)
  • mapx.xdc is only 50k in side (android mapbox is about 4mb)
  • leaflet is much faster when it comes to many poi
  • at least our usage of mapbox is slow in general, startup time on desktop is about 10 seconds, slow mapbox startup time deltachat-desktop#3768
  • idea is that the webxdc can be replaced by the end-user later (eg. by sending a own variation to saved-messages)

main open point is that the xdc is currently visible in the saved-messages chat, we should consider if it is better to hide it somehow. EDIT: this will be done subsequently, for now, @nicodh and me decided that a message describing what is going on will do EDIT: default maps.xdc is no longer visible due to help of core

@r10s r10s marked this pull request as draft August 2, 2023 11:33
@r10s r10s force-pushed the add-map branch 3 times, most recently from 3c168f7 to 51d95df Compare October 29, 2023 19:55
@r10s r10s force-pushed the add-map branch 2 times, most recently from 2c483ac to 109c4f2 Compare November 10, 2023 10:40
@r10s r10s added the enhancement actually in development, user visible enhancement label Nov 12, 2023
@r10s r10s force-pushed the add-map branch 6 times, most recently from 131f350 to c497414 Compare February 14, 2024 23:41
@r10s r10s changed the title add map add experimental map Apr 5, 2024
@r10s r10s requested a review from nicodh April 5, 2024 21:56
@r10s r10s marked this pull request as ready for review April 5, 2024 21:56
@r10s r10s requested review from adbenitez and zeitschlag April 6, 2024 10:13
@r10s r10s marked this pull request as draft April 10, 2024 22:48
@r10s
Copy link
Member Author

r10s commented Apr 10, 2024

back to draft as the core part is close to being ready and we can remove all the update-hacks here soon.

so, it makes sense to wait for review until that's done.

@r10s
Copy link
Member Author

r10s commented Apr 20, 2024

although maps for iOS will stay experimental for 1.46, we aim to get this in to move forward, to get bug reports etc. the UX improvement is huge - there is just no map before (we aimed to get that in even with the much more hacky swift-but-rust implementation :)

EDIT: deltachat/deltachat-core-rust#5461 is merged, i rebase this pr, so we can move forward :)

btw, this PR adds less than 40 LOC really needed to handle the map - the rest are buttons etc. without the help of core, we added 220 LOC - with fewer features

r10s added a commit to deltachat/deltachat-core-rust that referenced this pull request Apr 20, 2024
as discussed in several chats, this PR starts making it possible to use
Webxdc as integrations to the main app. In other word: selected parts of
the main app can be integrated as Webxdc, eg. Maps [^1]

this PR contains two parts:

- draft an Webxdc Integration API
- use the Webxdc Integration API to create a Maps Integration

to be clear: a Webxdc is not part of this PR. the PR is about marking a
Webxdc being used as a Map - and core then feeds the Webxdc with
location data. from the view of the Webxdc, the normal
`sendUpdate()`/`setUpdateListener()` is used.

things are still marked as "experimental", idea is to get that in to
allow @adbenitez and @nicodh to move forward on the integrations into
android and desktop, as well as improving the maps.xdc itself.
good news is that we currently can change the protocol between Webxdc
and core at any point :)


# Webxdc Integration API

see `dc_init_webxdc_integration()` in `deltachat.h` for overview and
documentation.

rust code is mostly in `webxdc/integration.rs` that is called by other
places as needed. current [user of the API is
deltachat-ios](deltachat/deltachat-ios#1912),
android/desktop will probably follow.

the jsonrpc part is missing and can come in another PR when things are
settled and desktop is really starting [^2] (so we won't need to do all
iterations twice :) makes also sense, when this is done by someone
actually trying that out on desktop

while the API is prepared to allow other types of integrations (photo
editor, compose tools ...) internally, we currently ignore the type. if
that gets more crazy, we probably also need a dedicated table for the
integrations and not just a single param.

# Maps Integration

rust code is mostly in `webxdc/maps_integration.rs` that is called by
`webxdc/integration.rs` as needed.

EDIT: the idea of having a split here, is that
`webxdc/maps_integration.rs` really can focus on the json part, on the
communication with the .xdc, including tests

this PR is basic implementation, enabling to move forward on
integrations on iOS, but also on desktop and android.

the current implementation allows already the following:
- global and per-chat maps
- add and display POIs
- show positions and tracks of the last 24 hours

the current maps.xdc uses leaflet, and is in some regards better than
the current android/desktop implementations (much faster, show age of
positions, fade out positions, always show names of POIs, clearer UI).
however, we are also not bound to leaflet, it can be anything

> [**screenshots of the current
state**](deltachat/deltachat-ios#1912)
> 👆

to move forward faster and to keep this PR small, the following will go
to a subsequent PR:

- consider allowing webxdc to use a different timewindow for the
location
- delete POIs
- jsonrpc 


[^1]: maps are a good example as anyways barely native (see android
app), did cause a lot of pain on many levels in the past (technically,
bureaucratically), and have a comparable simple api
[^2]: only going for jsonrpc would only make sense if large parts of
android/ios would use jsonrpc, we're not there

---------

Co-authored-by: link2xt <[email protected]>
r10s added 2 commits April 20, 2024 18:33
when experimental 'location streaming' is enabled
the map is available from profiles and from 'all media',
showing positions, POIs and recent tracks
Copy link
Collaborator

@zeitschlag zeitschlag left a comment

Choose a reason for hiding this comment

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

LGTM and does what it says! I added some questions here and there, but nothing blocking. Thanks again!

@r10s r10s merged commit d983304 into main Apr 21, 2024
1 check passed
@r10s r10s deleted the add-map branch April 21, 2024 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement actually in development, user visible enhancement
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

2 participants