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

Feature/geolocation outside extent or boundary #166

Open
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

jedi-of-the-sea
Copy link
Contributor

Summary

user location is tracked even if user is not within boundaryLayer, but within map extent.

Instructions for local reproduction and review

I used the meldemichel client for the implementation. To simulate a moving target, I added

// let lastCoordinate: [number, number] = [564387.0212149565, 5992080.331721107] // Neumünster
// let lastCoordinate: [number, number] = [566990.45, 5933972.45] // Hamburg
let lastCoordinate: [number, number] = [356123.45, 5942345.67] // Kiel

function getNextCoordinate(): [number, number] {
  const offset = 0.1 // Versatzwert
  const newCoordinate: [number, number] = [
    lastCoordinate[0] + offset,
    lastCoordinate[1] + offset,
  ]
  lastCoordinate = newCoordinate
  return newCoordinate
}

on top of the actions and set transformedCoords to getNextCoordinate()

@jedi-of-the-sea
Copy link
Contributor Author

jedi-of-the-sea commented Aug 28, 2024

Now that I'm finished I'm not sure if this is what was desired, as the extent of the map might be bigger than the boundaryLayer, and the user will be shown in an area where no map is rendered, as the case with meldemichel. I decided to leave it like that because I interpret the original description of the ticket like that, and let you decide if I should adjust the code. One could always adjust the map extent accordingly I think. Also, the info toast is triggered the whole time a user is outside the boundaryLayer. That was intentional, as I thought of someone moving around and seeing live if they are within the boundary. But the toasts might be annoying if the user is to far away?

@jedi-of-the-sea jedi-of-the-sea self-assigned this Aug 28, 2024
@jedi-of-the-sea jedi-of-the-sea added the enhancement New feature or request label Aug 28, 2024
Copy link
Member

@warm-coolguy warm-coolguy left a comment

Choose a reason for hiding this comment

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

@@ -3,6 +3,7 @@
## unpublished

- Fix: Adjust documentation to properly describe optionality of configuration parameters.
- Feature: Position is now tracked when user is outside of the view extent.
Copy link
Member

Choose a reason for hiding this comment

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

Not true.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -12,8 +12,8 @@ either `true` or `false`. When a users denies the location tracking, the button

| fieldName | type | description |
| - | - | - |
| boundaryLayerId | string? | Id of a vector layer to restrict geolocation markers and zooms to. When geolocation outside of its features occurs, an information will be shown once and the feature is stopped. The map will wait at most 10s for the layer to load; should it not happen, the geolocation feature is turned off. |
| boundaryOnError | ('strict' \| 'permissive')? | If the boundary layer check does not work due to loading or configuration errors, style `'strict'` will disable the geolocation feature, and style `'permissive'` will act as if no boundaryLayerId was set. Defaults to `'permissive'`. |
| boundaryLayerId | string? | Id of a vector layer to restrict geolocation markers and zooms to. When geolocation outside of its features occurs, an information will be shown. When loading the boundary layer, the map will wait at most 10s; should it not happen, the geolocation feature is turned off. |
Copy link
Member

Choose a reason for hiding this comment

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

Please return to only showing the message once per "user is outside the boundary" state.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

| boundaryLayerId | string? | Id of a vector layer to restrict geolocation markers and zooms to. When geolocation outside of its features occurs, an information will be shown once and the feature is stopped. The map will wait at most 10s for the layer to load; should it not happen, the geolocation feature is turned off. |
| boundaryOnError | ('strict' \| 'permissive')? | If the boundary layer check does not work due to loading or configuration errors, style `'strict'` will disable the geolocation feature, and style `'permissive'` will act as if no boundaryLayerId was set. Defaults to `'permissive'`. |
| boundaryLayerId | string? | Id of a vector layer to restrict geolocation markers and zooms to. When geolocation outside of its features occurs, an information will be shown. When loading the boundary layer, the map will wait at most 10s; should it not happen, the geolocation feature is turned off. |
| boundaryOnError | ('strict' \| 'permissive')? | If the boundary layer check does not work due to loading or configuration errors, style `'strict'` will disable the geolocation feature, and style `'permissive'` will act as if no boundaryLayerId was set. Defaults to `'permissive'`. |
Copy link
Member

Choose a reason for hiding this comment

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

There's a random extra whitespace here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comment on lines 41 to 45
const zoomLevel = rootGetters.configuration?.geoLocation?.zoomLevel
if (zoomLevel !== undefined && zoomLevel !== null) {
return zoomLevel
}
return 7
Copy link
Member

Choose a reason for hiding this comment

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

Good catch. I'm pretty sure the nullish coalescing operator also works in our setup, so you may write it shorter as this:

Suggested change
const zoomLevel = rootGetters.configuration?.geoLocation?.zoomLevel
if (zoomLevel !== undefined && zoomLevel !== null) {
return zoomLevel
}
return 7
return rootGetters.configuration?.geoLocation?.zoomLevel ?? 7

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comment on lines 2 to 3
position: number[],
transformedCoords: number[]
Copy link
Member

Choose a reason for hiding this comment

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

The names are confusing to me, I'd prefer something like "oldPosition" and "newPosition". I know they're named like that in the other spot, but for a multi-purpose function this is oddly specific.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jedi-of-the-sea
Copy link
Contributor Author

@warm-coolguy I implemented your suggestions and tried to change the code so that the geolocation tool behaves differently:

  • when user is outside of boundary layer but inside map extent, tracking continues but the toast message is only triggered once and there is no zoom to the marker anymore.

It should work like that, but I have not tested this with every parameter settings yet, so feel free to wait with further reviews until I have done that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants