From 4d7951ba69462db1f2d6d2ab4fd7dd907d579514 Mon Sep 17 00:00:00 2001 From: vshcherb Date: Thu, 3 Sep 2026 19:46:15 +0300 Subject: [PATCH 1/3] Clear POI overlay markers when all categories are unchecked MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The POI layer effect tested `... || move || isTypeChange` before the empty-categories branch. `move` is a moveend timestamp that stays truthy after the first map move, so the first branch always won, and the branch that removes the layer when the category list became empty was never reached — the orange POI markers stayed on the map until a page reload. Check for empty categories first, then the zoom/move/type-change refresh. Add selenium test search/100-poi-overlay-disable.mjs, which enables a POI category, then unchecks it and waits for the markers to disappear. It fails on the old code (waitByRemoved times out) and passes with the fix. Fixes https://github.com/osmandapp/web/issues/1945 Co-Authored-By: Claude Opus 5 --- map/src/map/layers/PoiLayer.js | 44 +++++++++---------- .../tests/search/100-poi-overlay-disable.mjs | 43 ++++++++++++++++++ 2 files changed, 64 insertions(+), 23 deletions(-) create mode 100644 tests/selenium/src/tests/search/100-poi-overlay-disable.mjs diff --git a/map/src/map/layers/PoiLayer.js b/map/src/map/layers/PoiLayer.js index 312a016d5..f2fc852b0 100644 --- a/map/src/map/layers/PoiLayer.js +++ b/map/src/map/layers/PoiLayer.js @@ -537,35 +537,33 @@ export default function PoiLayer() { !prevCategories || JSON.stringify(prevCategories.map((c) => c.category).sort()) !== JSON.stringify(ctx.showPoiCategories.map((c) => c.category).sort()); - if ((!isEmpty(ctx.showPoiCategories) && zoom !== prevZoom) || move || isTypeChange) { + if (isEmpty(ctx.showPoiCategories)) { + // if categories are cleared, then clear the list and related states + setPrevCategories(null); + if (poiList) { + clearPoiList(); + } + } else if (zoom !== prevZoom || move || isTypeChange) { if (prevController) { prevController.abort(); } setPrevController(controller); setPrevZoom(zoom); - if (ctx.showPoiCategories.length > 0) { - if (categoriesChanged) { - setPrevCategories(null); - } - reqIdRef.current += 1; - const runGetPoi = categoriesChanged || isTypeChange ? getPoiTask : debouncedGetPoi; - runGetPoi({ - controller, - ignore, - poiList: categoriesChanged ? null : poiList, - showPoiCategories: ctx.showPoiCategories, - poiIconCache: ctx.poiIconCache, - zoom, - reqId: reqIdRef.current, - visibleBboxInfo: getVisibleBboxInfo(ctx, map), - }); - } - } else if (isEmpty(ctx.showPoiCategories)) { - // if categories are cleared, then clear the list and related states - setPrevCategories(null); - if (poiList) { - clearPoiList(); + if (categoriesChanged) { + setPrevCategories(null); } + reqIdRef.current += 1; + const runGetPoi = categoriesChanged || isTypeChange ? getPoiTask : debouncedGetPoi; + runGetPoi({ + controller, + ignore, + poiList: categoriesChanged ? null : poiList, + showPoiCategories: ctx.showPoiCategories, + poiIconCache: ctx.poiIconCache, + zoom, + reqId: reqIdRef.current, + visibleBboxInfo: getVisibleBboxInfo(ctx, map), + }); } else if (poiList?.listFeatures?.features?.length > 0) { // same categories, no zoom/move change: re-render the existing pois without a new request const newLayer = await createPoiLayer({ diff --git a/tests/selenium/src/tests/search/100-poi-overlay-disable.mjs b/tests/selenium/src/tests/search/100-poi-overlay-disable.mjs new file mode 100644 index 000000000..b4f79967a --- /dev/null +++ b/tests/selenium/src/tests/search/100-poi-overlay-disable.mjs @@ -0,0 +1,43 @@ +import actionOpenMap from '../../actions/map/actionOpenMap.mjs'; +import actionLogIn from '../../actions/login/actionLogIn.mjs'; +import actionCheckPoi from '../../actions/map/actionCheckPoi.mjs'; +import actionFinish from '../../actions/actionFinish.mjs'; +import { clickBy, waitBy } from '../../lib.mjs'; +import { By } from 'selenium-webdriver'; + +export default async function test() { + await actionOpenMap(); + await actionLogIn(); + + const category = 'Cafe and restaurant'; + const iconWpt = 'amenity_restaurant'; + const poiName = 'Public Cafe'; + + await actionCheckPoi({ iconWpt, name: poiName, hidden: true }); + + // open configure map + await clickBy(By.id('se-show-menu-configuremap')); + await waitBy(By.id('se-configure-map-menu-name')); + + // enable POI overlay + await togglePoiCategory(category); + await actionCheckPoi({ iconWpt, name: poiName }); + + // disable POI overlay: markers must disappear without page reload + await togglePoiCategory(category); + await actionCheckPoi({ iconWpt, name: poiName, hidden: true }); + + await actionFinish(); +} + +// open POI categories from the opened configure map, switch the category over and apply +async function togglePoiCategory(category) { + await waitBy(By.id('se-configure-map-menu-poi-categories')); + await clickBy(By.id('se-configure-map-menu-poi-categories')); + + await waitBy(By.id(`se-poi-category-${category}`)); + await clickBy(By.id(`se-poi-category-${category}`)); + await clickBy(By.id('se-select-categories')); + + await waitBy(By.id('se-configure-map-menu-name')); +} From 9b794d7a86ea69b6f8890e30777ae53a0340b6c4 Mon Sep 17 00:00:00 2001 From: Kseniia Velychko Date: Mon, 7 Sep 2026 15:42:00 +0300 Subject: [PATCH 2/3] Return early on empty POI categories, merge overlay disable test into 91 --- map/src/map/layers/PoiLayer.js | 14 +++--- .../tests/search/100-poi-overlay-disable.mjs | 43 ------------------- .../src/tests/search/91-poi-markers-test.mjs | 18 ++++++-- 3 files changed, 22 insertions(+), 53 deletions(-) delete mode 100644 tests/selenium/src/tests/search/100-poi-overlay-disable.mjs diff --git a/map/src/map/layers/PoiLayer.js b/map/src/map/layers/PoiLayer.js index f2fc852b0..39bc1436b 100644 --- a/map/src/map/layers/PoiLayer.js +++ b/map/src/map/layers/PoiLayer.js @@ -532,18 +532,20 @@ export default function PoiLayer() { hideMarkersNearPin(map, ctx); async function getPoiList() { - const isTypeChange = typesChanged(); - const categoriesChanged = - !prevCategories || - JSON.stringify(prevCategories.map((c) => c.category).sort()) !== - JSON.stringify(ctx.showPoiCategories.map((c) => c.category).sort()); if (isEmpty(ctx.showPoiCategories)) { // if categories are cleared, then clear the list and related states setPrevCategories(null); if (poiList) { clearPoiList(); } - } else if (zoom !== prevZoom || move || isTypeChange) { + return; + } + const isTypeChange = typesChanged(); + const categoriesChanged = + !prevCategories || + JSON.stringify(prevCategories.map((c) => c.category).sort()) !== + JSON.stringify(ctx.showPoiCategories.map((c) => c.category).sort()); + if (zoom !== prevZoom || move || isTypeChange) { if (prevController) { prevController.abort(); } diff --git a/tests/selenium/src/tests/search/100-poi-overlay-disable.mjs b/tests/selenium/src/tests/search/100-poi-overlay-disable.mjs deleted file mode 100644 index b4f79967a..000000000 --- a/tests/selenium/src/tests/search/100-poi-overlay-disable.mjs +++ /dev/null @@ -1,43 +0,0 @@ -import actionOpenMap from '../../actions/map/actionOpenMap.mjs'; -import actionLogIn from '../../actions/login/actionLogIn.mjs'; -import actionCheckPoi from '../../actions/map/actionCheckPoi.mjs'; -import actionFinish from '../../actions/actionFinish.mjs'; -import { clickBy, waitBy } from '../../lib.mjs'; -import { By } from 'selenium-webdriver'; - -export default async function test() { - await actionOpenMap(); - await actionLogIn(); - - const category = 'Cafe and restaurant'; - const iconWpt = 'amenity_restaurant'; - const poiName = 'Public Cafe'; - - await actionCheckPoi({ iconWpt, name: poiName, hidden: true }); - - // open configure map - await clickBy(By.id('se-show-menu-configuremap')); - await waitBy(By.id('se-configure-map-menu-name')); - - // enable POI overlay - await togglePoiCategory(category); - await actionCheckPoi({ iconWpt, name: poiName }); - - // disable POI overlay: markers must disappear without page reload - await togglePoiCategory(category); - await actionCheckPoi({ iconWpt, name: poiName, hidden: true }); - - await actionFinish(); -} - -// open POI categories from the opened configure map, switch the category over and apply -async function togglePoiCategory(category) { - await waitBy(By.id('se-configure-map-menu-poi-categories')); - await clickBy(By.id('se-configure-map-menu-poi-categories')); - - await waitBy(By.id(`se-poi-category-${category}`)); - await clickBy(By.id(`se-poi-category-${category}`)); - await clickBy(By.id('se-select-categories')); - - await waitBy(By.id('se-configure-map-menu-name')); -} diff --git a/tests/selenium/src/tests/search/91-poi-markers-test.mjs b/tests/selenium/src/tests/search/91-poi-markers-test.mjs index fbbac277e..ceeab4821 100644 --- a/tests/selenium/src/tests/search/91-poi-markers-test.mjs +++ b/tests/selenium/src/tests/search/91-poi-markers-test.mjs @@ -19,7 +19,19 @@ export default async function test() { await clickBy(By.id('se-show-menu-configuremap')); await waitBy(By.id('se-configure-map-menu-name')); - // open POI categories + // enable POI overlay + await togglePoiCategory(category); + await actionCheckPoi({ iconWpt, name: poiName }); + + // disable POI overlay: markers must disappear without page reload + await togglePoiCategory(category); + await actionCheckPoi({ iconWpt, name: poiName, hidden: true }); + + await actionFinish(); +} + +// open POI categories from the opened configure map, switch the category over and apply +async function togglePoiCategory(category) { await waitBy(By.id('se-configure-map-menu-poi-categories')); await clickBy(By.id('se-configure-map-menu-poi-categories')); @@ -27,7 +39,5 @@ export default async function test() { await clickBy(By.id(`se-poi-category-${category}`)); await clickBy(By.id('se-select-categories')); - await actionCheckPoi({ iconWpt, name: poiName }); - - await actionFinish(); + await waitBy(By.id('se-configure-map-menu-name')); } From 2d7a55b2f8e734a6168c3e990e3b75e6d39e0a93 Mon Sep 17 00:00:00 2001 From: Kseniia Velychko Date: Mon, 7 Sep 2026 15:56:11 +0300 Subject: [PATCH 3/3] Invalidate pending POI requests when categories are cleared --- map/src/map/layers/PoiLayer.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/map/src/map/layers/PoiLayer.js b/map/src/map/layers/PoiLayer.js index 39bc1436b..0abcf63c6 100644 --- a/map/src/map/layers/PoiLayer.js +++ b/map/src/map/layers/PoiLayer.js @@ -467,7 +467,6 @@ export default function PoiLayer() { const getPoiTask = async ({ controller, - ignore, poiList, showPoiCategories, poiIconCache, @@ -508,6 +507,7 @@ export default function PoiLayer() { map, zoom, }); + if (reqId !== reqIdRef.current || ignore) return; const nextState = { layer: newLayer, listFeatures, info: res.info ?? poiList?.info }; updateLayerOnMap(nextState); setPoiList(nextState); @@ -534,6 +534,7 @@ export default function PoiLayer() { async function getPoiList() { if (isEmpty(ctx.showPoiCategories)) { // if categories are cleared, then clear the list and related states + reqIdRef.current += 1; setPrevCategories(null); if (poiList) { clearPoiList(); @@ -558,7 +559,6 @@ export default function PoiLayer() { const runGetPoi = categoriesChanged || isTypeChange ? getPoiTask : debouncedGetPoi; runGetPoi({ controller, - ignore, poiList: categoriesChanged ? null : poiList, showPoiCategories: ctx.showPoiCategories, poiIconCache: ctx.poiIconCache,