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
Open
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/plugins/GeoLocation/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.


## 1.3.1

Expand Down
4 changes: 2 additions & 2 deletions packages/plugins/GeoLocation/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

| 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.

| checkLocationInitially | boolean? | If `true` the location gets checked on page load. When `false` this can be triggered with a button. Defaults to `false`. |
| keepCentered | boolean? | If `true`, the map will re-center on the user on any position change. If `false`, only the first position will be centered on. Defaults to `false`. |
| renderType | 'iconMenu' \| 'independent'? | If nested in `IconMenu`, select 'iconMenu' to match styling. Defaults to 'independent'. |
Expand Down
48 changes: 22 additions & 26 deletions packages/plugins/GeoLocation/src/store/actions.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { PolarActionTree } from '@polar/lib-custom-types'
import { passesBoundaryCheck } from '@polar/lib-passes-boundary-check'
import VectorLayer from 'ol/layer/Vector'
import Point from 'ol/geom/Point'
import { Vector } from 'ol/source'
Expand All @@ -11,8 +10,10 @@ import Geolocation from 'ol/Geolocation.js'
import { transform as transformCoordinates } from 'ol/proj'
import Overlay from 'ol/Overlay'
import { getTooltip } from '@polar/lib-tooltip'
import { passesBoundaryCheck } from '@polar/lib-passes-boundary-check'
import { GeoLocationState, GeoLocationGetters } from '../types'
import geoLocationMarker from '../assets/geoLocationMarker'
import positionChanged from '../utils/positionChanged'

const actions: PolarActionTree<GeoLocationState, GeoLocationGetters> = {
setupModule({ getters, commit, dispatch }): void {
Expand Down Expand Up @@ -118,8 +119,8 @@ const actions: PolarActionTree<GeoLocationState, GeoLocationGetters> = {
async positioning({
rootGetters: { map, configuration },
getters: {
boundaryLayerId,
boundaryOnError,
boundaryLayerId,
geolocation,
configuredEpsg,
position,
Expand All @@ -132,38 +133,34 @@ const actions: PolarActionTree<GeoLocationState, GeoLocationGetters> = {
Proj.get('EPSG:4326') as Proj.Projection,
configuredEpsg
)

const boundaryCheckPassed =
typeof boundaryLayerId === 'string'
? await passesBoundaryCheck(map, boundaryLayerId, transformedCoords)
: containsCoordinate(
// NOTE: The fallback is the default value set by @masterportal/masterportalApi
configuration?.extent || [510000.0, 5850000.0, 625000.4, 6000000.0],
transformedCoords
)
const boundaryErrorOccurred = typeof boundaryCheckPassed === 'symbol'

if (
boundaryCheckPassed === false ||
(boundaryErrorOccurred && boundaryOnError !== 'permissive')
) {
dispatch('printPositioningFailed', boundaryErrorOccurred)
// if check initially breaks or user leaves boundary, turn off tracking
const coordinateInExtent = containsCoordinate(
// NOTE: The fallback is the default value set by @masterportal/masterportalApi
configuration?.extent || [510000.0, 5850000.0, 625000.4, 6000000.0],
transformedCoords
)
const boundaryCheckPassed = await passesBoundaryCheck(
map,
boundaryLayerId,
transformedCoords
)
const showBoundaryLayerError =
typeof boundaryCheckPassed === 'symbol' && boundaryOnError === 'strict'
if (!coordinateInExtent || showBoundaryLayerError) {
dispatch('printPositioningFailed', showBoundaryLayerError)
dispatch('untrack')
return
}

if (
position[0] !== transformedCoords[0] ||
position[1] !== transformedCoords[1]
) {
if (positionChanged(position, transformedCoords)) {
commit('setPosition', transformedCoords)
dispatch('addMarker', transformedCoords)
if (!boundaryCheckPassed) {
dispatch('printPositioningFailed', false)
}
}
},
printPositioningFailed(
{ dispatch, getters: { toastAction } },
boundaryErrorOccurred: string
boundaryErrorOccurred: boolean
) {
if (toastAction) {
const toast = boundaryErrorOccurred
Expand Down Expand Up @@ -213,7 +210,6 @@ const actions: PolarActionTree<GeoLocationState, GeoLocationGetters> = {
}),
})
)

if (keepCentered || !hadPosition) {
dispatch('zoomAndCenter')
}
Expand Down
6 changes: 5 additions & 1 deletion packages/plugins/GeoLocation/src/store/getters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,11 @@ const getters: PolarGetterTree<GeoLocationState, GeoLocationGetters> = {
return rootGetters.configuration?.geoLocation?.toastAction
},
zoomLevel: (_, __, ___, rootGetters): number => {
return rootGetters.configuration?.geoLocation?.zoomLevel || 7
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.

},
geoLocationMarkerLayer(_, __, ___, rootGetters) {
return rootGetters?.map
Expand Down
8 changes: 8 additions & 0 deletions packages/plugins/GeoLocation/src/utils/positionChanged.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
export default function positionChanged(
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.

): boolean {
return (
position[0] !== transformedCoords[0] || position[1] !== transformedCoords[1]
)
}
Loading