-
Notifications
You must be signed in to change notification settings - Fork 142
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
A feature with point is shown as a circle #209
Comments
@mariusandra I don't know what you had in mind in this case but I think that a point should not trace anything but a dot. This may be used with |
Hey, seems like a bug. Any chance you'd be up for submitting a PR to fix it in some way? I'm not actively using this library myself, so never stumbled upon it, and need to find time to work on a fix. Happy to quickly merge a fix though. |
|
Ahoj @baldulin I finally figured out this is not a bug. A Point feature can be represented in any manner on a map as it is a single point. A circle is not a bad idea even if I prefer the idea of a Marker.
If you mean that I should edit my geojson this is actually what I did. But about any anyone that simply want to use pigeon-map to display a kml file that contains many points like my example
This is indeed the detail I took time to figure out, I think the doc could gain in saying that the
Well this is not that trivial because the features don't accept the same props and a Point isn't a LineString. I am thinking about using the styleCallback to send an alternate svg to display instead of the circle. That could make it more flexible without making pigeon heavier. |
Hmm, yeah the documentation is very basic. I mean there is this example here doing exactly that. But maybe it needs some improvement.
I guess I'll just have a go at this then. Cause the style callback is a bit of a messy way of changing the styling, and I think that that pattern won't work for changing the component. |
You are right, you can't change the component since it is nested into the geoJson. What you can do is draw something else that a circle in some cases. This is what I did in the PR #211 . You may want to have a look and comment. |
Okay yeah, obviously it's not my decision if that pr gets merged. But my opinion on it is that, firstly it works. Rereading your earlier post I should have probably commented more on if it is trivial to exchange the full component. The diff below shows how that could be done. p.s. diff --git a/demo/Demo.tsx b/demo/Demo.tsx
index b76ef33..043d16f 100644
--- a/demo/Demo.tsx
+++ b/demo/Demo.tsx
@@ -257,8 +257,21 @@ export function Demo(): JSX.Element {
}
return { fill: '#d4e6ec99', strokeWidth: '1', stroke: 'white', r: '20' }
}}
+ renderer={{
+ Point: ((props) => {
+ const [left, top] = props.latLngToPixel([props.coordinates[1], props.coordinates[0]])
+ return <g style={{ pointerEvents: 'auto' }} transform={`translate(${left} ${top})`}>
+ <path
+ d="M52 31.5C52 36.8395 49.18 42.314 45.0107 47.6094C40.8672 52.872 35.619 57.678 31.1763 61.6922C30.7916 62.0398 30.2084 62.0398 29.8237 61.6922C25.381 57.678 20.1328 52.872 15.9893 47.6094C11.82 42.314 9 36.8395 9 31.5C9 18.5709 18.6801 9 30.5 9C42.3199 9 52 18.5709 52 31.5Z"
+ fill="red"
+ stroke="white"
+ strokeWidth="4"
+ />
+ <circle cx="30.5" cy="30.5" r="8.5" fill="white" opacity={0.98} />
+ </g>
+ }) as React.FC,
+ }}
/>
<ZoomControl />
</Map>
</div>
<div>
diff --git a/src/overlays/GeoJson.tsx b/src/overlays/GeoJson.tsx
index a35d315..6e3da13 100644
--- a/src/overlays/GeoJson.tsx
+++ b/src/overlays/GeoJson.tsx
@@ -10,6 +10,7 @@ interface GeoJsonProps extends PigeonProps {
feature?: any
style?: CSSProperties
children?: React.ReactNode
+ renderer?: Map<string, React.ElementType>
// callbacks
onClick?: ({ event: HTMLMouseEvent, anchor: Point, payload: any }) => void
@@ -41,6 +42,7 @@ interface GeometryProps {
latLngToPixel?: (latLng: Point, center?: Point, zoom?: number) => Point
svgAttributes?: SVGProps<SVGElement>
geometry?: GeoJsonGeometry
+ renderer?: Map<string, React.ElementType>
}
const defaultSvgAttributes = { fill: '#93c0d099', strokeWidth: '2', stroke: 'white', r: '30' }
@@ -119,6 +121,7 @@ export function GeometryCollection(props: GeometryProps): JSX.Element {
MultiLineString,
Polygon,
MultiPolygon,
+ ...props.renderer
}
const { type, coordinates, geometries } = props.geometry |
@baldulin, My PR got merged, but it didn't close this issue. This is good because I wanted to discuss your last comment. Two things, the issue I reported is in fact not a bug and I just found out how to avoid my problem, just my removing Points from my geojson tracks. Nevertheless, I thought this could be a nice option to display points as a custom path rather than a circle. I don't think users would want to change the style of other GeoJson elements are quite meaningful as they are. Thanks for the code, this is a nice way to test pigeon-maps without having to install it in another project. Unfortunately, I can't make it work as is. I also have a question, since it looks like you want to display the circle at the same time as the path. Do you think users would wish that? Also I see you added the tags around the path in your renderer. This could be nice to use an SVG group for the Point. I felt this wasn't possible given how is designed GeoJson component, but do you think this could be feasible? |
Nice, Congratz!
Yeah like I said, it work's and that option should be there. Like you say it's just the point that can have arbitrary representations (I always thought that nobody would really need that point, because either you use a marker or some kind of clustering). The reason to extend all other elements, is not really because they need to. But just to have a generalized way of modifying components. This obviously has a lot of benefits (like Lifecycle stuff, reusability and so on). But it's also easier to document.
And yeah like you say you can use svg groups as well. You could add text or anything you want (but you are in the svg context already). Because the renderer here takes a component, it does not inject props into an existing component. It should already work. If you couldn't run it, than its not apparent, but that code basically renders the default pigeon marker in red, I just copied it.
I don't know what users wish to be quite honest. Like I said I used both because the default pigeon marker uses both. It's just again a repetition of "if a users wishes to change the look of a point they should supply their own component instead of making a prebuild component look like they want to."
Yeah I guess you having issues with In the end you have to ask yourself if your fine with it as it is right now. As long as you are happy this issue is fixed. I got more hesitant to fix anything here because I thought it more prudent to upgrade the project. |
The feature in the example https://pigeon-maps.js.org/docs/geo-json
renders as
This may be useful for some cases but I would not use it as the default behaviour to display a point. When a trace is shown by joining points, this gives an ugly line of circles.
The text was updated successfully, but these errors were encountered: