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

addGeoJSON #87

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

addGeoJSON #87

wants to merge 1 commit into from

Conversation

leoalho
Copy link

@leoalho leoalho commented Jun 11, 2023

Hi, I started thinking of #81 and have made a method addgeoJSON. The method accepts the following object as a parameter:
{geoJSON, lineOptions, markerOptions, polygonOptions}. geoJSON is a geoJSON object as specified in RFC 7946. The line-, marker- and polygonOptions are the same as specified otherwise but do not have the coordinates property. The addGeoJSON should accept all possible geoJSON types. Only the options that are present in the geoJSON are required (for instance a geoJSON with only linestrings do not need to have the markerOptions and polygonOptions). I have manually tested that the function shoul work as intended for all geoJSON types.
The main con that I see with my implementation is that the user an not stylize individual lines, markers, or polygons but instead all lines, markers and polygons have a shared style but I thought that this implementation is the most straightforward.

@leoalho leoalho closed this Jun 11, 2023
@leoalho leoalho reopened this Jun 11, 2023
@leoalho leoalho changed the title Fixed linting addGeoJSON Jun 11, 2023
@StephanGeorg
Copy link
Owner

Thanks @leoalho for the PR. Currently I'm very busy so it will take a bit to have a look at it. Could you @kuuup-at-work pls review this and give your feedback? Would be awesome. Thanks guys.

@kuuup-at-work
Copy link

Hi.
Thanks for your work @leoalho.
Your current version does not work for me. The rendered image is empty and there are no error messages. It's hard to tell if I made a mistake or if it doesn't work because of your changes because there are no automatic tests.

@leoalho
Copy link
Author

leoalho commented Jul 25, 2023

Hi, thank you both @kuuup-at-work and @StephanGeorg for commenting. I'll naturaly add the appropriate automatic tests once I've gotten green light that my implementation works as desires, which it does not seem to do at the moment.

Could you @kuuup-at-work want to share how you tried to render the map? And did was the render completely empty, ie. without any basemap either?

If it helps at all here is an example rendering and associated code that I did when manually testing a geoJSON object consisting of a "GeometryCollection" type. Please note in the code, that I used a relative path to the dist of my fork of staticmaps.

testi1690298601653

const StaticMaps = require("../staticmaps/dist/staticmaps");
const path = require("path");

const geoJSON = {
  type: "GeometryCollection",
  geometries: [
    {
      type: "MultiPolygon",
      coordinates: [
        [
          [125.6, 10.2],
          [125.7, 10.2],
          [125.7, 10.1],
          [125.6, 10.1],
          [125.6, 10.2],
        ],
        [
          [125.65, 10.19],
          [125.61, 10.11],
          [125.69, 10.11],
          [125.65, 10.19],
        ],
      ],
    },
    {
      type: "LineString",
      coordinates: [
        [125.6, 10.25],
        [125.7, 10.25],
      ],
    },
  ],
};

const lineOptions = {
  color: "#00ff00",
  width: 3,
};

const polygonOptions = {
  color: "#ff0000",
};

const render = async (zoom, center, geoJSONOptions) => {
  const options = {
    width: 600,
    height: 400,
  };
  const map = new StaticMaps(options);

  map.addGeoJSON(geoJSONOptions);

  await map.render(center, zoom);

  const fileName = `testi${Date.now()}`;
  await map.image.save(path.join("images", `${fileName}.png`));
};

render(10, [125.65, 10.1], {
  geoJSON: geoJSON,
  lineOptions: lineOptions,
  polygonOptions: polygonOptions,
});

@leoalho
Copy link
Author

leoalho commented Sep 24, 2023

Hi @kuuup-at-work, have you had the time to check my last comment?

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

Successfully merging this pull request may close these issues.

3 participants