chris-m92 / react-leaflet-custom-control

React Leaflet v3 custom control
MIT License
22 stars 6 forks source link

Wrapping a regular react component #1

Closed pcpatfire closed 2 years ago

pcpatfire commented 2 years ago

I created a regular react component which is regularly displaying a sidebar for a leaflet map. As it is not a react-leaflet component it is covering any other leaflet component I add to the map. I wanted to make it a react-leaflet custom component wrapping it as follows:

<Control position="topleft">
    <MapSidebar setView={searchSetView} />
</Control>

The MapSidebaris not showing up as before. I get only a gray line on the top left position where the control should be. Am I doing anything wrong? How is your <Control> supposed to be used?

chris-m92 commented 2 years ago

I may have to update the README, My use case is as follows

// Map
<MapComponent>
   <Sidebar />
</MapComponent>

// Sidebar Component
<Control position='topright'>
  <ButtonGroup>
  ...
  </ButtonGroup>
</Control>

You're right, though, because it isn't react-leaflet control and in essence places a React Portal into the leaflet-control leaflet-bar DOM component, there are some weird things that can happen.

You can also put in custom CSS styles via the style prop. I had to do that on one of mine because it in essence overlapped another one.

If you have an idea on how to improve / fix this, I'm all ears

davetapley commented 2 years ago

@chris-m92 I'm here because I'm trying to port this leaflet-sidebar library over to my react-leaflet app. I'd love to use react-leaflet-custom-control, but I see you set leaflet-bar here: https://github.com/chris-m92/react-leaflet-custom-control/blob/45007faa42accc1bc0f8c7a8b2b9e6cf731077d2/src/Control.tsx#L36

That brings in /* general toolbar styles */ which aren't wanted when trying to do a side bar as me and (I presume) @pcpatfire are.

Would you accept a PR to not set that directly, but instead have extra classes passed in?

chris-m92 commented 2 years ago

Yeah, I'm good with that go ahead and submit a pr and I can push to npm

davetapley commented 2 years ago

Also this would be more elegant if this were merged in ⬇️

See also:

chris-m92 commented 2 years ago

@davetapley , I'll take a look at that PR from leaflet, thanks for linking it. In the meantime, I'll merge your PR and push an NPM update here soon.

chris-m92 commented 2 years ago

@davetapley PR is merged and updated in npm @1.3.1. Thanks for looking into this and hopefully Paul can maybe support some more native solutions in the future. Like I said, I'll take a look to see how we can integrate some non-corner containers as well, because I use sidebars similar to your use case in my own projects.