charts: add weight property to operational points

[Akctarus]

No description provided.

[Yohh]

nice, but you have to update filterStops types in utils.ts

[Akctarus]

nice, but you have to update filterStops types in utils.ts

nice catch, fixed

[Yohh]

sorry, but be carrefull to everywhere this new type is used, the utils for the story miss some changes, as reticle.ts, steps.ts... you should grep every occurrence of this obejct/type use

[Akctarus]

sorry, but be carrefull to everywhere this new type is used, the utils for the story miss some changes, as reticle.ts, steps.ts... you should grep every occurrence of this obejct/type use

Done !

[emersion]

So this only adds the new property but doesn't use it anywhere, right? Is this still a TODO, or will this come in a future PR?

[Akctarus]

So this only adds the new property but doesn't use it anywhere, right? Is this still a TODO, or will this come in a future PR?

A future PR on the back side will add the “weight” field for OPs, here I'm preparing the GET and GEV for this new property.

PR has evolved and this reviewer is on vacation

[theocrsb]

I have a strange behavior with manchette-with-spacetimechart

[kmer2016]

Refactoring calcWaypointsToDisplay and filterStops proposal:

The functions calcWaypointsToDisplay and filterStops share similar logic. Both aim to:

• Sort elements by weight to prioritize the most important ones.
• Check if there is enough space between elements before displaying them.
• Reorder the selected elements based on their original position for final output.

This similarity suggests an opportunity to create a reusable function that handles the shared logic, making the code cleaner and easier to maintain.

Proposed Refactor

introduce a generic function called filterVisibleElements. This function handles:

• Sorting elements by weight.
• Calculating positions.
• Checking spacing constraints to avoid overlaps.
• Returning elements in the correct order for display.

type VisibilityFilterOptions<T> = {
  elements: T[];
  getPosition: (element: T) => number;
  getWeight: (element: T) => number | undefined;
  minSpace: number;
};

const filterVisibleElements = <T>(
  { elements, getPosition, getWeight, minSpace }: VisibilityFilterOptions<T>
): T[] => {
  const sortedElements = elements.toSorted((a, b) => (getWeight(b) ?? 0) - (getWeight(a) ?? 0));
  const displayedElements: { element: T; position: number }[] = [];

  for (const element of sortedElements) {
    const position = getPosition(element);

    const hasSpace = !displayedElements.some(
      (displayed) => Math.abs(position - displayed.position) < minSpace
    );

    if (hasSpace) {
      displayedElements.push({ element, position });
    }
  }

  return displayedElements.sort((a, b) => getPosition(a.element) - getPosition(b.element)).map(({ element }) => element);
};

Both calcWaypointsToDisplay and filterStops now use filterVisibleElements. Their logic is simplified to focus on the specific details of their contexts.

Refactored calcWaypointsToDisplay

export const calcWaypointsToDisplay = (
  waypoints: Waypoint[],
  { height, isProportional, yZoom }: WaypointsOptions
): InteractiveWaypoint[] => {
  if (!isProportional || waypoints.length === 0) {
    return waypoints.map((waypoint) => ({ ...waypoint, display: true }));
  }

  const totalDistance = calcTotalDistance(waypoints);
  const heightWithoutFinalWaypoint = getHeightWithoutLastWaypoint(height);
  const minSpace = BASE_WAYPOINT_HEIGHT / yZoom;

  const displayedWaypoints = filterVisibleElements({
    elements: waypoints,
    getPosition: (waypoint) => (waypoint.position / totalDistance) * heightWithoutFinalWaypoint,
    getWeight: (waypoint) => waypoint.weight,
    minSpace,
  });

  return displayedWaypoints.map((waypoint) => ({ ...waypoint, display: true }));
};

Refactored filterStops:

export const filterStops = (
  stops: LayerData<OperationalPoints>[],
  ratioX: number,
  width: number,
  maxPosition: number,
  minSpace = 8
): LayerData<OperationalPoints>[] => {
  const displayedStops = filterVisibleElements({
    elements: stops,
    getPosition: (stop) => positionToPosX(stop.position.start, maxPosition, width, ratioX),
    getWeight: (stop) => stop.value.weight,
    minSpace,
  });

  return displayedStops;
};

Suggestions for Naming

I also suggest renaming the functions for clarity:

• calcWaypointsToDisplay → getDisplayedWaypoints
• filterStops → getDisplayedStops

[theocrsb]

LGTM and tested
I think it will be possible to make a small refacto to specify the type of the array of elements in filterVisibleElements.

[kmer2016]

Very nice ! OK for me, juste small suggestion

[kmer2016]

Very nice ! OK for me, just small suggestion