Skip to content

AB#730 Convert TileLayerContainer to a function component - #6065

Merged
Antiik91 merged 4 commits into
v3from
AB#730-part2
Oct 9, 2026
Merged

Antiik91 merged 4 commits into
v3from
AB#730-part2

Conversation

@vesameskanen

Copy link
Copy Markdown
Member

Proposed Changes

  • Convert TileLayerContainer from a class component to a function component, using the useLeaflet and useCurrentTime hooks
  • Fix the layer and its event parent never being removed on unmount
  • Fix the vehicle-popup className leaking to later popups
  • Remove unused/junk code (showSpinner, bogus popup options)
  • Simplify with optional chaining and shared parking helper
  • Keep popup content mounted while Leaflet fades the popup out, so the close cross no longer jumps on close
  • Rewrite unit tests for the function component

Pull Request Check List

  • A reasonable set of unit tests is included
  • Console does not show new warnings/errors
  • Changes are documented or they are self explanatory
  • This pull request does not have any merge conflicts
  • All existing tests pass in CI build

Review

  • Read and verify the code changes
  • Test the functionality by running the UI locally with all popular browsers available in your platform
  • Check that the implementation matches the design, when such one is defined in an issue in Azure Boards
  • Merge the pull request

vesameskanen and others added 4 commits October 6, 2026 14:25
Create the Leaflet GridLayer directly instead of extending react-leaflet's
GridLayer, and use useLeaflet/useConfigContext/useRouter instead of wrapper
components. The layer is now removed on unmount, and the popup class is
computed per render so 'vehicle-popup' no longer leaks to later popups.
Extract sendSelectionAnalytics and rewrite the unit tests accordingly.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use optional chaining, share the parking hub filtering between the click
handler and popup rendering, derive the vehicle popup flag from the target
layer, and read the current time with useCurrentTime instead of
withCurrentTime.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
# Conflicts:
#	app/component/map/tile-layer/TileLayerContainer.jsx
@Antiik91

Antiik91 commented Oct 9, 2026

Copy link
Copy Markdown
Member

Very strange behavior happened while testing:

Search route ie Tram 15 and select it. See the route line and rendered vehicle icons
Go back
Click trams in near you -> Map renders the Tram 15 line and the trams on that line.

This happens with every route I tested, couldn't verify the issue in production.

@hjvuor

hjvuor commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Very strange behavior happened while testing:

Search route ie Tram 15 and select it. See the route line and rendered vehicle icons Go back Click trams in near you -> Map renders the Tram 15 line and the trams on that line.

This happens with every route I tested, couldn't verify the issue in production.

I tested this cause it might've been my recent changes to the mapLayer context but couldnt reproduce (using this branch or v3), can you verify if this happens in v3/dev or just this branch

@Antiik91

Antiik91 commented Oct 9, 2026

Copy link
Copy Markdown
Member

NVM, it seemed a bug because of selected location and selected routes showed strange looking results, but when investigating further it works as intended.

I tested with 2 different routes and Near you (with specific location), but it works as intended. Just looked odd :D

@Antiik91
Antiik91 merged commit 257ad81 into v3 Oct 9, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants