Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ts/components/vehicleMarker): use ReactMarker for VehicleMarker Icon and Label #2847

Merged
merged 2 commits into from
Oct 11, 2024

Conversation

firestack
Copy link
Member

@firestack firestack commented Oct 10, 2024

(Note: PR Reviewable by commits)

This is the refactor of converting the Vehicle Markers to use ReactMarker which has many benefits, but a large one being that when a vehicle updates, the entire element is not recreated and replaced (which is likely better for both performance, but also for Devtools in browsers)

We could probably combine the label and vehicle icon, but that's out of scope for this PR.

The snapshot changes are not required, but to do so requires creating a custom DivIcon. They have no effect on the final rendering in browser from the tests I've done. If we do want to avoid this (and I do!) see #2848

@firestack firestack marked this pull request as ready for review October 10, 2024 15:50
@firestack firestack requested a review from a team as a code owner October 10, 2024 15:50
@firestack firestack force-pushed the kf/refactor/react-vehicle-icon branch 2 times, most recently from 1a93e56 to 22c9c30 Compare October 10, 2024 19:26
Copy link
Collaborator

@hannahpurcell hannahpurcell left a comment

Choose a reason for hiding this comment

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

Beautiful! Love the refactor and can't find a thing I'd change

@hannahpurcell
Copy link
Collaborator

Approval given, but kinda want to review the follow-up PR before merging if that's ok?

@firestack firestack changed the title feat(ts/components/vehicleMarker): use ReactMarker for VehickeMarker Icon and Label feat(ts/components/vehicleMarker): use ReactMarker for VehicleMarker Icon and Label Oct 11, 2024
@firestack firestack force-pushed the kf/refactor/react-vehicle-icon branch from 22c9c30 to e0b8c90 Compare October 11, 2024 18:59
`VehicleIcon` uses the `ref` to trigger `Leaflet` functions for popup control
@firestack firestack force-pushed the kf/refactor/react-vehicle-icon branch from e0b8c90 to a731a65 Compare October 11, 2024 19:20
@firestack firestack enabled auto-merge (squash) October 11, 2024 19:23
@firestack firestack merged commit 7cd7729 into main Oct 11, 2024
9 checks passed
@firestack firestack deleted the kf/refactor/react-vehicle-icon branch October 11, 2024 19:24
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.

2 participants