Skip to content

Conversation

@jussi-sa
Copy link
Contributor

@jussi-sa jussi-sa commented Sep 17, 2025

Description

Summary

  • Adds support for creating Mapbox Standard styles (with imports) in addition to classic styles
  • Fixes filter generation for admin layer properties (maritime and disputed as strings)
  • Introduces style utility for filtering expanded Mapbox styles from responses

Changes

New Features

Standard Style Support: StyleBuilderTool now creates modern Mapbox Standard styles by default

  • Uses imports to inherit from mapbox://styles/mapbox/standard
  • Adds proper metadata including mapbox:uiParadigm: 'imports'
  • Automatically assigns slot property to layers for Standard styles
  • Includes SDK version support metadata

Style Type Options: Three base style options available:

  • standard (default): Modern style with imports and slots
  • classic: Traditional style with direct sources (being deprecated)
  • blank: Minimal style with basic sources, no imports

Bug Fixes

Admin Layer Filters: Fixed filter generation for country/state boundaries

  • Properties maritime and disputed are now correctly treated as strings ("true"/"false") not booleans
  • This fixes the issue where country boundaries weren't appearing in classic styles

Improvements

Response Size Optimization: Added filterExpandedMapboxStyles utility to remove expanded style data from API
responses

  • Reduces response size significantly when styles include Mapbox imports
  • Applied to CreateStyleTool, UpdateStyleTool, and RetrieveStyleTool

Enhanced Layer Configuration:

  • Support for custom slot assignment in Standard styles
  • Better handling of layer-specific filters and properties
  • Improved type safety with comprehensive MapboxStyle types

Testing

Added unit tests + test cases in the style builder tool doc


Checklist

  • Code has been tested locally
  • Unit tests have been added or updated
  • Documentation has been updated if needed

Additional Notes

@jussi-sa jussi-sa requested a review from a team as a code owner September 17, 2025 08:46
.optional()
.describe(
'Layer slot for Mapbox Standard styles. Controls layer stacking order. ' +
'Bottom: below most map features, Middle: between base and labels, Top: above all base map features (default)'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

zmofei
zmofei previously approved these changes Sep 19, 2025
Comment on lines 130 to 135
'streets-v11',
'light-v10',
'dark-v10',
'satellite-v9',
'satellite-streets-v11',
'outdoors-v11'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a newer version of those styles.
https://docs.mapbox.com/api/maps/styles/#classic-mapbox-styles

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed with new versions

// 'dark',
// 'satellite',
// 'outdoors'
// ].includes(baseStyle);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] Consider to remove the comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 removed

class: {
description: 'Waterway classification',
description: 'class field',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] if those description are only <key> field, it will not help with the context, consider to remove them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed these fields, indeed, they are not useful for context

waterway: {
iso_3166_1: {
description: 'iso_3166_1 field',
values: [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] The iso_3166_1 list could be the same for each layer, consider puting them in one place, and referring to it, instead of listing it duplicate times.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

made a solution so that there one shared constant for all - reducing a lot of lines

Comment on lines 57 to 74
sections.push('**LineString layers:**');
sections.push('- `road` - All roads, paths, railways');
sections.push('- `admin` - Administrative boundaries');
sections.push('- `waterway` - Rivers, streams, canals as lines');
sections.push('- `aeroway` - Airport runways and taxiways');
sections.push('- `structure` - Bridges, tunnels, fences');
sections.push('- `natural_label` - Natural feature label placement paths');
sections.push('');
sections.push('**Point layers:**');
sections.push('- `place_label` - City, state, country labels');
sections.push('- `poi_label` - Points of interest');
sections.push('- `airport_label` - Airport labels');
sections.push('- `transit_stop_label` - Transit stops');
sections.push('- `motorway_junction` - Highway exits');
sections.push('- `housenum_label` - House numbers');
sections.push('');
sections.push('## Layer Types and Properties');
sections.push('');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] Those push likes hard, consider a better way to add?

I don't have a better way yet, but just one example

sections.push([
  '**LineString layers:**',
  '- `road` - All roads, paths, railways',
  '- `admin` - Administrative boundaries',
  '- `waterway` - Rivers, streams, canals as lines',
  '- `aeroway` - Airport runways and taxiways',
  '- `structure` - Bridges, tunnels, fences',
  '- `natural_label` - Natural feature label placement paths',
  '',
  '**Point layers:**',
  '- `place_label` - City, state, country labels',
  '- `poi_label` - Points of interest',
  '- `airport_label` - Airport labels',
  '- `transit_stop_label` - Transit stops',
  '- `motorway_junction` - Highway exits',
  '- `housenum_label` - House numbers',
  '',
  '## Layer Types and Properties',
  ''
].join('\n'));

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, i've done something similar now - now there are less push statements, one per "section"

@jussi-sa jussi-sa merged commit dfc7756 into main Sep 19, 2025
1 check passed
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.

2 participants