Add stop layer to new Debug UI

[leonardehrenfried]

Summary

This adds a vector tile layer for displaying stops in the new debug client. It looks like this:

It does this by adding some model classes that make it easy to write the layer style code in Java. This is a little less error prone than writing the JSON "by hand". Still, the domain (vector tile rendering and styling) is complex so you have to familiarise yourself with how Mapbox/Maplibre renders the map in order to write your own.

Issue

#5330

Unit tests

✔️

Documentation

Javadoc.

[leonardehrenfried]

@testower I'm a TypeScript noob. Can you give me a hand at getting around the compiler errors?

[testower]

I don't understand the need to define the map style in Java, when MapLibre has the style defnition fully typed. Did you look at the "MapStyle" interface?

[codecov]

Codecov Report

Attention: 54 lines in your changes are missing coverage. Please review.

Comparison is base (75462f0) 67.47% compared to head (cf5d875) 67.55%.
Report is 11 commits behind head on dev-2.x.

Files	Patch %	Lines
.../vectortiles/GraphInspectorVectorTileResource.java	0.00%	24 Missing ⚠️
...lanner/inspector/vector/stop/StopLayerBuilder.java	0.00%	13 Missing ⚠️
...ntripplanner/apis/vectortiles/model/LayerType.java	0.00%	4 Missing ⚠️
...ripplanner/apis/vectortiles/model/LayerParams.java	0.00%	3 Missing ⚠️
...va/org/opentripplanner/framework/io/HttpUtils.java	0.00%	3 Missing ⚠️
...in/java/org/opentripplanner/apis/APIEndpoints.java	0.00%	2 Missing ⚠️
...nner/apis/vectortiles/model/LayerStyleBuilder.java	95.12%	1 Missing and 1 partial ⚠️
...entripplanner/apis/vectortiles/DebugStyleSpec.java	94.73%	1 Missing ⚠️
...pector/vector/stop/StopLocationPropertyMapper.java	85.71%	0 Missing and 1 partial ⚠️
...entripplanner/standalone/server/GrizzlyServer.java	0.00%	1 Missing ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             dev-2.x    #5602      +/-   ##
=============================================
+ Coverage      67.47%   67.55%   +0.08%     
- Complexity     16199    16221      +22     
=============================================
  Files           1862     1865       +3     
  Lines          71213    71204       -9     
  Branches        7406     7398       -8     
=============================================
+ Hits           48048    48102      +54     
+ Misses         20672    20608      -64     
- Partials        2493     2494       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[leonardehrenfried]

I don't understand the need to define the map style in Java, when MapLibre has the style defnition fully typed. Did you look at the "MapStyle" interface?

First of all I didn't know that MapStyle exists. Are you sure that it's fully typed? I can only see the following:

export interface ILayer {
    id: string;
    type: string;
    metadata?: any;
    source?: unknown;
    minzoom?: number;
    maxzoom?: number;
    filter?: any;
    layout?: {
        [property: string]: any;
    };
    paint?: {
        [property: string]: any;
    };
}

Secondly, I was a lot more comfortable writing Java and I suspect other devs will be too.

Thirdly, I wanted the code that generates the data and the code that generates the styling close to each other sharing some identifiers (like layer names). This means that refactoring is a little easier.

I'm happy to debate these points and find a way forward that suits everyone.

[testower]

First of all I didn't know that MapStyle exists. Are you sure that it's fully typed? I can only see the following:

Yes you'll find it in the 'react-map-gl' package:

    export interface Style {
        layers: AnyLayer[];
        sources: Sources;

        bearing?: number | undefined;
        center?: number[] | undefined;
        fog?: Fog | undefined;
        glyphs?: string | undefined;
        metadata?: any;
        name?: string | undefined;
        pitch?: number | undefined;
        light?: Light | undefined;
        sprite?: string | undefined;
        terrain?: TerrainSpecification | undefined;
        transition?: Transition | undefined;
        version: number;
        zoom?: number | undefined;
    }

Secondly, I was a lot more comfortable writing Java and I suspect other devs will be too.

Thirdly, I wanted the code that generates the data and the code that generates the styling close to each other sharing some identifiers (like layer names). This means that refactoring is a little easier.

I'm sure there's some tradeoff here. In my view, the styling definition is client/implementation-specific and should live close to the client. I don't think we should offload code to the Java server just because it's more convenient.

Also, exposing the MapLibre / Mapbox styles in the api suggests that they are then officially part of the api? If that should be the case, then I think it makes more sense, but not for convenience.

[leonardehrenfried]

Well, we are not building a style against a generic vector tile schema like https://openmaptiles.org/schema/ but styling some very domain-specific geometries.

Whether you consider that part of "the API" is up for debate.

Next week I'm back at the dev meeting. We can talk about that there or on Gitter.

[leonardehrenfried]

Sorry @testower I made a tiny one-character change at the same time you approved.

[testower]

🎉

[testower]

That's neat. A potential improvement could be to instead zoom in one level if you encounter a cluster? Not critical for me.

[leonardehrenfried]

I'm not sure if that would be a good or bad UX but I'm willing to try it.

[testower]

It's something I've seen in other map implementations at least.

[leonardehrenfried]

We agreed that we will implement in an upcoming PR.

[abyrd]

Built locally and tested. UI started with:

cd client-next
npm install
npm run dev

Stop layer is visible and functioning as expected on the map. One comment: click-away from the stop popup closes the popup (in line with many other UIs), but if that click is right on another stop, it only closes the first one and doesn't open the second. The ability to click several stops one after the other to inspect them could be useful (open/close/open/close).

[leonardehrenfried]

Stop layer is visible and functioning as expected on the map. One comment: click-away from the stop popup closes the popup (in line with many other UIs), but if that click is right on another stop, it only closes the first one and doesn't open the second. The ability to click several stops one after the other to inspect them could be useful (open/close/open/close).

I will take a look at this next time I touch the UI code.