Skip to content

Conversation

@Duncan-Brain
Copy link
Collaborator

@Duncan-Brain Duncan-Brain commented Jan 8, 2026

Description

In addition to this old bug whose status is released. This also resolves a bug I saw this week where the upon login the point filters on map would appear not to work with clustered points. On refresh that bug would resolve.

This branch fixes the issue twice and does two things:

  • When setting markerClusterRef.current to a new MarkerCluster() it first removes the older cluster from the map, and cleans up any listeners before setting the new one. Previously it assumes that markerClusterRef.current was yet to be defined
  • It removes the unnecessary re-render of the assets when maxZoom is updated. This re-render was originally added for old sensor architecture . It is no longer necessary as we have removed old sensor architecture. I think it was because the upload sensors modal did not navigate away and back to maps like the other "add" locations. Rtk-query should cover us there when we update the add map drawer and add locations flow -- in any case this approach should not be used.
    • It also cleans it up identically the same as on LocationPicker

Jira link: LF-3068

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Passes test case
  • UI components visually reviewed on desktop view
  • UI components visually reviewed on mobile view
  • Other (please explain):

In addition to what is outline on the Jira ticket:

  1. Logout method
    a. logout and login to farm with clustered points
    b. navigate to map and filter that cluster
    c. observe number remains the same but zooming in shows items are correctly filtered
    d. refresh map, problem does not persist

  2. Add point location method
    a. add a new point location to an area that will cause a maxZoom difference (ocean is easiest)
    b. filter the cluster
    c. observe the number remains the same but zooming in shows items correctly filtered
    d. refresh on map, problem does not persist

Checklist:

  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • The precommit and linting ran successfully
  • I have added or updated language tags for text that's part of the UI
  • I have ordered translation keys alphabetically (optional: run pnpm i18n to help with this)
  • I have added the GNU General Public License to all new files

@Duncan-Brain Duncan-Brain requested review from a team as code owners January 8, 2026 20:46
@Duncan-Brain Duncan-Brain requested review from kathyavini and removed request for a team January 8, 2026 20:46
@Duncan-Brain Duncan-Brain added the bug Something isn't working label Jan 8, 2026
const [gMap, setGMap] = useState(null);
const [gMaps, setGMaps] = useState(null);
const [gMapBounds, setGMapBounds] = useState(null);
// Unused: Kept in for map debugging
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Wasn't sure about this.

setShowSuccessHeader(false);
};

useEffect(() => {
Copy link
Collaborator Author

@Duncan-Brain Duncan-Brain Jan 8, 2026

Choose a reason for hiding this comment

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

Calling drawAssets twice redraws the cluster twice. MaxZoom triggers this on login (moving from undefined to defined and adding a new point with a different maxZoom values (ocean is easiest) .

maxZoom,
) => {
// If clusterer is being replaced, remove the old one from the map and cleanup listeners
if (clustererRef?.current) {
Copy link
Collaborator Author

@Duncan-Brain Duncan-Brain Jan 8, 2026

Choose a reason for hiding this comment

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

If drawAssets() is ever called twice two clusters will be on the map. One would be responsive to filtering, and the other would not be.

@Duncan-Brain Duncan-Brain self-assigned this Jan 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants