From 1b973f3c9bd01c531a7326f288f76ba265e48d90 Mon Sep 17 00:00:00 2001
From: Bryan Housel <bryan@mapbox.com>
Date: Sun, 7 Jan 2018 23:46:54 -0500
Subject: [PATCH] Be more careful about enter/update selection in conflict
 resolution ui

This fixes one of the issues in #4351 where the radio button was not
selected. This was likely introduced during the upgrade to d3 v4, now
that enter selections do not automatically flow into update anymore.
(the fix is to add a `merge` to ensure that the `selection.each` actually
has some things to iterate over)
---
 modules/ui/conflicts.js | 72 ++++++++++++++++++++++++-----------------
 1 file changed, 42 insertions(+), 30 deletions(-)

diff --git a/modules/ui/conflicts.js b/modules/ui/conflicts.js
index 2180b235e..885b05479 100644
--- a/modules/ui/conflicts.js
+++ b/modules/ui/conflicts.js
@@ -13,8 +13,12 @@ import { geoExtent } from '../geo';
 import { osmChangeset } from '../osm';
 import { svgIcon } from '../svg';
 import { utilDetect } from '../util/detect';
-import { utilEntityOrMemberSelector } from '../util';
-import { utilRebind } from '../util/rebind';
+
+import {
+    utilEntityOrMemberSelector,
+    utilRebind,
+    utilWrap
+} from '../util';
 
 
 export function uiConflicts(context) {
@@ -48,25 +52,29 @@ export function uiConflicts(context) {
     function conflicts(selection) {
         keybindingOn();
 
-        var header = selection
+        var headerEnter = selection.selectAll('.header')
+            .data([0])
+            .enter()
             .append('div')
             .attr('class', 'header fillL');
 
-        header
+        headerEnter
             .append('button')
             .attr('class', 'fr')
             .on('click', cancel)
             .call(svgIcon('#icon-close'));
 
-        header
+        headerEnter
             .append('h3')
             .text(t('save.conflict.header'));
 
-        var body = selection
+        var bodyEnter = selection.selectAll('.body')
+            .data([0])
+            .enter()
             .append('div')
             .attr('class', 'body fillL');
 
-        var conflictsHelp = body
+        var conflictsHelpEnter = bodyEnter
             .append('div')
             .attr('class', 'conflicts-help')
             .text(t('save.conflict.help'));
@@ -82,9 +90,7 @@ export function uiConflicts(context) {
         var blob = new Blob([data], { type: 'text/xml;charset=utf-8;' });
         var fileName = 'changes.osc';
 
-        var linkEnter = conflictsHelp.selectAll('.download-changes')
-            .data([0])
-            .enter()
+        var linkEnter = conflictsHelpEnter.selectAll('.download-changes')
             .append('a')
             .attr('class', 'download-changes');
 
@@ -107,30 +113,30 @@ export function uiConflicts(context) {
             .text(t('save.conflict.download_changes'));
 
 
-        body
+        bodyEnter
             .append('div')
             .attr('class', 'conflict-container fillL3')
             .call(showConflict, 0);
 
-        body
+        bodyEnter
             .append('div')
             .attr('class', 'conflicts-done')
             .attr('opacity', 0)
             .style('display', 'none')
             .text(t('save.conflict.done'));
 
-        var buttons = body
+        var buttonsEnter = bodyEnter
             .append('div')
             .attr('class','buttons col12 joined conflicts-buttons');
 
-        buttons
+        buttonsEnter
             .append('button')
             .attr('disabled', _conflictList.length > 1)
             .attr('class', 'action conflicts-button col6')
             .text(t('save.title'))
             .on('click.try_again', tryAgain);
 
-        buttons
+        buttonsEnter
             .append('button')
             .attr('class', 'secondary-action conflicts-button col6')
             .text(t('confirm.cancel'))
@@ -139,7 +145,7 @@ export function uiConflicts(context) {
 
 
     function showConflict(selection, index) {
-        if (index < 0 || index >= _conflictList.length) return;
+        index = utilWrap(index, _conflictList.length);
 
         var parent = d3_select(selection.node().parentNode);
 
@@ -156,20 +162,23 @@ export function uiConflicts(context) {
             }, 250);
         }
 
-        var item = selection
+        var conflict = selection
             .selectAll('.conflict')
             .data([_conflictList[index]]);
 
-        var enter = item.enter()
+        conflict.exit()
+            .remove();
+
+        var conflictEnter = conflict.enter()
             .append('div')
             .attr('class', 'conflict');
 
-        enter
+        conflictEnter
             .append('h4')
             .attr('class', 'conflict-count')
             .text(t('save.conflict.count', { num: index + 1, total: _conflictList.length }));
 
-        enter
+        conflictEnter
             .append('a')
             .attr('class', 'conflict-description')
             .attr('href', '#')
@@ -179,7 +188,7 @@ export function uiConflicts(context) {
                 zoomToEntity(d.id);
             });
 
-        var details = enter
+        var details = conflictEnter
             .append('div')
             .attr('class', 'conflict-detail-container');
 
@@ -214,7 +223,7 @@ export function uiConflicts(context) {
             .on('click', function(d, i) {
                 d3_event.preventDefault();
 
-                var container = parent.select('.conflict-container');
+                var container = parent.selectAll('.conflict-container');
                 var sign = (i === 0 ? -1 : 1);
 
                 container
@@ -225,8 +234,6 @@ export function uiConflicts(context) {
                     .call(showConflict, index + sign);
             });
 
-        item.exit()
-            .remove();
     }
 
 
@@ -237,14 +244,15 @@ export function uiConflicts(context) {
             .selectAll('li')
             .data(function(d) { return d.choices || []; });
 
-        var enter = choices.enter()
+        // enter
+        var choicesEnter = choices.enter()
             .append('li')
             .attr('class', 'layer');
 
-        var label = enter
+        var labelEnter = choicesEnter
             .append('label');
 
-        label
+        labelEnter
             .append('input')
             .attr('type', 'radio')
             .attr('name', function(d) { return d.id; })
@@ -254,14 +262,18 @@ export function uiConflicts(context) {
                 choose(ul, d);
             });
 
-        label
+        labelEnter
             .append('span')
             .text(function(d) { return d.text; });
 
-        choices
+        // update
+        choicesEnter
+            .merge(choices)
             .each(function(d, i) {
                 var ul = this.parentNode;
-                if (ul.__data__.chosen === i) choose(ul, d);
+                if (ul.__data__.chosen === i) {
+                    choose(ul, d);
+                }
             });
     }