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

Error when calling Path.setStyle before added to the map. Reopen #6516 #6662

Closed
NielsHolt opened this issue May 13, 2019 · 6 comments
Closed

Comments

@NielsHolt
Copy link
Contributor

Hi
Update L.Polyline's _pxBounds when their stroke weight changes was changed in #6516
In version 1.5 Path.setStyle now calls this._updateBounds() if stroke is set in options
But it fails it setStyle is called before the path is added to the map
this._updateBounds using _rawPxBounds witch isn't set before _project is called

I think that the fix is to call this._project() instead of this._updateBounds() in setStyle

@cherniavskii
Copy link
Sponsor Collaborator

Hi @NielsHolt
Can you fill our issue template?
Thanks.

@NielsHolt
Copy link
Contributor Author

  • I've looked at the documentation to make sure the behavior is documented and expected
  • I'm sure this is a Leaflet code issue, not an issue with my own code nor with the framework I'm using (Cordova, Ionic, Angular, React…)
  • I've searched through the issues to make sure it's not yet reported

Steps to reproduce
Steps to reproduce the behavior:

var polyline = L.polyline([[45.51, -122.68],[37.77, -122.43],[34.04, -118.2]]);
polyline.setStyle({color: 'red', weight: 4});
polyline.addTo(map);

Expected behavior
In version 1.4 the above code adds a red polyline to map

Current behavior
In version 1.5.1 it fails calling _updateBounds()

Environment

  • Leaflet version: 1.5.1
  • Browser (with version): Chrome
  • OS/Platform (with version): Windows

Additional context
HOW TO FIX IT:
In \src\layer\vector\Path.js line 104

setStyle: function (style) {
...
    if (this.options.stroke && style.hasOwnProperty('weight')) {
        this._updateBounds();
}

Replace with

    if (this.options.stroke && style.hasOwnProperty('weight')) {
        this._project();  //<-

Minimal example reproducing the issue

Please create an example using https://leafletjs.com/edit.html or any other jsfiddle-like site.

  • this example is as simple as possible
  • this example does not rely on any third party code

@IvanSanchez
Copy link
Member

@NielsHolt Please make a pull request with the proposed fix, it's easier to review & merge.

@NielsHolt
Copy link
Contributor Author

Hi @IvanSanchez
I have created a pull request that should fix both #6662 and #6664

@cherniavskii cherniavskii pinned this issue Nov 5, 2019
@cherniavskii cherniavskii unpinned this issue Nov 17, 2019
NielsHolt added a commit to FCOO/Leaflet that referenced this issue Dec 14, 2019
naedx added a commit to naedx/Leaflet that referenced this issue May 14, 2020
This is a copy of commit #19b0fb3cac7f6fbdae6c15a02dee9da824f1112b
nick-less added a commit to nick-less/umap that referenced this issue Jan 11, 2021
@fodor0205
Copy link
Contributor

The original description does not seem to be correct:

But it fails it setStyle is called before the path is added to the map

The described error only occurs with the following order:

  1. Polyline added to the map
  2. setStyle() called with weight

No error occurs the other way around: when setStyle is called before addTo().

@johnd0e
Copy link
Collaborator

johnd0e commented Jan 20, 2021

Steps to reproduce the behavior:

var polyline = L.polyline([[45.51, -122.68],[37.77, -122.43],[34.04, -118.2]]);
polyline.setStyle({color: 'red', weight: 4});
polyline.addTo(map);

Unable to reproduce.
#6941 (comment)

@johnd0e johnd0e closed this as completed Jan 20, 2021
mourner pushed a commit that referenced this issue Oct 29, 2021
* Fix issue #6662 and #6664

* Add unit test for Path weight set error

* Simplify Polyline weight set test case

* Alternative fix for setting weight after empty Polyline is added to the map

* Add unit test for setting weight after empty Polygon is added to the map

* Return when empty Polyline is being rendered

* More general formulation for test

* Minor refactoring to make tests more uniform

Co-authored-by: fodor0205 <fodor0205@gmail.com>
Co-authored-by: johndoe <johnd0e@mail.ua>
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

5 participants