Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 5 additions & 1 deletion locales/en/transit.json
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,9 @@
"CustomLayover": "Custom layover time (minutes)",
"ShowPaths": "Show paths",
"HidePaths": "Hide paths",
"WaypointMinZoom": "Minimum map zoom for path waypoints",
"WaypointMinZoomHelp": "Waypoints are drawn and can be added or removed from this zoom level upward (allowed range: 1–15, where 1 is the whole-world view and 15 is street-level). Lower values allow editing at a wider map view.",
"WaypointMinZoomRefreshHint": "You need to refresh the page for this change to apply on the map.",
Comment thread
kaligrafy marked this conversation as resolved.
"true": "Yes",
"false": "No",
"Generate": "Generate path",
Expand Down Expand Up @@ -519,7 +522,8 @@
"DefaultDecelerationIsRequired": "Deceleration is required.",
"DefaultDecelerationIsInvalid": "Deceleration is invalid.",
"DefaultDecelerationIsTooLow": "Deceleration is too low.",
"DefaultDecelerationIsTooHigh": "Deceleration is too high (uncomfortable)."
"DefaultDecelerationIsTooHigh": "Deceleration is too high (uncomfortable).",
"WaypointMinZoomOutOfRange": "Waypoint minimum zoom must be an integer from 1 to 15."
}
},
"transitRouting": {
Expand Down
6 changes: 5 additions & 1 deletion locales/fr/transit.json
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,9 @@
"CustomLayover": "Battement personnalisé (minutes)",
"ShowPaths": "Montrer les lignes et parcours",
"HidePaths": "Cacher les lignes et parcours",
"WaypointMinZoom": "Zoom minimal de la carte pour les points de repère de parcours",
"WaypointMinZoomHelp": "Les points de repère s'affichent et peuvent être ajoutés ou retirés à partir de ce niveau de zoom (plage permise : 1 à 15, où 1 correspond à la vue mondiale et 15 à la rue). Une valeur plus basse permet d'éditer à une vue plus large.",
"WaypointMinZoomRefreshHint": "Il faut actualiser la page pour que ce réglage s'applique sur la carte.",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Refresh hint contradicts runtime sync behavior.

The text states users must refresh the page (Il faut actualiser la page), but PathWaypointZoomSync and TransitionMainMap implement live synchronization. When the preference changes, the map layers update immediately via the MAP_UPDATE_LAYERS_MIN_ZOOM_EVENT. No page refresh is required.

📝 Suggested correction
-"WaypointMinZoomRefreshHint": "Il faut actualiser la page pour que ce réglage s'applique sur la carte."
+"WaypointMinZoomRefreshHint": "Les changements s'appliquent immédiatement sur la carte."
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"WaypointMinZoomRefreshHint": "Il faut actualiser la page pour que ce réglage s'applique sur la carte.",
"WaypointMinZoomRefreshHint": "Les changements s'appliquent immédiatement sur la carte.",
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@locales/fr/transit.json` at line 402, The current translation for
WaypointMinZoomRefreshHint incorrectly tells users to refresh the page; update
the French string to inform users that changing the preference applies
immediately (no refresh needed) because PathWaypointZoomSync and
TransitionMainMap perform live updates via the MAP_UPDATE_LAYERS_MIN_ZOOM_EVENT
so the map layers update in real time when the setting changes.

"true": "Oui",
"false": "Non",
"Generate": "Générer le parcours",
Expand Down Expand Up @@ -519,7 +522,8 @@
"DefaultDecelerationIsRequired": "La décélération st requise.",
"DefaultDecelerationIsInvalid": "La décélération est invalide.",
"DefaultDecelerationIsTooLow": "La décélération est trop faible.",
"DefaultDecelerationIsTooHigh": "La décélération est trop élevée (inconfortable)."
"DefaultDecelerationIsTooHigh": "La décélération est trop élevée (inconfortable).",
"WaypointMinZoomOutOfRange": "Le zoom minimal pour les points de repère doit être un entier entre 1 et 15."
}
},
"transitRouting": {
Expand Down
14 changes: 14 additions & 0 deletions packages/chaire-lib-common/src/config/Preferences.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,13 @@
const MIN_DEFAULT_DECELERATION = 0.3;
const MAX_DEFAULT_DECELERATION = 1.5;

/** Allowed range for `map.pathWaypointMinZoom` (map zoom level, inclusive) */
const MIN_WAYPOINT_MIN_ZOOM = 1;
const MAX_WAYPOINT_MIN_ZOOM = 15;

interface PreferencesModelWithIdAndData extends PreferencesModel {
id: string;
data: { [key: string]: any };

Check warning on line 38 in packages/chaire-lib-common/src/config/Preferences.ts

View workflow job for this annotation

GitHub Actions / code-lint

Unexpected any. Specify a different type
}

const prefChangeEvent = 'change';
Expand Down Expand Up @@ -185,6 +189,16 @@
this._isValid = false;
}
}
const waypointMinZoom = _get(this.attributes, 'map.pathWaypointMinZoom');
// Must be an integer in [MIN_WAYPOINT_MIN_ZOOM, MAX_WAYPOINT_MIN_ZOOM]; rejects NaN, decimals, strings.
if (
!Number.isInteger(waypointMinZoom) ||
waypointMinZoom < MIN_WAYPOINT_MIN_ZOOM ||
waypointMinZoom > MAX_WAYPOINT_MIN_ZOOM
) {
this._errors.push('transit:transitPath:errors:WaypointMinZoomOutOfRange');
this._isValid = false;
}
return this._isValid;
}

Expand Down Expand Up @@ -289,7 +303,7 @@
}

// Not implemented for Preferences, we should never delete the preferences object.
public delete(_socket: any): Promise<any> {

Check warning on line 306 in packages/chaire-lib-common/src/config/Preferences.ts

View workflow job for this annotation

GitHub Actions / code-lint

Unexpected any. Specify a different type

Check warning on line 306 in packages/chaire-lib-common/src/config/Preferences.ts

View workflow job for this annotation

GitHub Actions / code-lint

Unexpected any. Specify a different type
return new Promise((resolve) => {
resolve(null);
});
Expand Down Expand Up @@ -366,7 +380,7 @@
}

// TODO: type this:
public get(path: string, defaultValue: unknown = undefined): any {

Check warning on line 383 in packages/chaire-lib-common/src/config/Preferences.ts

View workflow job for this annotation

GitHub Actions / code-lint

Unexpected any. Specify a different type
return super.get(path, defaultValue);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,12 @@ stubEmitter.on('preferences.read', mockStubReadPreferences);

const originalPreferences = _cloneDeep(Preferences.attributes);

beforeEach(() => {
// Reset preferences to the default value
Preferences.setAttributes(_cloneDeep(originalPreferences));
beforeEach(async () => {
// Full reset via load (merge defaults + project + {}), same as a fresh session; update() cannot drop keys missing from the patch
mockStubReadPreferences.mockImplementationOnce((callback) => {
callback(Status.createOk({}));
});
await Preferences.load(stubEmitter);
jest.clearAllMocks();
fetchMock.doMock();
fetchMock.mockClear();
Expand Down Expand Up @@ -194,6 +197,54 @@ describe('map.basemapShortname preference', () => {
});
});

describe('map.pathWaypointMinZoom preference', () => {
const waypointZoomError = 'transit:transitPath:errors:WaypointMinZoomOutOfRange';

test('has 10 as default value', () => {
expect(Preferences.get('map.pathWaypointMinZoom')).toBe(10);
});

test.each(Array.from({ length: 15 }, (_, i) => ({ z: i + 1 })))(
'validate passes for pathWaypointMinZoom=$z',
({ z }) => {
Preferences.set('map.pathWaypointMinZoom', z);
expect(Preferences.validate()).toBe(true);
expect(Preferences.errors).not.toContain(waypointZoomError);
}
);

test('resetPathToDefault restores default zoom', () => {
Preferences.set('map.pathWaypointMinZoom', 3);
Preferences.resetPathToDefault('map.pathWaypointMinZoom');
expect(Preferences.get('map.pathWaypointMinZoom')).toBe(10);
expect(Preferences.validate()).toBe(true);
});

test.each<{ value: unknown; desc: string }>([
{ value: 0, desc: 'zero' },
{ value: 16, desc: 'above range' },
{ value: -1, desc: 'negative' },
{ value: null, desc: 'null' },
{ value: undefined, desc: 'undefined' },
{ value: '8', desc: 'string' },
{ value: Number.NaN, desc: 'NaN' },
{ value: 10.5, desc: 'decimal' }
])('validate fails for invalid pathWaypointMinZoom ($desc)', ({ value }) => {
Preferences.set('map.pathWaypointMinZoom', value);
expect(Preferences.validate()).toBe(false);
expect(Preferences.errors).toContain(waypointZoomError);
});

test('validate fails when pathWaypointMinZoom is missing from map', async () => {
const map = _cloneDeep(Preferences.get('map')) as Record<string, unknown>;
delete map['pathWaypointMinZoom'];
await Preferences.update({ map: map as PreferencesModel['map'] }, stubEmitter, false);
expect(Preferences.validate()).toBe(false);
expect(Preferences.errors).toContain(waypointZoomError);
});

});

test('Test get from default or project default', () => {
Preferences.set('defaultSection', 'foo');
expect(Preferences.getFromDefault('defaultSection')).toBe('agencies');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,14 @@
overlayOpacity: number;
/** Overlay color: 'black' or 'white' */
overlayColor: 'black' | 'white';
/** Min map zoom (1–15) for path waypoint layers / editing; not under transit.paths so Path defaults do not copy it */
pathWaypointMinZoom: number;
};
colorPicker: {
/** Hexadecimal strings of the various colors that should be available */
colors: string[];
};
[key: string]: any;

Check warning on line 53 in packages/chaire-lib-common/src/config/defaultPreferences.config.ts

View workflow job for this annotation

GitHub Actions / code-lint

Unexpected any. Specify a different type
}

// TODO: Type more fields
Expand All @@ -62,7 +64,8 @@
zoom: 10,
basemapShortname: 'osm',
overlayOpacity: 50,
overlayColor: 'black'
overlayColor: 'black',
pathWaypointMinZoom: 10
Comment thread
kaligrafy marked this conversation as resolved.
},
socketUploadChunkSize: 10240000,
defaultWalkingSpeedMetersPerSeconds: 5 / 3.6,
Expand Down
30 changes: 30 additions & 0 deletions packages/chaire-lib-frontend/src/services/map/MapLayerManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,36 @@ class MapLibreLayerManager {
getLayerConfig(layerName: string) {
return this._layersByName[layerName];
}

/**
* Updates a layer's minimum zoom on the map and in the stored spec (so re-add uses the new value).
*
* @param layerName MapLibre layer id
* @param minZoom Minimum zoom (inclusive)
*/
updateLayerMinZoom(layerName: string, minZoom: number) {
const entry = this._layersByName[layerName];
if (!entry) {
return;
}
const spec = entry.layer as LayerSpecification & { minzoom?: number; maxzoom?: number };
spec.minzoom = minZoom;
if (!this._map?.getLayer(layerName)) {
return;
}
const existingLayer = this._map.getLayer(layerName);
const maxZoom = existingLayer?.maxzoom ?? this._map.getMaxZoom();
this._map.setLayerZoomRange(layerName, minZoom, maxZoom);
}

/**
* Applies the same minimum zoom to several layers (e.g. path waypoint layers after preference change).
*/
updateLayersMinZoom(layerNames: string[], minZoom: number) {
for (let i = 0; i < layerNames.length; i++) {
this.updateLayerMinZoom(layerNames[i], minZoom);
}
}
}

export default MapLibreLayerManager;
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,61 @@
* License text available at https://opensource.org/licenses/MIT
*/
import React from 'react';

import _toString from 'lodash/toString';
import Collapsible from 'react-collapsible';
import { useTranslation } from 'react-i18next';
import PreferencesResetToDefaultButton from '../PreferencesResetToDefaultButton';
import InputWrapper from 'chaire-lib-frontend/lib/components/input/InputWrapper';
import InputStringFormatted from 'chaire-lib-frontend/lib/components/input/InputStringFormatted';
import PreferencesSectionProps from '../PreferencesSectionProps';
import { parseIntOrNull } from 'chaire-lib-common/lib/utils/MathUtils';
import {
WAYPOINT_MIN_ZOOM_ALLOWED_MAX,
WAYPOINT_MIN_ZOOM_ALLOWED_MIN,
WAYPOINT_MIN_ZOOM_DEFAULT
} from '../../../../config/layers.config';

const PreferencesSectionTransitPaths: React.FunctionComponent<PreferencesSectionProps> = (props) => {
const { t } = useTranslation(['main', 'transit']);
const prefs = props.preferences.attributes;

// TODO: Implement this component or delete it.
const PreferencesSectionTransitPaths: React.FunctionComponent<PreferencesSectionProps> = (
_props: PreferencesSectionProps
) => {
return null;
return (
<Collapsible trigger={t('transit:transitPath:Paths')} open={true} transitionTime={100}>
<div className="tr__form-section">
<InputWrapper twoColumns={true} key="waypointMinZoom" label={t('transit:transitPath:WaypointMinZoom')}>
<InputStringFormatted
id="formFieldPreferencesTransitPathWaypointMinZoom"
value={prefs.map?.pathWaypointMinZoom ?? WAYPOINT_MIN_ZOOM_DEFAULT}
onValueUpdated={(payload) => {
// HTML min/max sets valid:false without updating prefs, so validate() never runs.
// Always forward so Preferences.validate() can set errors (see FormErrors at bottom of panel).
if (payload.valid === false) {
props.onValueChange('map.pathWaypointMinZoom', {
value: payload.value,
valid: true
});
} else {
props.onValueChange('map.pathWaypointMinZoom', payload);
}
}}
key={`formFieldPreferencesTransitPathWaypointMinZoom_${props.resetChangesCount}`}
stringToValue={parseIntOrNull}
valueToString={(val) => _toString(parseIntOrNull(val))}
type="number"
min={WAYPOINT_MIN_ZOOM_ALLOWED_MIN}
max={WAYPOINT_MIN_ZOOM_ALLOWED_MAX}
/>
<PreferencesResetToDefaultButton
resetPrefToDefault={props.resetPrefToDefault}
path="map.pathWaypointMinZoom"
preferences={props.preferences}
/>
</InputWrapper>
<p className="apptr__form-help-text">{t('transit:transitPath:WaypointMinZoomHelp')}</p>
<p className="apptr__form-help-text">{t('transit:transitPath:WaypointMinZoomRefreshHint')}</p>
</div>
</Collapsible>
);
};

export default PreferencesSectionTransitPaths;
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ import { MapUpdateLayerEventType } from 'chaire-lib-frontend/lib/services/map/ev

// Local workspace imports
import layersConfig, { sectionLayers, overlaySource } from '../../config/layers.config';
import pathWaypointZoomSync, {
MAP_UPDATE_LAYERS_MIN_ZOOM_EVENT,
MapUpdateLayersMinZoomPayload
} from '../../services/map/PathWaypointZoomSync';
import { polygonSelectionService } from '../../services/map/PolygonSelectionService';
import transitionMapEvents from '../../services/map/events';
import mapCustomEvents from '../../services/map/events/MapRelatedCustomEvents';
Expand Down Expand Up @@ -207,11 +211,20 @@ class MainMap extends React.Component<MainMapProps & WithTranslation & PropsWith
this.layerManager.setMap(map);
this.popupManager.setMap(map);
this.layerManager.updateEnabledLayers(this.state.layers);
// The waypoint layers bake their minzoom at module-load time (layers.config),
// before user preferences are loaded asynchronously. Reapply it now that the
// map exists so the user's saved preference takes effect on first load.
pathWaypointZoomSync.applyNow(serviceLocator.eventManager);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Le commentaire dit "Trigger and initial generic min-zoom update", mais le nom de la variable n'a rien de générique. Renommer?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Aussi, as-tu vraiment besoin d,appeler le applyNow? Il semble que le layer.config va déjà chercher le min zoom depuis les préférences, donc ça devrait être ok? Tu n'as peut-être pas besoin du tout de cette fonction.


this.setState({ mapLoaded: true });
serviceLocator.eventManager.emit('map.loaded');
};

/** Generic handler: update the minzoom of the given map layers. */
updateLayersMinZoom = (payload: MapUpdateLayersMinZoomPayload) => {
this.layerManager.updateLayersMinZoom(payload.layerNames, payload.minZoom);
};

showPathsByAttribute = (attribute: string, value: any) => {
// attribute must be agency_id or line_id
if (attribute === 'agency_id') {
Expand Down Expand Up @@ -273,6 +286,8 @@ class MainMap extends React.Component<MainMapProps & WithTranslation & PropsWith
serviceLocator.eventManager.on('map.deleteSelectedNodes', this.deleteSelectedNodes);
serviceLocator.eventManager.on('map.deleteSelectedPolygon', this.onDeleteSelectedPolygon);
serviceLocator.eventManager.on('collection.update.nodes', this.onNodesUpdatedHandler);
serviceLocator.eventManager.on(MAP_UPDATE_LAYERS_MIN_ZOOM_EVENT, this.updateLayersMinZoom);
pathWaypointZoomSync.start(serviceLocator.eventManager);
};

onDeleteSelectedPolygon = () => {
Expand Down Expand Up @@ -318,6 +333,8 @@ class MainMap extends React.Component<MainMapProps & WithTranslation & PropsWith
serviceLocator.eventManager.off('map.deleteSelectedNodes', this.deleteSelectedNodes);
serviceLocator.eventManager.off('map.deleteSelectedPolygon', this.onDeleteSelectedPolygon);
serviceLocator.eventManager.off('collection.update.nodes', this.onNodesUpdatedHandler);
serviceLocator.eventManager.off(MAP_UPDATE_LAYERS_MIN_ZOOM_EVENT, this.updateLayersMinZoom);
pathWaypointZoomSync.stop();

// Remove map event listeners BEFORE react-map-gl cleans up
// We need to do this early because react-map-gl will call map.remove() automatically
Expand Down
35 changes: 30 additions & 5 deletions packages/transition-frontend/src/config/layers.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,33 @@
* License text available at https://opensource.org/licenses/MIT
*/

/** Minimum zoom level for waypoint visibility and operations */
export const WAYPOINT_MIN_ZOOM = 14;
import Preferences from 'chaire-lib-common/lib/config/Preferences';

/** Default and allowed bounds for `map.pathWaypointMinZoom` (must match preferences validation) */
export const WAYPOINT_MIN_ZOOM_DEFAULT = 10;
export const WAYPOINT_MIN_ZOOM_ALLOWED_MIN = 1;
export const WAYPOINT_MIN_ZOOM_ALLOWED_MAX = 15;

/** MapLibre layers whose `minzoom` follows the waypoint preference */
export const TRANSIT_PATH_WAYPOINT_MAP_LAYER_NAMES = [
'transitPathWaypoints',
'transitPathWaypointsSelected',
'transitPathWaypointsErrors'
] as const;

/**
* Minimum map zoom for waypoint visibility and add/remove/drag operations.
* Reads `map.pathWaypointMinZoom` and clamps to the allowed range; falls back to
* `WAYPOINT_MIN_ZOOM_DEFAULT` if missing or non-finite.
*/
export const getWaypointMinZoom = (): number => {
Comment thread
kaligrafy marked this conversation as resolved.
const raw = Preferences.get('map.pathWaypointMinZoom');
const n = typeof raw === 'number' ? raw : parseInt(String(raw), 10);
if (!Number.isFinite(n)) {
return WAYPOINT_MIN_ZOOM_DEFAULT;
}
return Math.min(WAYPOINT_MIN_ZOOM_ALLOWED_MAX, Math.max(WAYPOINT_MIN_ZOOM_ALLOWED_MIN, Math.round(n)));
};

// Define which layers should be visible for each section
export const sectionLayers = {
Expand Down Expand Up @@ -343,7 +368,7 @@ const layersConfig = {

transitPathWaypoints: {
type: 'circle',
minzoom: WAYPOINT_MIN_ZOOM,
minzoom: getWaypointMinZoom(),
paint: {
'circle-radius': {
base: 1,
Expand All @@ -369,7 +394,7 @@ const layersConfig = {

transitPathWaypointsSelected: {
type: 'circle',
minzoom: WAYPOINT_MIN_ZOOM,
minzoom: getWaypointMinZoom(),
paint: {
'circle-radius': {
base: 1,
Expand All @@ -395,7 +420,7 @@ const layersConfig = {

transitPathWaypointsErrors: {
type: 'circle',
minzoom: WAYPOINT_MIN_ZOOM,
minzoom: getWaypointMinZoom(),
paint: {
'circle-radius': {
base: 1,
Expand Down
Loading
Loading