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

Paths ignore className changes #378

Closed
jahed opened this issue Sep 10, 2017 · 4 comments
Closed

Paths ignore className changes #378

jahed opened this issue Sep 10, 2017 · 4 comments

Comments

@jahed
Copy link

jahed commented Sep 10, 2017

Expected behavior

When changing the className prop of any Path elements (Polygon, Circle, etc.), the respective DOM element should update to reflect the change.

Actual behavior

On initial mount, Paths pickup the className. However if it's changed, the element's class isn't updated.

This is due to this issue which seems to be in limbo on Leaflet. Some seem to think it's not an issue, despite the setStyle method saying it updates all the Path options (which className is):
Leaflet/Leaflet#2662

So I would suggest implementing our own className update if nothing else is available.

Steps to reproduce

Here's a simple example where clicking the circle makes it red (using CSS):

https://www.webpackbin.com/bins/-KthIFTWFI0_pJjAR70x

@jahed
Copy link
Author

jahed commented Sep 10, 2017

I understand the previous response on this ticket:

#94

However, className is a very standard prop in React, so not supporting the same behaviour in React and going the route Leaflet has taken, IMO is extremely confusing and frustrating when debugging this library among other standard React components.

If the plan really is to not implement this, I suggest leaving a warning somewhere obvious in the documentation. Possibly under the Vector Layers section.

@jahed
Copy link
Author

jahed commented Sep 10, 2017

For anyone who's looking for a quick fix, just extend the individual components as needed. It may be flaky in certain circumstances, but so far it's worked fine for me:

import { Polygon as LeafletPolygon } from 'react-leaflet'
import { difference } from 'lodash'

export default class Polygon extends LeafletPolygon {
    updateLeafletElement(fromProps, toProps) {
        super.updateLeafletElement(fromProps, toProps)

        if (toProps.className !== fromProps.className) {
            const fromClasses = fromProps.className.split(' ')
            const toClasses = toProps.className.split(' ')
            this.leafletElement._path.classList.remove(...difference(fromClasses, toClasses))
            this.leafletElement._path.classList.add(...difference(toClasses, fromClasses))
        }
    }
}

@jahed jahed closed this as completed Sep 17, 2017
@AdrianGolda
Copy link

I don't know why this issue is closed I spent 3 hours trying to find a solution for this bug and I didn't. And to me this looks like a serious bug.

@jahed
Copy link
Author

jahed commented Apr 25, 2020

I closed it myself as there was no response and I no longer use react-leaflet so I won't know if it's still an issue. I wrote my own wrapper instead. It's really straightforward, especially now with React Hooks.

I suggest you make your own issue with up-to-date diagnostics since this is pretty old now.

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

2 participants