Skip to content

Commit

Permalink
Merge pull request #749 from geoadmin/bug-PB-89-preview-geojson
Browse files Browse the repository at this point in the history
PB-89: Fix preview of geojson layers
  • Loading branch information
ltshb authored Apr 2, 2024
2 parents 4b4f876 + 3d09674 commit 7958d63
Show file tree
Hide file tree
Showing 5 changed files with 166 additions and 69 deletions.
10 changes: 9 additions & 1 deletion src/modules/menu/components/search/SearchResultCategory.vue
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,13 @@ const props = defineProps({
const { title, results } = toRefs(props)
const entries = ref(null)
const emit = defineEmits(['entrySelected', 'firstEntryReached', 'lastEntryReached'])
const emit = defineEmits([
'entrySelected',
'firstEntryReached',
'lastEntryReached',
'setPreview',
'clearPreview',
])
function onEntrySelected(entry) {
emit('entrySelected', entry)
Expand Down Expand Up @@ -49,6 +55,8 @@ defineExpose({ focusFirstEntry, focusLastEntry })
@entry-selected="onEntrySelected(entry)"
@first-entry-reached="emit('firstEntryReached')"
@last-entry-reached="emit('lastEntryReached')"
@set-preview="emit('setPreview', $event)"
@clear-preview="emit('clearPreview', $event)"
/>
</ul>
</div>
Expand Down
80 changes: 70 additions & 10 deletions src/modules/menu/components/search/SearchResultList.vue
Original file line number Diff line number Diff line change
@@ -1,21 +1,30 @@
<script setup>
import { computed, ref } from 'vue'
import { computed, ref, watch } from 'vue'
import { useI18n } from 'vue-i18n'
import { useStore } from 'vuex'
import { SearchResultTypes } from '@/api/search.api'
import SearchResultCategory from '@/modules/menu/components/search/SearchResultCategory.vue'
import debounce from '@/utils/debounce'
import log from '@/utils/logging'
const dispatcher = { dispatcher: 'SearchResultList.vue' }
const emit = defineEmits(['close', 'firstResultEntryReached'])
const store = useStore()
const i18n = useI18n()
const resultCategories = ref([])
const preview = ref(null)
const results = computed(() => store.state.search.results)
const hasDevSiteWarning = computed(() => store.getters.hasDevSiteWarning)
const isPhoneMode = computed(() => store.getters.isPhoneMode)
const previewLayer = computed(() => store.state.layers.previewLayer)
const previewedPinnedLocation = computed(() => store.state.map.previewedPinnedLocation)
const locationResults = computed(() =>
results.value.filter((result) => result.resultType === SearchResultTypes.LOCATION)
)
Expand Down Expand Up @@ -43,28 +52,77 @@ const categories = computed(() => {
]
})
watch(preview, (newPreview) => setPreviewDebounced(newPreview))
function focusFirstEntry() {
const firstCategoryWithResults = categories.value.find(
(category) => category.results.length > 0
)
resultCategories.value[categories.value.indexOf(firstCategoryWithResults)]?.focusFirstEntry()
const firstCategory = categories.value.findIndex((category) => category.results.length > 0)
if (firstCategory >= 0) {
resultCategories.value[firstCategory]?.focusFirstEntry()
}
}
function onFirstEntryReached(index) {
if (index === 0) {
const previousCategoryIndex = categories.value.findLastIndex(
(category, i) => i < index && category.results.length > 0
)
if (previousCategoryIndex < 0) {
emit('firstResultEntryReached')
} else if (index > 0) {
} else {
// jumping up to the previous category's last result
resultCategories.value[index - 1]?.focusLastEntry()
resultCategories.value[previousCategoryIndex]?.focusLastEntry()
}
}
function onLastEntryReached(index) {
if (index < resultCategories.value.length - 1) {
resultCategories.value[index + 1]?.focusFirstEntry()
const nextCategoryIndex = categories.value.findIndex(
(category, i) => i > index && category.results.length > 0
)
if (nextCategoryIndex > 0) {
resultCategories.value[nextCategoryIndex]?.focusFirstEntry()
}
}
function setPreview(entry) {
preview.value = entry
}
function clearPreview(entry) {
// only clear the preview if not another entry has been set
if (preview.value?.id === entry.id) {
preview.value = null
}
}
// We debounce the preview to avoid too many store dispatch
const PREVIEW_DEBOUNCING_DELAY = 50
const setPreviewDebounced = debounce((entry) => {
log.debug(`Set preview`, entry, previewLayer.value, previewedPinnedLocation.value)
if (!entry) {
if (previewLayer.value) {
store.dispatch('clearPreviewLayer', dispatcher)
}
if (previewedPinnedLocation.value) {
store.dispatch('setPreviewedPinnedLocation', { coordinates: null, ...dispatcher })
}
} else if (entry.resultType === SearchResultTypes.LAYER) {
store.dispatch('setPreviewLayer', {
layer: entry.layerId,
...dispatcher,
})
if (previewedPinnedLocation.value) {
store.dispatch('setPreviewedPinnedLocation', { coordinates: null, ...dispatcher })
}
} else if (entry.coordinate) {
store.dispatch('setPreviewedPinnedLocation', {
coordinates: entry.coordinate,
...dispatcher,
})
if (previewLayer.value) {
store.dispatch('clearPreviewLayer', dispatcher)
}
}
}, PREVIEW_DEBOUNCING_DELAY)
defineExpose({ focusFirstEntry })
</script>
Expand Down Expand Up @@ -93,6 +151,8 @@ defineExpose({ focusFirstEntry })
:data-cy="`search-results-${category.id}`"
@first-entry-reached="onFirstEntryReached(index)"
@last-entry-reached="onLastEntryReached(index)"
@set-preview="setPreview"
@clear-preview="clearPreview"
/>
</div>
</div>
Expand Down
38 changes: 12 additions & 26 deletions src/modules/menu/components/search/SearchResultListEntry.vue
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,13 @@ const props = defineProps({
},
})
const emits = defineEmits(['entrySelected', 'firstEntryReached', 'lastEntryReached'])
const emits = defineEmits([
'entrySelected',
'firstEntryReached',
'lastEntryReached',
'setPreview',
'clearPreview',
])
const { index, entry } = toRefs(props)
Expand All @@ -42,8 +48,8 @@ const layerName = computed(() => {
function selectItem() {
emits('entrySelected')
emits('clearPreview', entry)
store.dispatch('selectResultEntry', { entry: entry.value, ...dispatcher })
store.dispatch('clearPreviewLayer', dispatcher)
}
function goToFirst() {
Expand All @@ -70,28 +76,6 @@ function goToLast() {
item.value.parentElement.lastElementChild?.focus()
}
function startResultPreview() {
if (resultType.value === SearchResultTypes.LAYER) {
store.dispatch('setPreviewLayer', {
layer: entry.value.layerId,
...dispatcher,
})
} else if (entry.value.coordinate) {
store.dispatch('setPreviewedPinnedLocation', {
coordinates: entry.value.coordinate,
...dispatcher,
})
}
}
function stopResultPreview() {
if (resultType.value === SearchResultTypes.LAYER) {
store.dispatch('clearPreviewLayer', dispatcher)
} else {
store.dispatch('setPreviewedPinnedLocation', { coordinates: null, ...dispatcher })
}
}
defineExpose({
goToFirst,
goToLast,
Expand All @@ -109,8 +93,10 @@ defineExpose({
@keydown.home.prevent="goToFirst"
@keydown.end.prevent="goToLast"
@keyup.enter="selectItem"
@mouseenter="startResultPreview"
@mouseleave="stopResultPreview"
@mouseenter="emits('setPreview', entry)"
@mouseleave="emits('clearPreview', entry)"
@focusin="emits('setPreview', entry)"
@focusout="emits('clearPreview', entry)"
>
<TextSearchMarker
class="search-category-entry-main px-2 flex-grow-1"
Expand Down
105 changes: 74 additions & 31 deletions src/store/plugins/load-geojson-style-and-data.plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@ const dispatcher = { dispatcher: 'load-geojson-style-and-data.plugin' }

const intervalsByLayerId = {}

async function load(url) {
function load(url) {
const controller = new AbortController()
try {
return await axios.get(url)
return { response: axios.get(url, { signal: controller.signal }), controller }
} catch (error) {
log.error(`Error while loading URL ${url}`, error)
throw error
Expand All @@ -37,7 +38,7 @@ async function autoReloadData(store, geoJsonLayer) {
intervalsByLayerId[geoJsonLayer.id] = setInterval(async () => {
try {
store.dispatch('setShowLoadingBar', { loading: true, ...dispatcher })
const { data } = await load(geoJsonLayer.geoJsonUrl)
const { data } = await load(geoJsonLayer.geoJsonUrl).response
const layerCopy = geoJsonLayer.clone()
layerCopy.geoJsonData = data
// we update through the action updateLayers, so that if multiple copies of the same GeoJSON layer are present,
Expand All @@ -63,29 +64,62 @@ function clearAutoReload(layerId) {

/**
* @param {GeoAdminGeoJsonLayer} geoJsonLayer
* @returns {Promise<GeoAdminGeoJsonLayer>}
* @returns {{ controllers: [AbortController]; clone: Promise<GeoAdminGeoJsonLayer> }}
*/
async function loadDataAndStyle(geoJsonLayer) {
function loadDataAndStyle(geoJsonLayer) {
log.debug(`Loading data/style for added GeoJSON layer`, geoJsonLayer)
const clone = geoJsonLayer.clone()
try {
const [{ data: style }, { data }] = await Promise.all([
load(geoJsonLayer.styleUrl),
load(geoJsonLayer.geoJsonUrl),
])

// as the layer comes from the store (99.9% chances), we copy it before altering it
// (otherwise, Vuex raises an error)
clone.geoJsonData = data
clone.geoJsonStyle = style
clone.isLoading = false
return clone
} catch (error) {
log.error(`Error while fetching GeoJSON data/style for layer ${geoJsonLayer?.id}`, error)
clone.isLoading = false
clone.errorKey = 'loading_error_network_failure'
clone.hasError = true
return clone

const style = load(geoJsonLayer.styleUrl)
const data = load(geoJsonLayer.geoJsonUrl)

return {
controllers: [style.controller, data.controller],
clone: Promise.all([style.response, data.response])
.then(([{ data: style }, { data }]) => {
const clone = geoJsonLayer.clone()
// as the layer comes from the store (99.9% chances), we copy it before altering it
// (otherwise, Vuex raises an error)
clone.geoJsonData = data
clone.geoJsonStyle = style
clone.isLoading = false
return clone
})
.catch((error) => {
log.error(
`Error while fetching GeoJSON data/style for layer ${geoJsonLayer?.id}`,
error
)
const clone = geoJsonLayer.clone()
clone.isLoading = false
clone.errorKey = 'loading_error_network_failure'
clone.hasError = true
return clone
}),
}
}

let pendingPreviewLayer = null

async function loadAndUpdatePreviewLayer(store, layer) {
cancelLoadPreviewLayer()
log.debug(`Loading geojson data for preview layer ${layer.id}`)
store.dispatch('setShowLoadingBar', { loading: true, ...dispatcher })
const { clone, controllers } = loadDataAndStyle(layer)
pendingPreviewLayer = { controllers, layerId: layer.id }
const updatedLayer = await clone
// Before updating the preview layer we need to be sure that it as not been cleared meanwhile
store.dispatch('setShowLoadingBar', { loading: false, ...dispatcher })
if (store.state.layers.previewLayer) {
log.debug(`Updating geojson data for preview layer ${layer.id}`)
store.dispatch('setPreviewLayer', { layer: updatedLayer, ...dispatcher })
}
}

function cancelLoadPreviewLayer() {
if (pendingPreviewLayer) {
log.debug(`Abort pending loading of preview geojson layer ${pendingPreviewLayer.layerId}`)
pendingPreviewLayer.controllers.map((controller) => controller.abort())
pendingPreviewLayer = null
}
}

Expand Down Expand Up @@ -113,7 +147,7 @@ export default function loadGeojsonStyleAndData(store) {
const updatedLayers = await Promise.all(
geoJsonLayers
.filter((layer) => layer.isLoading)
.map((layer) => loadDataAndStyle(layer))
.map((layer) => loadDataAndStyle(layer).clone)
)
if (updatedLayers.length > 0) {
store.dispatch('updateLayers', { layers: updatedLayers, ...dispatcher })
Expand All @@ -129,17 +163,26 @@ export default function loadGeojsonStyleAndData(store) {
autoReloadData(store, layer)
})
}

if (mutation.type === 'addLayer') {
addLayersSubscriber([mutation.payload.layer])
}
if (mutation.type === 'setLayers') {
} else if (mutation.type === 'setLayers') {
addLayersSubscriber(mutation.payload.layers)
}
if (mutation.type === 'removeLayerWithId' && intervalsByLayerId[mutation.payload.layerId]) {
} else if (
mutation.type === 'setPreviewLayer' &&
mutation.payload.layer instanceof GeoAdminGeoJsonLayer &&
mutation.payload.layer.isLoading
) {
loadAndUpdatePreviewLayer(store, mutation.payload.layer)
} else if (mutation.type === 'setPreviewLayer' && mutation.payload.layer === null) {
cancelLoadPreviewLayer()
} else if (
mutation.type === 'removeLayerWithId' &&
intervalsByLayerId[mutation.payload.layerId]
) {
// when a layer is removed, if a matching interval is found, we clear it
clearAutoReload(mutation.payload.layerId)
}
if (mutation.type === 'removeLayerByIndex') {
} else if (mutation.type === 'removeLayerByIndex') {
// As we come after the work has been done,
// we cannot get the layer ID removed from the store from the mutation's payload.
// So we instead go through all intervals, and clear any that has no matching layer in the active layers
Expand Down
2 changes: 1 addition & 1 deletion tests/cypress/support/commands.js
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ Cypress.Commands.add(
* application startup phase and also after changing views that might disable click on the map like
* for example the drawing mode
*/
Cypress.Commands.add('waitMapIsReady', ({ timeout = 15000, olMap = true } = {}) => {
Cypress.Commands.add('waitMapIsReady', ({ timeout = 20000, olMap = true } = {}) => {
cy.waitUntilState((state) => state.app.isMapReady, { timeout: timeout })
// We also need to wait for the pointer event to be set
if (olMap) {
Expand Down

0 comments on commit 7958d63

Please sign in to comment.