Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
16 changes: 9 additions & 7 deletions modules/presets/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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));
}
Expand All @@ -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;
};

Expand Down
32 changes: 32 additions & 0 deletions test/spec/presets/index.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { locationManager, presetIndex } from '../../../modules';

describe('iD.presetIndex', function () {
var _savedPresets, _savedAreaKeys;

Expand Down Expand Up @@ -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']);
});
});

});