Skip to content

fix: use hosted valhalla default_style.json (#345) #388

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.

Sign up for GitHub

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

Open
Guptaji-007 wants to merge 3 commits into
base: master
Choose a base branch
from
Open

fix: use hosted valhalla default_style.json (#345) #388

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a1f6b88
fix: use hosted valhalla default_style.json (#345)
Guptaji-007 Mar 20, 2026
643a377
test: mock hosted Valhalla layers instead of fetch response
Guptaji-007 Mar 20, 2026
3580394
Merge branch 'master' into fix/use-hosted-default-style-345
Guptaji-007 Mar 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 68 additions & 23 deletions src/components/tiles/valhalla-layers-toggle.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import type { LayerSpecification } from 'maplibre-gl';
import { ValhallaLayersToggle } from './valhalla-layers-toggle';
import {
VALHALLA_SOURCE_ID,
VALHALLA_LAYERS,
VALHALLA_LAYER_IDS,
VALHALLA_EDGES_LAYER_ID,
VALHALLA_NODES_LAYER_ID,
VALHALLA_SHORTCUTS_LAYER_ID,
Expand Down Expand Up @@ -64,10 +64,39 @@ describe('ValhallaLayersToggle', () => {
mockMap = createMockMap();
mockMapReady = true;
vi.clearAllMocks();
vi.stubGlobal(
'fetch',
vi.fn(async () => ({
ok: true,
json: async () => ({
layers: [
{
id: 'edges',
type: 'line',
source: 'valhalla',
'source-layer': 'edges',
},
{
id: 'shortcuts',
type: 'line',
source: 'valhalla',
'source-layer': 'shortcuts',
},
{
id: 'nodes',
type: 'circle',
source: 'valhalla',
'source-layer': 'nodes',
},
],
}),
}))
);
Copy link
Copy Markdown
Member

@nilsnolde nilsnolde Mar 20, 2026

Choose a reason for hiding this comment

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

hm, I have no idea about maplibre API, but this looks like style.json syntax. is this needed? I'd have imagine the URL is all it takes to configure the MVT source!

Copy link
Copy Markdown
Author

@Guptaji-007 Guptaji-007 Mar 20, 2026

Choose a reason for hiding this comment

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

The MVT source URL is enough to configure the vector tile source, but MapLibre still needs layer definitions (type, paint, layout, source-layer) to render the Valhalla overlays. In the current setup, the hosted default_style.json is being used as the source of those layer definitions. I’ve updated the test to mock the hosted-layer provider directly instead of stubbing fetch/raw style JSON inline.

Copy link
Copy Markdown
Author

@Guptaji-007 Guptaji-007 Mar 20, 2026

Choose a reason for hiding this comment

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

@nilsnolde please have a look at the latest commit.

});

afterEach(() => {
vi.restoreAllMocks();
vi.unstubAllGlobals();
});

describe('rendering', () => {
Expand Down Expand Up @@ -109,13 +138,15 @@ describe('ValhallaLayersToggle', () => {
const toggle = screen.getByRole('switch');
await user.click(toggle);

expect(mockMap.addSource).toHaveBeenCalledWith(
VALHALLA_SOURCE_ID,
expect.objectContaining({
type: 'vector',
tiles: expect.any(Array),
})
);
await waitFor(() => {
expect(mockMap.addSource).toHaveBeenCalledWith(
VALHALLA_SOURCE_ID,
expect.objectContaining({
type: 'vector',
tiles: expect.any(Array),
})
);
});
});

it('should add all layers when toggled on', async () => {
Expand All @@ -125,10 +156,18 @@ describe('ValhallaLayersToggle', () => {
const toggle = screen.getByRole('switch');
await user.click(toggle);

expect(mockMap.addLayer).toHaveBeenCalledTimes(VALHALLA_LAYERS.length);
for (const layer of VALHALLA_LAYERS) {
expect(mockMap.addLayer).toHaveBeenCalledWith(layer);
}
await waitFor(() => {
expect(mockMap.addLayer).toHaveBeenCalledTimes(
VALHALLA_LAYER_IDS.length
);
});

const addLayerIds = mockMap.addLayer.mock.calls.map(
(call: [{ id: string }]) => call[0].id
);
expect(addLayerIds).toContain(VALHALLA_EDGES_LAYER_ID);
expect(addLayerIds).toContain(VALHALLA_SHORTCUTS_LAYER_ID);
expect(addLayerIds).toContain(VALHALLA_NODES_LAYER_ID);
});

it('should remove all layers when toggled off', async () => {
Expand Down Expand Up @@ -180,9 +219,11 @@ describe('ValhallaLayersToggle', () => {
const toggle = screen.getByRole('switch');
await user.click(toggle);

expect(mockMap.addLayer).toHaveBeenCalledTimes(
VALHALLA_LAYERS.length - 1
);
await waitFor(() => {
expect(mockMap.addLayer).toHaveBeenCalledTimes(
VALHALLA_LAYER_IDS.length - 1
);
});
});

it('should update checked state when toggled', async () => {
Expand Down Expand Up @@ -283,9 +324,11 @@ describe('ValhallaLayersToggle', () => {
const toggle = screen.getByRole('switch');
await user.click(toggle);

expect(mockMap.addLayer).toHaveBeenCalledWith(
expect.objectContaining({ id: 'custom-valhalla-layer' })
);
await waitFor(() => {
expect(mockMap.addLayer).toHaveBeenCalledWith(
expect.objectContaining({ id: 'custom-valhalla-layer' })
);
});
});

it('should set visibility none for an invisible custom valhalla layer when re-added', async () => {
Expand All @@ -306,11 +349,13 @@ describe('ValhallaLayersToggle', () => {
const toggle = screen.getByRole('switch');
await user.click(toggle);

expect(mockMap.setLayoutProperty).toHaveBeenCalledWith(
'custom-hidden-layer',
'visibility',
'none'
);
await waitFor(() => {
expect(mockMap.setLayoutProperty).toHaveBeenCalledWith(
'custom-hidden-layer',
'visibility',
'none'
);
});
});

it('should not re-add a custom layer that uses a different source', async () => {
Expand Down
16 changes: 10 additions & 6 deletions src/components/tiles/valhalla-layers-toggle.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ import { Switch } from '@/components/ui/switch';
import { Label } from '@/components/ui/label';
import {
VALHALLA_SOURCE_ID,
VALHALLA_LAYERS,
VALHALLA_LAYER_IDS,
getValhallaLayers,
getValhallaSourceSpec,
} from './valhalla-layers';

Expand Down Expand Up @@ -38,7 +39,7 @@ export const ValhallaLayersToggle = ({
};
}, [mainMap]);

const handleToggle = (checked: boolean) => {
const handleToggle = async (checked: boolean) => {
if (!mainMap || !mapReady) return;

const map = mainMap.getMap();
Expand All @@ -48,11 +49,14 @@ export const ValhallaLayersToggle = ({
if (!map.getSource(VALHALLA_SOURCE_ID)) {
map.addSource(VALHALLA_SOURCE_ID, getValhallaSourceSpec());
}
for (const layer of VALHALLA_LAYERS) {

const valhallaLayers = await getValhallaLayers();
for (const layer of valhallaLayers) {
if (!map.getLayer(layer.id)) {
map.addLayer(layer);
}
}

for (const entry of customLayers) {
const layerSource =
'source' in entry.layer ? entry.layer.source : undefined;
Expand All @@ -71,9 +75,9 @@ export const ValhallaLayersToggle = ({
}
}
} else {
for (const layer of VALHALLA_LAYERS) {
if (map.getLayer(layer.id)) {
map.removeLayer(layer.id);
for (const layerId of VALHALLA_LAYER_IDS) {
if (map.getLayer(layerId)) {
map.removeLayer(layerId);
}
}

Expand Down
Loading
Loading