Refactor map class components - #5986
Merged
Merged
Conversation
Preserves behavior: the leaflet Icon instance is still created once and subsequent icon prop updates are applied via icon.initialize(), previously done in componentDidUpdate. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replaces the legacy React context API (static contextTypes) with the useConfigContext() hook for reading the app config. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replaces the legacy React context API (static contextTypes) with the useConfigContext() hook, and componentDidMount/componentWillUnmount's zoomend listener with a useEffect. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replaces the legacy React context API (static contextTypes) with the useConfigContext() and useRouter() hooks. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replaces the legacy React context API (static contextTypes) with the useConfigContext() hook. componentDidMount and componentDidUpdate are mirrored with two useEffect calls (one with an empty dependency array for mount-only behavior, one running after every render to match the previous componentDidUpdate semantics). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replaces the legacy React context API (static contextTypes) with the useConfigContext() hook and react-intl's useIntl() hook. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replaces the legacy React context API (static contextTypes) with the useConfigContext() and useRouter() hooks. Also drops the unused getStore context type. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replaces the legacy React context API (static contextTypes) with the useConfigContext() hook, react-intl's useIntl(), and found's useRouter() for match. The map instance ref is now tracked with useRef instead of an instance property. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replaces the legacy React context API (static contextTypes) with the useConfigContext() hook and react-intl's useIntl(). The componentDidMount/componentWillUnmount zoomend listener is now a useEffect, and forceUpdate() is replaced with a useReducer-based re-render trigger. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replaces class state/instance fields with useState/useRef, and lifecycle methods with useEffect. The config context uses useConfigContext(), intl uses react-intl's useIntl(), and executeAction continues to use the legacy (props, context) API since no hook equivalent exists yet (matches the pattern already used in NearYouMap.jsx). setMWTRef now receives a plain object exposing enableMapTracking/disableMapTracking/forceRefresh instead of the class instance, matching how callers (RoutePageMap, ItineraryPage, NaviContainer) actually use the ref. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Defaulting a destructured parameter to undefined is a no-op (same as omitting the default), so drop these from the converted map function components. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
React 16 doesn't batch state updates outside of event handlers (e.g. inside a Promise .then() callback). Calling setLoading(false) and setLocation(...) as two separate updates caused an intermediate render with loading=false but location.address still undefined, crashing splitStringToAddressAndPlace(). Combining loading and location into a single state object (matching the original class's single setState call) fixes the race. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
setMWTRef was only called once on mount, handing the parent enableMapTracking/disableMapTracking/forceRefresh closures captured from the first render. Those closures kept referencing stale position/mapTrackingState values forever, which made geolocation tracking behave inconsistently for callers (NaviContainer, ItineraryPage, RoutePageMap) depending on when they invoked the ref. Fix: expose a stable object whose methods are reassigned to the latest closures on every render, so callers always get current behavior while the object identity handed to the parent never changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
vesameskanen
marked this pull request as draft
September 15, 2026 11:31
Pass undefined instead of null for the color prop so LegMarker's default 'currentColor' applies, instead of literally rendering --background-color: null in the inline style. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Only emit the SVG color attribute when colorOverride is set, instead of interpolating undefined/null directly, which previously produced an invalid literal color="undefined"/color="null" attribute. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Separate pure, Leaflet-independent logic from the Leaflet-specific rendering in LegMarker, StopMarker and TransitLegMarkers so it can be unit tested and reused if the map engine is ever changed: - LegMarker: extract getLegMarkerIconName, shouldDisplayLegRouteNumber and getLegRouteNumberHtml. - StopMarker: extract getModeIconSize, getModeIconClassName, getStopIconRadii, buildStopIconSvg and getStopIconClassName. - TransitLegMarkers: export the already-pure doMarkersOverlap, getArrowMarkerStyle and getSpeechBubbleStyle geometry helpers. No behavioral changes; the components still produce identical output. Added unit tests for all newly exported functions, including regression coverage for the null/undefined color handling fixed earlier on this branch. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The whole Leaflet map is rendered inside a `<div aria-hidden="true">` (Map.jsx), so nothing rendered within it is ever exposed to screen readers. LegMarker's sr-only span (and the redundant aria-hidden on its sibling) was therefore dead code left over from before that wrapper existed. Removed it and updated the corresponding unit test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
vesameskanen
marked this pull request as ready for review
September 23, 2026 08:29
Antiik91
requested changes
Sep 24, 2026
Co-authored-by: Janne Antikainen <antiik91@gmail.com>
Co-authored-by: Janne Antikainen <antiik91@gmail.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Only one class component left in map folder. Modernization was not possible without rewrite.