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

Marker gone in development mode #56

Closed
Bodata opened this issue Dec 20, 2016 · 18 comments
Closed

Marker gone in development mode #56

Bodata opened this issue Dec 20, 2016 · 18 comments

Comments

@Bodata
Copy link

Bodata commented Dec 20, 2016

I updated the leaflet gem from 0.7.7 to 1.0.2
In development my map markers are not found anymore.

The system does a get /assets/marker-icon-[lot of number].png%22marker-icon.png

I think it appends "%22marker-icon.png" it is a very strange filename

When i downgraded to 0.7.7 things worked ok again. When i updated to 1.0.2 markers where gone again !!

I am working on Window, perhaps this has something to do with it.

@MartinHinz
Copy link
Contributor

Had the same problem, I solved it in my fork originally by explicitly setting the L.Icon.Default.prototype.options for the markers with the respective assets paths to the icons in the vendor/assets/javascripts/leaflet.js.erb.

L.Icon.Default.prototype.options.iconUrl = '<%= asset_path 'marker-icon.png'%>';
L.Icon.Default.prototype.options.iconRetinaUrl = '<%= asset_path 'marker-icon-2x.png'%>';
L.Icon.Default.prototype.options.shadowUrl = '<%= asset_path 'marker-shadow.png'%>';

Additionally I had to make sure that L.Icon.Default.imagePath returns a whitespace, not an empty string:

L.Icon.Default.imagePath = (function () {
  return '';
}());

see my fork for full details:

https://github.com/MartinHinz/leaflet-rails/tree/leaflet1.0.1

Last commit.

Not sure if this will work in the current environment, but might give a clue in which direction bug fixing might be necessary...

@axyjo
Copy link
Owner

axyjo commented Dec 20, 2016

Hmm -- I remember testing this out specifically, and it seemed to work. Will investigate. Thanks for the reports @Bodata and @MartinHinz

@axyjo
Copy link
Owner

axyjo commented Dec 20, 2016

Just tested with Rails 5 and it seems to work in both development and production environments -- what version of Rails are you on @Bodata and @MartinHinz?

@MartinHinz
Copy link
Contributor

Used a Rails 5 environment to test my fork. But have not tested your last version yet, I have to admit. Linux system, btw.

@Bodata
Copy link
Author

Bodata commented Dec 21, 2016

I am on rails 4.2, working on windows.
Have you looked into my remark about the appending of a "%22marker-icon.png" string.
Is ascii 22 perhaps not seen as an and of string in my case ?
There should be some code changes i guess

If i should give extra output let me know, i could test if it behaves identical on linux.

@MartinHinz
Copy link
Contributor

Just tested in Rails 5 application (Ruby 2.3.1), confirm error. Same situation like I experienced before the last commit of my fork, so obviously the similar source of the error. What happens is:

Url of the marker is composed like this:

http://localhost:3000/assets/marker-icon-574c3a5cca85f4114085b6841596d62f00d7c892c7b03f28cbfa301deb1dc437.png")marker-icon.png'

Note that the marker-icon.png shows up twice in the path.

It seems, there is no L.Icon.Default.imagePath set, so in L.Icon.Default._getIconUrl the image path is taken from this._detectIconPath(), and L.Icon.prototype._getIconUrl.call(this, name) is appended. Your function from leaflet.js.erb is not called, it seems. I only made it work in my fork by using the original function, and 'pretend' and imagePath as beeing a whitespace, and than setting the L.Icon.Default.prototype.options.[iconUrl, iconRetinaUrl, shadowUrl] with the assets path in leaflet.js.erb. See commit MartinHinz@eb643f0

@MartinHinz
Copy link
Contributor

Might be easier to compare working solution:

@Bodata, @axyjo : Could you try my current fork of version 1.0.2:

https://github.com/MartinHinz/leaflet-rails/tree/fix_icon_paths

If it works for you, I might issue a pull request again.

@Bodata
Copy link
Author

Bodata commented Dec 21, 2016

I will try, but have to work on other things now.
Let you know if i have tested, but could take time !!

@axyjo
Copy link
Owner

axyjo commented Dec 21, 2016

Just to be clear, you're both including leaflet and not leaflet-src into your Javascript, right?

@Aschen
Copy link

Aschen commented Dec 21, 2016

Hi,

I had the same problem with Rails 5 on Windows.
I use @MartinHinz branch to fix it, just add the following line to your Gemfile :

gem 'leaflet-rails', git: "git://github.com/MartinHinz/leaflet-rails.git", branch: "fix_icon_paths"

@MartinHinz
Copy link
Contributor

Relevant part from the application.js of my application:

//= require jquery
//= require jquery_ujs
//= require dropzone.min
//= require filereader
//= require turbolinks
//= require jquery_nested_form
//= require leaflet.js
//= require_tree .

So it is leaflet.js, I suppose!?

@MartinHinz
Copy link
Contributor

MartinHinz commented Dec 21, 2016

Having now proof that it works also in other environments than mine, I will make this a pull request. See #57

@axyjo
Copy link
Owner

axyjo commented Dec 21, 2016

I'm still hesitant on merging in this approach -- I'd like to get to the root cause of this. I'll set up a windows installation to test it out. Until then, @MartinHinz seems like a viable approach.

@MartinHinz
Copy link
Contributor

MartinHinz commented Dec 22, 2016

The root cause seem to be this:

It seems that L.Marker's icon option is instantiated with the L.Icon.Default during initialisation:

L.Marker = L.Layer.extend({

	// @section
	// @aka Marker options
	options: {
		// @option icon: Icon = *
		// Icon class to use for rendering the marker. See [Icon documentation](#L.Icon) for details on how to customize the marker icon. If not specified, a new `L.Icon.Default` is used.
		icon: new L.Icon.Default(),
                …

So changing L.Icon.Default afterwards is not changing the instantiated object of the icon within Marker.

The canonical way to change the icons (and their paths) is according to the documentation in leaflet-src.js (Line 6242-43):

 * In order to customize the default icon, just change the properties of `L.Icon.Default.prototype.options`
 * (which is a set of `Icon options`).

That is what my snippet is doing essentially.

Keep in mind, that it is not a windows issue, since I am using (Arch) Linux, too.

@Bodata
Copy link
Author

Bodata commented Dec 26, 2016

Hi,

I confirm that i do not include leaflet-src. I agree the problem is not windows only.
I worked my way back to find where the bug originated.

#gem 'leaflet-rails', github: 'axyjo/leaflet-rails', ref: 'dfc8621f99d6187c051d19b31a979a75e702bd22' # does not work
gem 'leaflet-rails', github: 'axyjo/leaflet-rails', ref: '7eab9e5d01faa3fadce0650491c0c911ae36d0b9' # works ok

If i take the last working version and add the files of the next version, image and stylesheets directories can be updated.
As i try to update the the javascripts directory markers disappear.
The 0.7.7 version has leaflet.js.erb file
the 1.1.0rc has 3 files

  • leaflet.js.erb
  • leaflet-src.js.erb
  • leaflet-src.map
    This is getting to difficult to for me ... hope it helps you find you bug.

Regards,
Peter

@quixoticwillbt
Copy link

quixoticwillbt commented Jan 4, 2017

I'm having the same problem on OSX.
The following workaround seems to work for me.

 var icon = new L.Icon.Default();
       var options = {icon: icon};
       var marker = L.marker([lat, lng], options).addTo(map);

@pedantic-git
Copy link
Contributor

I've submitted an alternative fix for this problem in #67 that simply reruns the icon initialization code of L.Marker after the overrides have been applied. It works in my use case, and I'd be interested to know if it fixes this for other people too.

@axyjo
Copy link
Owner

axyjo commented Apr 21, 2017

Released a new version -- 1.0.3 should fix it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants