From bd85d31b732836d484e40b96044c73e276bfaf64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ky=E2=84=93e=20Hensel?= Date: Thu, 13 Feb 2025 01:16:54 +1100 Subject: [PATCH] fix relation membership list using a non-deterministic order (#10648) --- CHANGELOG.md | 2 + modules/actions/join.js | 2 +- modules/ui/sections/raw_membership_editor.js | 56 ++++++++++++++++---- 3 files changed, 50 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5bdc1f896..baf1c2307 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,7 @@ _Breaking developer changes, which may affect downstream projects or sites that * Prevent degenerate ways caused by deleting a corner of a triangle ([#10003], thanks [@k-yle]) * Fix briefly disappearing data layer during background layer tile layer switching transition ([#10748]) * Preserve imagery offset during tile layer switching transition ([#10748]) +* Fix the relation membership list using a non-deterministic order ([#10648], thanks [@k-yle]) * Fix over-saturated map tiles near the border of the tile service's coverage area ([#10747], thanks [@hlfan]) * Fix too dim markers of selected/hovered photo of some street level imagery layers ([#10755], thanks [@draunger]) * Fix `+` symbol appearing in changeset comments from external tools ([#10766], thanks [@k-yle]) @@ -62,6 +63,7 @@ _Breaking developer changes, which may affect downstream projects or sites that [#7381]: https://github.com/openstreetmap/iD/issues/7381 [#9635]: https://github.com/openstreetmap/iD/pull/9635 [#10003]: https://github.com/openstreetmap/iD/pull/10003 +[#10648]: https://github.com/openstreetmap/iD/pull/10648 [#10720]: https://github.com/openstreetmap/iD/issues/10720 [#10747]: https://github.com/openstreetmap/iD/issues/10747 [#10748]: https://github.com/openstreetmap/iD/issues/10748 diff --git a/modules/actions/join.js b/modules/actions/join.js index 5d20ad40a..1aee129a6 100644 --- a/modules/actions/join.js +++ b/modules/actions/join.js @@ -133,7 +133,7 @@ export function actionJoin(ids) { var sortedParentRelations = function (id) { return graph.parentRelations(graph.entity(id)) .filter((rel) => !rel.isRestriction() && !rel.isConnectivity()) - .sort((a, b) => a.id - b.id); + .sort((a, b) => a.id.localeCompare(b.id)); }; var relsA = sortedParentRelations(ids[0]); for (i = 1; i < ids.length; i++) { diff --git a/modules/ui/sections/raw_membership_editor.js b/modules/ui/sections/raw_membership_editor.js index 58f0d076d..6b9266b83 100644 --- a/modules/ui/sections/raw_membership_editor.js +++ b/modules/ui/sections/raw_membership_editor.js @@ -50,6 +50,8 @@ export function uiSectionRawMembershipEditor(context) { var _entityIDs = []; var _showBlank; var _maxMemberships = 1000; + /** @type {Set} relations that were added after this panel was opened */ + const recentlyAdded = new Set(); function getSharedParentRelations() { var parents = []; @@ -114,7 +116,37 @@ export function uiSectionRawMembershipEditor(context) { membership.role = roles.length === 1 ? roles[0] : roles; }); - return memberships; + const existingRelations = memberships + .filter(membership => !recentlyAdded.has(membership.relation.id)) + .map(membership => ({ + ...membership, + // We only sort relations that were not added just now. + // Sorting uses the same label as shown in the UI. + // If the label is not unique, the relation ID ensures + // that the sort order is still stable. + _sortKey: [ + baseDisplayValue(membership.relation), + membership.relation.id, + ].join('-'), + })) + .sort((a, b) => { + return a._sortKey.localeCompare( + b._sortKey, + localizer.localeCodes(), + { numeric: true }, + ); + }); + + + const newlyAddedRelations = memberships + .filter(membership => recentlyAdded.has(membership.relation.id)); + + return [ + // the sorted relations come first + ...existingRelations, + // then the ones that were just added from this panel + ...newlyAddedRelations, + ]; } function selectRelation(d3_event, d) { @@ -184,6 +216,7 @@ export function uiSectionRawMembershipEditor(context) { } if (d.relation) { + recentlyAdded.add(d.relation.id); context.perform( actionAddMembers(d.relation.id, _entityIDs, role), t('operations.add_member.annotation', { @@ -251,14 +284,6 @@ export function uiSectionRawMembershipEditor(context) { var graph = context.graph(); - function baseDisplayValue(entity) { - var matched = presetManager.match(entity, graph); - var presetName = (matched && matched.name()) || t('inspector.relation'); - var entityName = utilDisplayName(entity) || ''; - - return presetName + ' ' + entityName; - } - function baseDisplayLabel(entity) { var matched = presetManager.match(entity, graph); var presetName = (matched && matched.name()) || t('inspector.relation'); @@ -323,6 +348,15 @@ export function uiSectionRawMembershipEditor(context) { callback(result); } + function baseDisplayValue(entity) { + const graph = context.graph(); + var matched = presetManager.match(entity, graph); + var presetName = (matched && matched.name()) || t('inspector.relation'); + var entityName = utilDisplayName(entity) || ''; + + return presetName + ' ' + entityName; + } + function renderDisclosureContent(selection) { var memberships = getMemberships(); @@ -621,8 +655,12 @@ export function uiSectionRawMembershipEditor(context) { section.entityIDs = function(val) { if (!arguments.length) return _entityIDs; + const didChange = _entityIDs.join(',') !== val.join(','); _entityIDs = val; _showBlank = false; + if (didChange) { + recentlyAdded.clear(); // reset when the selected feature changes + } return section; };