diff --git a/CHANGELOG.md b/CHANGELOG.md index d9de63aea45..6fb38fbdfd8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -70,6 +70,7 @@ _Breaking developer changes, which may affect downstream projects or sites that * Use Röntgen icon set directly from upstream npm package ([#11784], thanks [@tordans]) * Replace deprecated `document.createEvent`/`initEvent` with modern [Event] constructor ([#11870], thanks [@JaiswalShivang]) * Fix crash in country combo field when entering unrecognized ISO country codes ([#11904], thanks [@JaiswalShivang]) +* Ensure the recent presets list is always full by filtering for location before limiting the count. ([#11405], thanks [@Razen04]) [#8464]: https://github.com/openstreetmap/iD/issues/8464 @@ -93,6 +94,7 @@ _Breaking developer changes, which may affect downstream projects or sites that [#11862]: https://github.com/openstreetmap/iD/pull/11862 [#11870]: https://github.com/openstreetmap/iD/issues/11870 [#11904]: https://github.com/openstreetmap/iD/issues/11904 +[#11405]: https://github.com/openstreetmap/iD/issues/11405 [#id-tagging-schema/pull/1507]: https://github.com/openstreetmap/id-tagging-schema/pull/1507 [@ilias52730]: https://github.com/ilias52730 [@Razen04]: https://github.com/Razen04 diff --git a/modules/presets/index.js b/modules/presets/index.js index fb7e2faf1d8..61ddb6581b4 100644 --- a/modules/presets/index.js +++ b/modules/presets/index.js @@ -406,9 +406,14 @@ export function presetIndex() { _this.defaults = (geometry, n, startWithRecents, loc, extraPresets) => { + const validHere = Array.isArray(loc) ? locationManager.locationSetsAt(loc) : null; + let recents = []; if (startWithRecents) { - recents = _this.recent().matchGeometry(geometry).collection.slice(0, 4); + // filtering before slicing to prevent unused slots in the recent preset list, issue #11405 + recents = _this.recent().matchGeometry(geometry).collection + .filter(a => !a.locationSetID || validHere[a.locationSetID]) + .slice(0, 4); } let defaults; @@ -417,7 +422,9 @@ export function presetIndex() { var preset = _this.item(id); if (preset && preset.matchGeometry(geometry)) return preset; return null; - }).filter(Boolean); + }) + .filter(Boolean) + .filter(a => !a.locationSetID || validHere[a.locationSetID]); } else { defaults = _defaults[geometry].collection.concat(_this.fallback(geometry)); } @@ -426,11 +433,6 @@ export function presetIndex() { utilArrayUniq(recents.concat(defaults).concat(extraPresets || [])).slice(0, n - 1) ); - if (Array.isArray(loc)) { - const validHere = locationManager.locationSetsAt(loc); - result.collection = result.collection.filter(a => !a.locationSetID || validHere[a.locationSetID]); - } - return result; }; diff --git a/test/spec/presets/index.js b/test/spec/presets/index.js index 0214c2fa238..9b9128def7b 100644 --- a/test/spec/presets/index.js +++ b/test/spec/presets/index.js @@ -1,3 +1,5 @@ +import { locationManager, presetIndex } from '../../../modules'; + describe('iD.presetIndex', function () { var _savedPresets, _savedAreaKeys; @@ -425,4 +427,34 @@ describe('iD.presetIndex', function () { }); }); + describe('PresetIndex.recents', () => { + it('fills all recent slots by filtering before slicing (#11405)', () => { + const testIndex = presetIndex(); + + locationManager.locationSetsAt = () => ({ 'world': true, 'us_only': false }); + + const p1 = { id: 'p1', matchGeometry: () => true, locationSetID: 'world' }; + const p2 = { id: 'p2', matchGeometry: () => true, locationSetID: 'us_only' }; // Invalid + const p3 = { id: 'p3', matchGeometry: () => true, locationSetID: 'world' }; + const p4 = { id: 'p4', matchGeometry: () => true, locationSetID: 'us_only' }; // Invalid + const p5 = { id: 'p5', matchGeometry: () => true, locationSetID: 'world' }; + + testIndex.recent = () => ({ + matchGeometry: () => ({ + collection: [p1, p2, p3, p4, p5] + }) + }); + + const locCoords = [0, 51]; + const result = testIndex.defaults('point', 10, true, locCoords); + + const ids = result.collection.map(p => p.id); + + expect(ids).to.include('p5'); + expect(ids).to.not.include('p2'); + expect(ids).to.not.include('p4'); + expect(ids.slice(0, 3)).to.eql(['p1', 'p3', 'p5']); + }); + }); + });