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

get current location for overview map center, put a marker there #1242

Merged
merged 4 commits into from
Dec 5, 2024

Conversation

stevecassidy
Copy link
Contributor

Get current location for overview map center

JIRA Ticket

None

Description

The overview map had a fixed center point somewhere in Africa. More useful to move it
to the current location.

Proposed Changes

  • get the current location when displaying the overview map, use it as the center point
  • add a marker (+) at the current location on the map

How to Test

Open a notebook with geo-locations (BSS) and view the Map. See the cross at your current location.
Move the map, click the circle 'center map' button and see it center the map on your location.

Checklist

  • I have confirmed all commits have been signed.
  • I have added JSDoc style comments to any new functions or classes.
  • Relevant documentation such as READMEs, guides, and class comments are updated.

@luke-mcfarlane-rocketlab
Copy link
Contributor

Have a strange issue where it worked once but doesn't seem to work every time

Copy link
Contributor

@PeterBaker0 PeterBaker0 left a comment

Choose a reason for hiding this comment

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

Haven't tested, just spying code.

type: 'Feature',
geometry: {
type: 'Point',
coordinates: map_center,
Copy link
Contributor

Choose a reason for hiding this comment

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

is this safe if data is undefined - what's the behaviour?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Have put a guard around this.

app/src/gui/components/notebook/overview_map.tsx Outdated Show resolved Hide resolved
@PeterBaker0 PeterBaker0 self-requested a review December 5, 2024 05:53
Copy link
Contributor

@PeterBaker0 PeterBaker0 left a comment

Choose a reason for hiding this comment

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

Works well after discussed changes re: race conditions.

Would recommend some refactoring when we visit it again to do with error conditions and not relying on react query isLoading to check if something is happening 'for the first time' - would prefer refs to track this if needed.

@stevecassidy stevecassidy merged commit f18be21 into main Dec 5, 2024
2 checks passed
@stevecassidy stevecassidy deleted the fix-overview-map-center branch December 5, 2024 07:49
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