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

More baselayers #224

Open
pietervdvn opened this issue Mar 26, 2024 · 3 comments
Open

More baselayers #224

pietervdvn opened this issue Mar 26, 2024 · 3 comments

Comments

@pietervdvn
Copy link
Contributor

Hi,

Because I didn't like the colour scheme of 'protomaps.light', I'm in the process of creating a new one - partly inspired by Mapnik/Osm-carto, partly by MapTiler. You can already browse it at https://dev.mapcomplete.org/cyclofix.html? (ignore the pins)

It uses the current hosted version of protomaps.

Would this be considered a nice addition for protomaps? If so, please point me where this theme should be put, I'll open a PR then.

@bdon
Copy link
Member

bdon commented Mar 26, 2024

In general, no, themes beyond the base 5 (light, dark, white, grayscale, black) we can't commit to maintaining indefinitely into the future.

The suggested theming approach is to create a set of named color variables like here https://github.com/protomaps/basemaps/blob/main/styles/src/themes.ts instead of writing a complete style, because that would let any color scheme take advantage of changes in the upstream style.

Otherwise you can suggest an improvement specific to what you don't like? Deriving things from OSM Carto is fine, but other styles will likely not be license-compatible with the styles in this repo.

@Edefritz
Copy link
Contributor

Thanks for your suggestions about creating a theme instead of meddling with the styles themselves.

But as far as I can see, there is no way to pass a custom theme to any of the exported functions of the package. They basically only allow passing a key to existing themes:

export default function (source: string, key: string): LayerSpecification[] {
  const theme = themes[key];
  return nolabels_layers(source, theme).concat(labels_layers(source, theme));
}

export function noLabels(source: string, key: string): LayerSpecification[] {
  const theme = themes[key];
  return nolabels_layers(source, theme);
}

export function labels(source: string, key: string): LayerSpecification[] {
  const theme = themes[key];
  return labels_layers(source, theme);
}

I guess to do that we need something like this?

export function layersWithTheme(source: string, theme: Theme): LayerSpecification[] {
  return nolabels_layers(source, theme).concat(labels_layers(source, theme));
}

I'm happy to provide a PR if necessary. Or am I missing something?

@bdon
Copy link
Member

bdon commented Apr 5, 2024

@Edefritz yes, that looks like the right approach. It doesn't express a good way to separate labels/no labels for custom themes, or a way to inherit parts of a theme without re-defining every single theme property, but we can address those issues later. Can you open a PR?

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

3 participants