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 clicking divIcon with auto size #9253

Open
4 tasks done
ryantrust opened this issue Feb 9, 2024 · 1 comment
Open
4 tasks done

Error when clicking divIcon with auto size #9253

ryantrust opened this issue Feb 9, 2024 · 1 comment
Labels
bug needs triage Triage pending

Comments

@ryantrust
Copy link

Checklist

  • I've looked at the documentation to make sure the behavior isn't documented and expected.
  • I'm sure this is an issue with Leaflet, not with my app or other dependencies (Angular, Cordova, React, etc.).
  • I've searched through the current issues to make sure this hasn't been reported yet.
  • I agree to follow the Code of Conduct that this project adheres to.

Steps to reproduce

  • Create a marker with a divIcon with auto size (iconSize: 'auto' or iconSize: [auto, 100] or iconSize: [100, auto]).
  • Click on the marker
const map = L.map('map').setView([51.505, -0.09], 13);

const divIcon = L.divIcon({
	html: "DIV ICON",
	iconSize: [100, 'auto']
});

const marker = L.marker([51.5, -0.09], {
	icon: divIcon
}).addTo(map);

Expected behavior

Map should pan to the marker on click (technically on 'focus').

Current behavior

In Marker.js, _panOnFocus passes options.paddingBottomRight = [NaN, NaN] (or similar) to map.panInside.

This causes Error: Invalid LatLng object: (NaN, NaN).

Minimal example reproducing the issue

https://codepen.io/vtdn5C4Ibd1/pen/RwdYQaQ?editors=0011

Environment

  • Leaflet version: 1.9.4
  • Browser (with version): Firefox 122.0.1 (64-bit)
  • OS/Platform (with version): MacBook Pro, Apple M1 Pro, Sonoma 14.3 (23D56)
@ryantrust ryantrust added bug needs triage Triage pending labels Feb 9, 2024
@IvanSanchez
Copy link
Member

According to the docs, the iconSize option must be a L.Point - i.e. an array of two numbers. Usage of the string "auto" is neither documented nor expected.

At first sight, this is a case of GIGO.

I'm not keen on changing the behaviour of iconSize to allow for "auto" or any other CSS-related values. There's too much variability, and I'd rather not need to implement MutationObservers all around to keep track of sizes. The good old "put another larger DOM element inside the DivIcon with overflow: none" should work for these situations.

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

No branches or pull requests

2 participants