Feature image marker #42

risinghero · 2016-07-08T11:40:45Z

Added advanced marker image support.
Basic usage:
image:"www/your-image.png"
Code supports raster formats (.png, .jpg) and vector (svg only - via 3-d party libraries).
Advanced usage:

image:{
    url: "www/your-image.svg", //for static images
    height: 50,
    width:50,
    data: "base64 encoded bitmap", //use only if you need dynamic raster image,
    svg: "svg image code" //use only if you need dynamic vector image
}

Fix hide callback in iOS

Brandl · 2016-08-02T18:46:05Z

@EddyVerbruggen can we get this merged?
(Mainly asking for the TS support)

cusspvz · 2016-10-10T16:19:19Z

@EddyVerbruggen is there a chance for you to review this PR? I think @Anothar did a nice job over here!

@Anothar, thanks for your awesome job over here! Just a tip from developer to developer, for further reviews, avoid to indent things even if they aren't indented as it should. It makes the reviewing much harder when the PR is not about it. I even would advice you to set back some indents and give attention to it later, probably after this merge.

risinghero · 2016-10-10T18:16:10Z

Thx @cusspvz ))

avoid to indent things even if they aren't indented as it should.

Yuh, you are right. I really tried not no change indenting, but IDE still formatted html files(
If I'll have time I'll return indenting in the weekend.
Hope @EddyVerbruggen will have time to review PR.

lbweb · 2016-10-14T06:42:19Z

Hi @Anothar!
In a separate pull request listed by @dagatsoin here he says:

"The Mapbox SDK is now iOS 3.2.0 and also fixes a memory leak which add 30-50mo each time the user show/hide the map"

I am having this exact issue! Can you explain how to update the IOS SDK sitting behind your fork to 3.20?

Awesome fork! Custom images work great!

risinghero · 2016-10-14T06:45:57Z

Hi @lbweb !

Awesome fork! Custom images work great!

Thanks!
I'll try to update it today or tomorrow.

pke · 2016-10-31T21:09:14Z

We cannot use this plugin without custom marker support. When will this be merged? What is the hold-up?

EddyVerbruggen · 2016-10-31T21:36:14Z

@pke Why can't you use the fork that implements it?

pke · 2016-10-31T22:06:35Z

Because using a fork is always more maintenance work to keep it in sync with the origin repo.
Why isn't that merged yet? Is there anything we can do to accelerate the review process?

EddyVerbruggen · 2016-10-31T22:13:05Z

The reason is the same as for anyone else working on anything.

And in this particular case it will clash with 2 other PR's, however if you can review it I'd be happy to hit 'merge'. Main concern usually is backward compatibility (folks upgrading should get the same behavior).

pke · 2016-11-12T22:14:11Z

src/android/Mapbox.java

@@ -168,7 +183,23 @@ public void run() {
            FrameLayout.LayoutParams params = new FrameLayout.LayoutParams(webViewWidth - left - right, webViewHeight - top - bottom);
            params.setMargins(left, top, right, bottom);
            mapView.setLayoutParams(params);
-
+			//@anothar we have to override back button or app will just close
+            mapView.setOnKeyListener(new View.OnKeyListener() {


What has that to do with adding marker icons? Did that slip in as an error in the PR?

Hi, @pke it is not error. But you are right is doesn't relate to image markers. Is fixes bug issue 26. To be more exact without this fix android plugin closes your app when you press back button on phone. I know it is not good to add this fix to this PR. But I wasn't sure when/if everything gets merged and needed this fix.

I'd go so far and say it doesn't relate to map in any way. This behaviour is top be decided in the app. And pressing the back button on Android should actually close the app. That's the default UX

And pressing the back button on Android should actually close the app.

You are right that your app should decide behaviour. The problem is that you can't define behaviour from your js code without this fix! Without this fix your app always closes and you can't override it. You can't show previous screen to user and you can't override it from js withous this fix. With this fix you can set behaviour to which you want from your js code. This was the real problem for me. Because my app contains many screens and map is not the first one. And when I pressed back button it just closed. i tried to override it but without any success. Later I understood that event is invoked and processed in map view while it should be delegated to webview to decide what to do. If not to do this then js events for back button are not invoked.

Sure u can. There is a Cordova backbutton event for that

So, what if you redirect this "onKey" to the webview event?

@cusspvz

Event is not delegated to parent container of mapview and webview. Thus your app doesn't force close.

Event is delegated to webview. WebView invokes js events to backbutton. Thus all cordova handling code for backbutton works correctly.
This problem exists because mapview is not part of webview. It's separate control. It won't delegate it events to js code because it knows nothing about js. That is why we have to do it manually or if your user will press hardware backbuttom while map is focused your app will unexpectedly shutdown(unexpectedly only for cordova).

This google maps plugin https://github.com/mapsplugin/cordova-plugin-googlemaps does seem to handle that differently, doesn't it? I see no key handling in there.

I can't say about google maps plugin anything. I haven't tested it. Maybe it solves this problem different way.
You are not sure that problem exists?

@Anothar I know those are separate Views and the others rather than webview do not know JS code. What I meant with "redirect" was to implement a cordova exec init callback on the plugin listening for "back" keys and emit the "back button" on the JS.

risinghero · 2016-11-14T18:39:57Z

I see. Why is it better to implement cordova callback? It is specific only for Android with hardware backbutton.
14 нояб. 2016 г. 21:59 пользователь José Moreira [email protected] написал:@cusspvz commented on this pull request.

In src/android/Mapbox.java:

@@ -168,7 +183,23 @@ public void run() {
FrameLayout.LayoutParams params = new FrameLayout.LayoutParams(webViewWidth - left - right, webViewHeight - top - bottom);
params.setMargins(left, top, right, bottom);

mapView.setLayoutParams(params);
      //@anothar we have to override back button or app will just close
       mapView.setOnKeyListener(new View.OnKeyListener() {

@Anothar I know those are separate Views and the others rather than webview do not know JS code. What I meant with "redirect" was to implement a cordova exec init callback on the plugin listening for "back" keys and emit the "back button" on the JS.

—You are receiving this because you were mentioned.Reply to this email directly, view it on GitHub, or mute the thread.

pke · 2016-11-14T20:06:47Z

It's better because Cordova stays in control of the back button that way. And by that the developer.
Cordova apps close when the back button is hit and onbackbutton is not catched. By just adding this plugin here this expected flow would be altered without giving the coder a way to customise it

cusspvz · 2016-11-14T22:22:22Z

@Anothar One brief example, imagine that I have an app with react-router, if someone enters into a route where it has only a map, at this time, it exits the app.

Considering your example, it would do nothing.

The desired behaviour would be a history.go(-1), which is the default behavior of a WebView.

On this I have the same opinion as @pke , the best would be to inject a KeyPress event on the WebView using a cordova.exec callback.

EDIT: @pke talked about onbackbutton, I don't know if it is specific for the Cordova, it is out of my background, but if it is, it should be implemented that way.

If you need any help from me to implement it, I could study a little bit an help you. :)

risinghero · 2016-11-15T09:50:10Z

@cusspvz @pke you are right.

If you need any help from me to implement it, I could study a little bit an help you. :)

Thx @cusspvz :) I'll do it myself on the weekend. And write here when it is done.

EDIT: @pke talked about onbackbutton, I don't know if it is specific for the Cordova, it is out of my background, but if it is, it should be implemented that way.

It is specific Cordova event for back button press.

cusspvz · 2016-11-28T14:49:10Z

@Anothar any updates on this?

I would love to see this landed.
If you need any help, I'm available.

cusspvz · 2016-12-28T23:22:22Z

@Anothar, I'm now a collaborator, and I would like to see this merged for the next minor release.

Is there a chance for us to divide the work you did on this branch on different PRs?

I'm sorry for asking you to do it, I understand why you've opted out to increase your branch with fixes, but as I'm using it on a project, I would love to merge all of them. 👍

You should see some conflicting as I've already merged some commits of this PR:

Added typescript definitions file Added typescript definitions file #64
Fix .hide callback issue on iOS Fix .hide callback issue on iOS #63

Thanks! :)

cusspvz · 2016-12-28T23:24:20Z

@EddyVerbruggen can we get this merged?
(Mainly asking for the TS support)

@Brandl, I've already merged TS support on #64. Please expect it to be on the next minor release.

risinghero · 2016-12-29T06:31:28Z

Hi, @cusspvz , as I see you already divided)
Sorry for delaying. I'll try hard to do required event till the end of the year (2 days left)).

@Anothar, I'm now a collaborator, and I would like to see this merged for the next minor release.

Great news!)

…pbox into features_image_marker

risinghero · 2016-12-30T13:06:46Z

Hi @cusspvz . I've implemented backbutton callback. If you can-please resolve conflicts. I'm a little bit frustrated with github merge interface. Hope this branch can be merged.

cusspvz · 2016-12-30T13:11:56Z

Hi @Anothar ! Awesome!!

If you can-please resolve conflicts. I'm a little bit frustrated with github merge interface.

Well, I don't use it neither, I'm used to the cli for the rebasing process.
Gonna be away from the computer the next hours, but I will do this asap.

Inspite, if you want to learn how to do manual rebases and pick/unpick commits, I can opt out for guiding you instead. 😄

risinghero · 2016-12-30T17:57:15Z

Hi @cusspvz

Inspite, if you want to learn how to do manual rebases and pick/unpick commits, I can opt out for guiding you instead. 😄

I'll remember and ask for your guiding one day (place for some serious smile).
I've merged using github. Although merging in it is really stupid-just editing the file, but works. But build somehow doesn't pass. Actually should it pass as I don't see build config in master?

cusspvz · 2016-12-30T22:46:23Z

@Anothar

I'll remember and ask for your guiding one day (place for some serious smile).

I was thinking the same about the smile, too fancy, mine was serious as well. :)

I've merged using github. Although merging in it is really stupid-just editing the file, but works

In this case, the best option is a pick/unpick rebase scenario, I've got your branch backed up and I will tear down it entirely per feature/effect. This will make the review and landing easier than having a huge PR discussion with multiple things.

I've added another PR (#67), concerning the back button handling (just changed a little bit the design, I've explained why, but I think it should land before the next minor release as well)

I'm excited with this getting closer to be merged. Really think it is a "must have"! :)

cusspvz · 2016-12-30T22:49:58Z

But build somehow doesn't pass. Actually should it pass as I don't see build config in master?

Forget the building result, it isn't configured yet. It was activated yesterday but i didn't landing the config yet. It fails because it is applying the default building pipeline which is a ruby build scenario.

risinghero · 2016-12-31T11:07:42Z

Forget the building result, it isn't configured yet. It was activated yesterday but i didn't landing the config yet. It fails because it is applying the default building pipeline which is a ruby build scenario.

Already forgot)

In this case, the best option is a pick/unpick rebase scenario, I've got your branch backed up and I will tear down it entirely per feature/effect. This will make the review and landing easier than having a huge PR discussion with multiple things.

Great! This really simplifies review)
Happy new Year (there should be smile with champagne - [github-make cool smiles-it's number one feature!])!

risinghero · 2018-11-20T06:06:01Z

Hi @cusspvz , @dagatsoin , @EddyVerbruggen please give me rights to merge this. I want to recheck and test everything and merge. This actually stops me from further collaborating into this project.

risinghero added 4 commits July 6, 2016 16:08

Added support for markers with custom image for android

337aed6

Set tab space to 2

7017828

Support for markers in iOS

13f2b6f

Fix spaces

730e5ad

risinghero changed the title ~~Features image marker~~ Feature image marker Jul 8, 2016

This was referenced Jul 8, 2016

[WIP] [Next version] #29

Closed

Adding Custom Markers #43

Open

risinghero added 2 commits July 14, 2016 17:35

Fix for back button on android

86c0967

Added typescript definitions file

f81e555

risinghero mentioned this pull request Jul 31, 2016

Using plugin with typescript #40

Closed

risinghero added 2 commits July 31, 2016 14:10

Removed header

57028d9

Telerik-Verified-Plugins#44

5ea17bd

Fix hide callback in iOS

Fix build on iOS

2c651aa

risinghero mentioned this pull request Oct 10, 2016

Add a back button in order to have fullscreen map #26

Open

lbweb mentioned this pull request Oct 14, 2016

How do you Update IOS SDK? #58

Open

pke reviewed Nov 12, 2016

View reviewed changes

cusspvz added this to the 1.3.0 milestone Dec 28, 2016

risinghero added 2 commits December 30, 2016 17:54

Added backbutton callback

72bd50f

Merge branch 'features_image_marker' of https://github.com/anothar/Ma…

79da5a9

…pbox into features_image_marker

Merge branch 'master' into features_image_marker

579229f

cusspvz mentioned this pull request Dec 30, 2016

Android backbutton handling issue #67

Open

dagatsoin mentioned this pull request Jun 7, 2017

Button for centering the map to our own geolocation #79

Open

Merge branch 'master' into features_image_marker

dd5febc

Feature image marker #42

Are you sure you want to change the base?

Feature image marker #42

Conversation

risinghero commented Jul 8, 2016

Brandl commented Aug 2, 2016

cusspvz commented Oct 10, 2016

risinghero commented Oct 10, 2016 • edited Loading

lbweb commented Oct 14, 2016

risinghero commented Oct 14, 2016

pke commented Oct 31, 2016

EddyVerbruggen commented Oct 31, 2016

pke commented Oct 31, 2016

EddyVerbruggen commented Oct 31, 2016

pke Nov 12, 2016

Choose a reason for hiding this comment

risinghero Nov 13, 2016 • edited Loading

Choose a reason for hiding this comment

pke Nov 13, 2016

Choose a reason for hiding this comment

risinghero Nov 13, 2016 • edited Loading

Choose a reason for hiding this comment

pke Nov 13, 2016

Choose a reason for hiding this comment

cusspvz Nov 14, 2016

Choose a reason for hiding this comment

risinghero Nov 14, 2016 • edited Loading

Choose a reason for hiding this comment

pke Nov 14, 2016

Choose a reason for hiding this comment

risinghero Nov 14, 2016

Choose a reason for hiding this comment

cusspvz Nov 14, 2016

Choose a reason for hiding this comment

risinghero commented Nov 14, 2016

mapView.setLayoutParams(params);

pke commented Nov 14, 2016

cusspvz commented Nov 14, 2016 • edited Loading

risinghero commented Nov 15, 2016

cusspvz commented Nov 28, 2016

cusspvz commented Dec 28, 2016

cusspvz commented Dec 28, 2016 • edited Loading

risinghero commented Dec 29, 2016 • edited Loading

risinghero commented Dec 30, 2016

cusspvz commented Dec 30, 2016

risinghero commented Dec 30, 2016 • edited Loading

cusspvz commented Dec 30, 2016

cusspvz commented Dec 30, 2016

risinghero commented Dec 31, 2016

risinghero commented Nov 20, 2018 • edited Loading

risinghero commented Oct 10, 2016 •

edited

Loading

risinghero Nov 13, 2016 •

edited

Loading

risinghero Nov 13, 2016 •

edited

Loading

risinghero Nov 14, 2016 •

edited

Loading

cusspvz commented Nov 14, 2016 •

edited

Loading

cusspvz commented Dec 28, 2016 •

edited

Loading

risinghero commented Dec 29, 2016 •

edited

Loading

risinghero commented Dec 30, 2016 •

edited

Loading

risinghero commented Nov 20, 2018 •

edited

Loading