From 6f73ae48d7d665e0a4131887eab54590404144d8 Mon Sep 17 00:00:00 2001 From: Tom MacWright Date: Tue, 22 Jan 2013 14:49:28 -0500 Subject: [PATCH 01/32] Fix global leak --- js/id/renderer/map.js | 1 + 1 file changed, 1 insertion(+) diff --git a/js/id/renderer/map.js b/js/id/renderer/map.js index b228ea765..ee145103f 100644 --- a/js/id/renderer/map.js +++ b/js/id/renderer/map.js @@ -12,6 +12,7 @@ iD.Map = function() { .on('zoom', zoomPan), dblclickEnabled = true, fastEnabled = true, + transformStart, minzoom = 0, background = iD.Background() .projection(projection), From 851eae68b86f44f465a83bb8b8e0fb90256c2eb2 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Tue, 22 Jan 2013 09:52:38 -0500 Subject: [PATCH 02/32] Match terminology with ReverseWay action --- js/id/modes/select.js | 2 +- js/id/ui/inspector.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/js/id/modes/select.js b/js/id/modes/select.js index 883acacb1..0cfd725f7 100644 --- a/js/id/modes/select.js +++ b/js/id/modes/select.js @@ -74,7 +74,7 @@ iD.modes.Select = function(entity, initial) { inspector .on('changeTags', changeTags) - .on('changeWayDirection', function(d) { + .on('reverseWay', function(d) { mode.history.perform( iD.actions.ReverseWay(d.id), 'reversed a way'); diff --git a/js/id/ui/inspector.js b/js/id/ui/inspector.js index 7837ba21f..be9db53f0 100644 --- a/js/id/ui/inspector.js +++ b/js/id/ui/inspector.js @@ -1,5 +1,5 @@ iD.ui.inspector = function() { - var event = d3.dispatch('changeTags', 'changeWayDirection', + var event = d3.dispatch('changeTags', 'reverseWay', 'update', 'remove', 'close', 'splitWay'), taginfo = iD.taginfo(), initial = false, @@ -84,7 +84,7 @@ iD.ui.inspector = function() { minorButtons.append('a') .attr('href', '#') .text('Reverse Direction') - .on('click', function() { event.changeWayDirection(entity); }); + .on('click', function() { event.reverseWay(entity); }); } if (entity.geometry() === 'vertex') { minorButtons.append('a') From ff15aa8e7b077b86ba05fa1415231f0730011eb9 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Tue, 22 Jan 2013 14:02:02 -0500 Subject: [PATCH 03/32] Rewrite d3.keybinding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A keybinding now represents a set of key commands that can be unbound as a set. Multiple keybindings are possible, and, providing a namespace is provided to the constructor, will not conflict with each other. Also, key combination strings such as ⌘+A are now supported. --- js/id/id.js | 32 ++-- js/id/modes/add_area.js | 9 +- js/id/modes/add_line.js | 9 +- js/id/modes/add_point.js | 9 +- js/id/modes/draw_area.js | 21 +-- js/id/modes/draw_line.js | 30 ++-- js/id/modes/select.js | 12 +- js/id/renderer/map.js | 9 - js/lib/d3.keybinding.js | 307 +++++++++++++++++++++------------ test/index.html | 2 + test/lib/happen.js | 27 +-- test/spec/lib/d3.keybinding.js | 55 ++++++ 12 files changed, 329 insertions(+), 193 deletions(-) create mode 100644 test/spec/lib/d3.keybinding.js diff --git a/js/id/id.js b/js/id/id.js index 31ff26836..9d7fb38bf 100644 --- a/js/id/id.js +++ b/js/id/id.js @@ -184,26 +184,18 @@ window.iD = function(container) { map.size(m.size()); }); - map.keybinding() - .on('a', function(evt, mods) { - if (mods) return; - controller.enter(iD.modes.AddArea()); - }) - .on('⌫.prevent_navigation', function(evt, mods) { - evt.preventDefault(); - }) - .on('p', function(evt, mods) { - if (mods) return; - controller.enter(iD.modes.AddPoint()); - }) - .on('l', function(evt, mods) { - if (mods) return; - controller.enter(iD.modes.AddLine()); - }) - .on('z', function(evt, mods) { - if (mods === '⇧⌘' || mods === '⌃⇧') history.redo(); - if (mods === '⌘' || mods === '⌃') history.undo(); - }); + var keybinding = d3.keybinding('main') + .on('P', function() { controller.enter(iD.modes.AddPoint()); }) + .on('L', function() { controller.enter(iD.modes.AddLine()); }) + .on('A', function() { controller.enter(iD.modes.AddArea()); }) + .on('⌘+Z', function() { history.undo(); }) + .on('⌃+Z', function() { history.undo(); }) + .on('⌘+⇧+Z', function() { history.redo(); }) + .on('⌃+⇧+Z', function() { history.redo(); }) + .on('⌫', function(e) { e.preventDefault(); }); + + d3.select(document) + .call(keybinding); var hash = iD.Hash().controller(controller).map(map); diff --git a/js/id/modes/add_area.js b/js/id/modes/add_area.js index a9fcb830e..d3936229e 100644 --- a/js/id/modes/add_area.js +++ b/js/id/modes/add_area.js @@ -6,6 +6,8 @@ iD.modes.AddArea = function() { description: 'Add parks, buildings, lakes, or other areas to the map.' }; + var keybinding = d3.keybinding('add-area'); + mode.enter = function() { var map = mode.map, history = mode.history, @@ -38,9 +40,12 @@ iD.modes.AddArea = function() { controller.enter(iD.modes.DrawArea(way.id)); }); - map.keybinding().on('⎋.addarea', function() { + keybinding.on('⎋', function() { controller.exit(); }); + + d3.select(document) + .call(keybinding); }; mode.exit = function() { @@ -49,7 +54,7 @@ iD.modes.AddArea = function() { }, 1000); mode.map.tail(false); mode.map.surface.on('click.addarea', null); - mode.map.keybinding().on('⎋.addarea', null); + keybinding.off(); }; return mode; diff --git a/js/id/modes/add_line.js b/js/id/modes/add_line.js index b83202097..db4e88e18 100644 --- a/js/id/modes/add_line.js +++ b/js/id/modes/add_line.js @@ -6,6 +6,8 @@ iD.modes.AddLine = function() { description: 'Lines can be highways, streets, pedestrian paths, or even canals.' }; + var keybinding = d3.keybinding('add-line'); + mode.enter = function() { var map = mode.map, node, @@ -60,16 +62,19 @@ iD.modes.AddLine = function() { controller.enter(iD.modes.DrawLine(way.id, direction)); }); - map.keybinding().on('⎋.addline', function() { + keybinding.on('⎋', function() { controller.exit(); }); + + d3.select(document) + .call(keybinding); }; mode.exit = function() { mode.map.dblclickEnable(true); mode.map.tail(false); mode.map.surface.on('click.addline', null); - mode.map.keybinding().on('⎋.addline', null); + keybinding.off(); }; return mode; diff --git a/js/id/modes/add_point.js b/js/id/modes/add_point.js index 82ad283c2..70275d050 100644 --- a/js/id/modes/add_point.js +++ b/js/id/modes/add_point.js @@ -5,6 +5,8 @@ iD.modes.AddPoint = function() { description: 'Restaurants, monuments, and postal boxes are points.' }; + var keybinding = d3.keybinding('add-point'); + mode.enter = function() { var map = mode.map, history = mode.history, @@ -22,15 +24,18 @@ iD.modes.AddPoint = function() { controller.enter(iD.modes.Select(node, true)); }); - map.keybinding().on('⎋.addpoint', function() { + keybinding.on('⎋', function() { controller.exit(); }); + + d3.select(document) + .call(keybinding); }; mode.exit = function() { mode.map.tail(false); mode.map.surface.on('click.addpoint', null); - mode.map.keybinding().on('⎋.addpoint', null); + keybinding.off(); }; return mode; diff --git a/js/id/modes/draw_area.js b/js/id/modes/draw_area.js index 2f1fa6304..2fe02f37c 100644 --- a/js/id/modes/draw_area.js +++ b/js/id/modes/draw_area.js @@ -4,6 +4,8 @@ iD.modes.DrawArea = function(wayId) { id: 'draw-area' }; + var keybinding = d3.keybinding('draw-area'); + mode.enter = function() { var map = mode.map, surface = map.surface, @@ -102,11 +104,14 @@ iD.modes.DrawArea = function(wayId) { .on('mouseover.drawarea', mouseover) .on('click.drawarea', click); - map.keybinding() - .on('⌫.drawarea', backspace) - .on('⌦.drawarea', del) - .on('⎋.drawarea', ret) - .on('↩.drawarea', ret); + keybinding + .on('⌫', backspace) + .on('⌦', del) + .on('⎋', ret) + .on('↩', ret); + + d3.select(document) + .call(keybinding); }; mode.exit = function() { @@ -122,11 +127,7 @@ iD.modes.DrawArea = function(wayId) { .on('mousemove.drawarea', null) .on('click.drawarea', null); - mode.map.keybinding() - .on('⎋.drawarea', null) - .on('⌫.drawarea', null) - .on('⌦.drawarea', null) - .on('↩.drawarea', null); + keybinding.off(); window.setTimeout(function() { mode.map.dblclickEnable(true); diff --git a/js/id/modes/draw_line.js b/js/id/modes/draw_line.js index 8442fbf80..e857bafe4 100644 --- a/js/id/modes/draw_line.js +++ b/js/id/modes/draw_line.js @@ -4,6 +4,8 @@ iD.modes.DrawLine = function(wayId, direction) { id: 'draw-line' }; + var keybinding = d3.keybinding('draw-line'); + mode.enter = function() { var map = mode.map, surface = map.surface, @@ -133,16 +135,19 @@ iD.modes.DrawLine = function(wayId, direction) { .on('mousemove.drawline', mousemove) .on('click.drawline', click); - map.keybinding() - .on('⌫.drawline', backspace) - .on('⌦.drawline', del) - .on('⎋.drawline', ret) - .on('↩.drawline', ret) - .on('z.drawline', function(evt, mods) { - if (mods === '⌘' || mods === '⌃') undo(); - }); + keybinding + .on('⌫', backspace) + .on('⌦', del) + .on('⎋', ret) + .on('↩', ret) + .on('⌘-Z', undo) + .on('⌃-Z', undo); - d3.select('#undo').on('click.drawline', undo); + d3.select(document) + .call(keybinding); + + d3.select('#undo') + .on('click.drawline', undo); }; mode.exit = function() { @@ -159,12 +164,7 @@ iD.modes.DrawLine = function(wayId, direction) { .on('mousemove.drawline', null) .on('click.drawline', null); - mode.map.keybinding() - .on('⌫.drawline', null) - .on('⌦.drawline', null) - .on('⎋.drawline', null) - .on('↩.drawline', null) - .on('z.drawline', null); + keybinding.off(); d3.select('#undo').on('click.drawline', null); diff --git a/js/id/modes/select.js b/js/id/modes/select.js index 0cfd725f7..b735352aa 100644 --- a/js/id/modes/select.js +++ b/js/id/modes/select.js @@ -6,6 +6,7 @@ iD.modes.Select = function(entity, initial) { }; var inspector = iD.ui.inspector().initial(!!initial), + keybinding = d3.keybinding('select'), behaviors; function remove() { @@ -132,10 +133,10 @@ iD.modes.Select = function(entity, initial) { surface.on('click.select', click) .on('dblclick.browse', dblclick); - mode.map.keybinding().on('⌫.select', function(e) { - remove(); - e.preventDefault(); - }); + keybinding.on('⌫', remove); + + d3.select(document) + .call(keybinding); surface.selectAll("*") .filter(function (d) { @@ -166,8 +167,9 @@ iD.modes.Select = function(entity, initial) { var q = iD.util.stringQs(location.hash.substring(1)); location.hash = '#' + iD.util.qsString(_.omit(q, 'id'), true); + keybinding.off(); + surface.on("click.select", null); - mode.map.keybinding().on('⌫.select', null); mode.history.on('change.entity-undone', null); surface.selectAll(".selected") diff --git a/js/id/renderer/map.js b/js/id/renderer/map.js index ee145103f..7e74d7b66 100644 --- a/js/id/renderer/map.js +++ b/js/id/renderer/map.js @@ -2,7 +2,6 @@ iD.Map = function() { var connection, history, dimensions = [], dispatch = d3.dispatch('move', 'drawn'), - keybinding = d3.keybinding(), projection = d3.geo.mercator().scale(1024), roundedProjection = iD.svg.RoundProjection(projection), zoom = d3.behavior.zoom() @@ -49,8 +48,6 @@ iD.Map = function() { supersurface .call(tail); - - d3.select(document).call(keybinding); } function pxCenter() { return [dimensions[0] / 2, dimensions[1] / 2]; } @@ -345,12 +342,6 @@ iD.Map = function() { return map; }; - map.keybinding = function (_) { - if (!arguments.length) return keybinding; - keybinding = _; - return map; - }; - map.background = background; map.projection = projection; map.redraw = redraw; diff --git a/js/lib/d3.keybinding.js b/js/lib/d3.keybinding.js index 01256aa20..e16ade725 100644 --- a/js/lib/d3.keybinding.js +++ b/js/lib/d3.keybinding.js @@ -1,120 +1,197 @@ -d3.keybinding = function() { - // via https://github.com/keithamus/jwerty/ - // and https://github.com/madrobby/keymaster - var _keys = { - // MOD aka toggleable keys - mods: { - // Shift key, ⇧ - '⇧': 16, - // CTRL key, on Mac: ⌃ - '⌃': 17, - // ALT key, on Mac: ⌥ (Alt) - '⌥': 18, - // META, on Mac: ⌘ (CMD), on Windows (Win), on Linux (Super) - '⌘': 91 - }, - // Normal keys - keys: { - // Backspace key, on Mac: ⌫ (Backspace) - '⌫': 8, backspace: 8, - // Tab Key, on Mac: ⇥ (Tab), on Windows ⇥⇥ - '⇥': 9, '⇆': 9, tab: 9, - // Return key, ↩ - '↩': 13, 'return': 13, enter: 13, '⌅': 13, - // Pause/Break key - 'pause': 19, 'pause-break': 19, - // Caps Lock key, ⇪ - '⇪': 20, caps: 20, 'caps-lock': 20, - // Escape key, on Mac: ⎋, on Windows: Esc - '⎋': 27, escape: 27, esc: 27, - // Space key - space: 32, - // Page-Up key, or pgup, on Mac: ↖ - '↖': 33, pgup: 33, 'page-up': 33, - // Page-Down key, or pgdown, on Mac: ↘ - '↘': 34, pgdown: 34, 'page-down': 34, - // END key, on Mac: ⇟ - '⇟': 35, end: 35, - // HOME key, on Mac: ⇞ - '⇞': 36, home: 36, - // Insert key, or ins - ins: 45, insert: 45, - // Delete key, on Mac: ⌦ (Delete) - '⌦': 46, del: 46, 'delete': 46, - // Left Arrow Key, or ← - '←': 37, left: 37, 'arrow-left': 37, - // Up Arrow Key, or ↑ - '↑': 38, up: 38, 'arrow-up': 38, - // Right Arrow Key, or → - '→': 39, right: 39, 'arrow-right': 39, - // Up Arrow Key, or ↓ - '↓': 40, down: 40, 'arrow-down': 40, - // odities, printing characters that come out wrong: - // Num-Multiply, or * - '*': 106, star: 106, asterisk: 106, multiply: 106, - // Num-Plus or + - '+': 107, 'plus': 107, - // Num-Subtract, or - - '-': 109, subtract: 109, - // Semicolon - ';': 186, semicolon:186, - // = or equals - '=': 187, 'equals': 187, - // Comma, or , - ',': 188, comma: 188, - //'-': 189, //??? - // Period, or ., or full-stop - '.': 190, period: 190, 'full-stop': 190, - // Slash, or /, or forward-slash - '/': 191, slash: 191, 'forward-slash': 191, - // Tick, or `, or back-quote - '`': 192, tick: 192, 'back-quote': 192, - // Open bracket, or [ - '[': 219, 'open-bracket': 219, - // Back slash, or \ - '\\': 220, 'back-slash': 220, - // Close backet, or ] - ']': 221, 'close-bracket': 221, - // Apostraphe, or Quote, or ' - '\'': 222, quote: 222, apostraphe: 222 +/* + * This code is licensed under the MIT license. + * + * Copyright © 2013, iD authors. + * + * Portions copyright © 2011, Keith Cirkel + * See https://github.com/keithamus/jwerty + * + */ +d3.keybinding = function(namespace) { + var bindings = []; + + function matches(binding, event) { + for (var p in binding.event) { + if (event[p] != binding.event[p]) + return false; } - }; - // To minimise code bloat, add all of the NUMPAD 0-9 keys in a loop - var i = 95, n = 0; - while (++i < 106) _keys.keys['num-' + n] = i; ++n; - // To minimise code bloat, add all of the top row 0-9 keys in a loop - i = 47, n = 0; - while (++i < 58) _keys.keys[n] = i; ++n; - // To minimise code bloat, add all of the F1-F25 keys in a loop - i = 111, n = 1; - while (++i < 136) _keys.keys['f' + n] = i; ++n; - // To minimise code bloat, add all of the letters of the alphabet in a loop - i = 64; - while(++i < 91) _keys.keys[String.fromCharCode(i).toLowerCase()] = i; - var pairs = d3.entries(_keys.keys), - event = d3.dispatch.apply(d3, d3.keys(_keys.keys)); - - function keys(selection) { - selection.on('keydown', function () { - var tagName = d3.select(d3.event.target).node().tagName; - if (tagName == 'INPUT' || tagName == 'SELECT' || tagName == 'TEXTAREA') { - return; - } - - var modifiers = ''; - if (d3.event.shiftKey) modifiers += '⇧'; - if (d3.event.ctrlKey) modifiers += '⌃'; - if (d3.event.altKey) modifiers += '⌥'; - if (d3.event.metaKey) modifiers += '⌘'; - - pairs.filter(function(d) { - return d.value === d3.event.keyCode; - }).forEach(function(d) { - event[d.key](d3.event, modifiers); - }); - }); + return (!binding.capture) === (event.eventPhase !== Event.CAPTURING_PHASE); } - return d3.rebind(keys, event, 'on'); + function capture() { + for (var i = 0; i < bindings.length; i++) { + var binding = bindings[i]; + if (matches(binding, d3.event)) { + binding.callback(); + } + } + } + + function bubble() { + var tagName = d3.select(d3.event.target).node().tagName; + if (tagName == 'INPUT' || tagName == 'SELECT' || tagName == 'TEXTAREA') { + return; + } + capture(); + } + + function keybinding(selection) { + selection = selection || d3.select(document); + selection.on('keydown.capture' + namespace, capture, true); + selection.on('keydown.bubble' + namespace, bubble, false); + return keybinding; + } + + keybinding.off = function(selection) { + selection = selection || d3.select(document); + selection.on('keydown.capture' + namespace, null); + selection.on('keydown.bubble' + namespace, null); + return keybinding; + }; + + keybinding.on = function(code, callback, capture) { + var binding = { + event: { + keyCode: 0, + shiftKey: false, + ctrlKey: false, + altKey: false, + metaKey: false + }, + capture: capture, + callback: callback + }; + + code = code.toLowerCase().match(/(?:(?:[^+])+|\+\+|^\+$)/g); + + for (var i = 0; i < code.length; i++) { + // Normalise matching errors + if (code[i] === '++') code[i] = '+'; + + if (code[i] in d3.keybinding.modifierCodes) { + binding.event[d3.keybinding.modifierProperties[d3.keybinding.modifierCodes[code[i]]]] = true; + } else if (code[i] in d3.keybinding.keyCodes) { + binding.event.keyCode = d3.keybinding.keyCodes[code[i]]; + } + } + + bindings.push(binding); + + return keybinding; + }; + + return keybinding; }; + +(function () { + d3.keybinding.modifierCodes = { + // Shift key, ⇧ + '⇧': 16, shift: 16, + // CTRL key, on Mac: ⌃ + '⌃': 17, ctrl: 17, + // ALT key, on Mac: ⌥ (Alt) + '⌥': 18, alt: 18, option: 18, + // META, on Mac: ⌘ (CMD), on Windows (Win), on Linux (Super) + '⌘': 91, meta: 91, cmd: 91, 'super': 91, win: 91 + }; + + d3.keybinding.modifierProperties = { + 16: 'shiftKey', + 17: 'ctrlKey', + 18: 'altKey', + 91: 'metaKey' + }; + + d3.keybinding.keyCodes = { + // Backspace key, on Mac: ⌫ (Backspace) + '⌫': 8, backspace: 8, + // Tab Key, on Mac: ⇥ (Tab), on Windows ⇥⇥ + '⇥': 9, '⇆': 9, tab: 9, + // Return key, ↩ + '↩': 13, 'return': 13, enter: 13, '⌅': 13, + // Pause/Break key + 'pause': 19, 'pause-break': 19, + // Caps Lock key, ⇪ + '⇪': 20, caps: 20, 'caps-lock': 20, + // Escape key, on Mac: ⎋, on Windows: Esc + '⎋': 27, escape: 27, esc: 27, + // Space key + space: 32, + // Page-Up key, or pgup, on Mac: ↖ + '↖': 33, pgup: 33, 'page-up': 33, + // Page-Down key, or pgdown, on Mac: ↘ + '↘': 34, pgdown: 34, 'page-down': 34, + // END key, on Mac: ⇟ + '⇟': 35, end: 35, + // HOME key, on Mac: ⇞ + '⇞': 36, home: 36, + // Insert key, or ins + ins: 45, insert: 45, + // Delete key, on Mac: ⌦ (Delete) + '⌦': 46, del: 46, 'delete': 46, + // Left Arrow Key, or ← + '←': 37, left: 37, 'arrow-left': 37, + // Up Arrow Key, or ↑ + '↑': 38, up: 38, 'arrow-up': 38, + // Right Arrow Key, or → + '→': 39, right: 39, 'arrow-right': 39, + // Up Arrow Key, or ↓ + '↓': 40, down: 40, 'arrow-down': 40, + // odities, printing characters that come out wrong: + // Num-Multiply, or * + '*': 106, star: 106, asterisk: 106, multiply: 106, + // Num-Plus or + + '+': 107, 'plus': 107, + // Num-Subtract, or - + '-': 109, subtract: 109, + // Semicolon + ';': 186, semicolon:186, + // = or equals + '=': 187, 'equals': 187, + // Comma, or , + ',': 188, comma: 188, + //'-': 189, //??? + // Period, or ., or full-stop + '.': 190, period: 190, 'full-stop': 190, + // Slash, or /, or forward-slash + '/': 191, slash: 191, 'forward-slash': 191, + // Tick, or `, or back-quote + '`': 192, tick: 192, 'back-quote': 192, + // Open bracket, or [ + '[': 219, 'open-bracket': 219, + // Back slash, or \ + '\\': 220, 'back-slash': 220, + // Close backet, or ] + ']': 221, 'close-bracket': 221, + // Apostrophe, or Quote, or ' + '\'': 222, quote: 222, apostrophe: 222 + }; + + // NUMPAD 0-9 + var i = 95, n = 0; + while (++i < 106) { + d3.keybinding.keyCodes['num-' + n] = i; + ++n; + } + + // 0-9 + i = 47; n = 0; + while (++i < 58) { + d3.keybinding.keyCodes[n] = i; + ++n; + } + + // F1-F25 + i = 111; n = 1; + while (++i < 136) { + d3.keybinding.keyCodes['f' + n] = i; + ++n; + } + + // a-z + i = 64; + while (++i < 91) { + d3.keybinding.keyCodes[String.fromCharCode(i).toLowerCase()] = i; + } +})(); diff --git a/test/index.html b/test/index.html index 425258d20..4099f2786 100644 --- a/test/index.html +++ b/test/index.html @@ -124,6 +124,8 @@ + + diff --git a/test/lib/happen.js b/test/lib/happen.js index 7c46d238d..8777d883a 100644 --- a/test/lib/happen.js +++ b/test/lib/happen.js @@ -19,10 +19,10 @@ evt = new Event(o.type); evt.keyCode = o.keyCode || 0; evt.charCode = o.charCode || 0; - evt.shift = o.shift || false; - evt.meta = o.meta || false; - evt.ctrl = o.ctrl || false; - evt.alt = o.alt || false; + evt.shiftKey = o.shiftKey || false; + evt.metaKey = o.metaKey || false; + evt.ctrlKey = o.ctrlKey || false; + evt.altKey = o.altKey || false; } else { evt = document.createEvent('KeyboardEvent'); // https://developer.mozilla.org/en/DOM/event.initKeyEvent @@ -33,10 +33,10 @@ true, // in boolean canBubbleArg, true, // in boolean cancelableArg, null, // in nsIDOMAbstractView viewArg, Specifies UIEvent.view. This value may be null. - o.ctrl || false, // in boolean ctrlKeyArg, - o.alt || false, // in boolean altKeyArg, - o.shift || false, // in boolean shiftKeyArg, - o.meta || false, // in boolean metaKeyArg, + o.ctrlKey || false, // in boolean ctrlKeyArg, + o.altKey || false, // in boolean altKeyArg, + o.shiftKey || false, // in boolean shiftKeyArg, + o.metaKey || false, // in boolean metaKeyArg, o.keyCode || 0, // in unsigned long keyCodeArg, o.charCode || 0 // in unsigned long charCodeArg); ); @@ -53,10 +53,10 @@ o.screenY || 0, // screenY o.clientX || 0, // clientX o.clientY || 0, // clientY - o.ctrl || 0, // ctrl - o.alt || false, // alt - o.shift || false, // shift - o.meta || false, // meta + o.ctrlKey || 0, // ctrl + o.altKey || false, // alt + o.shiftKey || false, // shift + o.metaKey || false, // meta o.button || false, // mouse button null // relatedTarget ); @@ -65,7 +65,8 @@ x.dispatchEvent(evt); }; - var shortcuts = ['click', 'mousedown', 'mouseup', 'mousemove', 'keydown', 'keyup', 'keypress'], + var shortcuts = ['click', 'mousedown', 'mouseup', 'mousemove', + 'mouseover', 'mouseout', 'keydown', 'keyup', 'keypress'], s, i = 0; while (s = shortcuts[i++]) { diff --git a/test/spec/lib/d3.keybinding.js b/test/spec/lib/d3.keybinding.js new file mode 100644 index 000000000..23e1a0e0c --- /dev/null +++ b/test/spec/lib/d3.keybinding.js @@ -0,0 +1,55 @@ +describe("d3.keybinding", function() { + var keybinding, spy, input; + + beforeEach(function () { + keybinding = d3.keybinding('keybinding-test'); + spy = sinon.spy(); + input = d3.select('body') + .append('input'); + }); + + afterEach(function () { + keybinding.off(d3.select(document)); + input.remove(); + }); + + describe("#on", function () { + it("returns self", function () { + expect(keybinding.on('a', spy)).to.equal(keybinding); + }); + + it("adds a binding for the specified bare key", function () { + d3.select(document).call(keybinding.on('A', spy)); + + happen.keydown(document, {keyCode: 65, metaKey: true}); + expect(spy).not.to.have.been.called; + + happen.keydown(document, {keyCode: 65}); + expect(spy).to.have.been.called; + }); + + it("adds a binding for the specified key combination", function () { + d3.select(document).call(keybinding.on('⌘+A', spy)); + + happen.keydown(document, {keyCode: 65}); + expect(spy).not.to.have.been.called; + + happen.keydown(document, {keyCode: 65, metaKey: true}); + expect(spy).to.have.been.called; + }); + + it("does not dispatch when focus is in input elements by default", function () { + d3.select(document).call(keybinding.on('A', spy)); + + happen.keydown(input.node(), {keyCode: 65}); + expect(spy).not.to.have.been.called; + }); + + it("dispatches when focus is in input elements when the capture flag was passed", function () { + d3.select(document).call(keybinding.on('A', spy, true)); + + happen.keydown(input.node(), {keyCode: 65}); + expect(spy).to.have.been.called; + }); + }); +}); From 814c3608db75d84ff3b8ce4bf30a1a67f7a6a45d Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Tue, 22 Jan 2013 14:42:29 -0500 Subject: [PATCH 04/32] Hook into undos in a different way This way doesn't depend on details of keybindings and the undo button. Also, implement this in DrawArea mode. --- js/id/graph/history.js | 4 +++- js/id/modes/draw_area.js | 14 +++++++++++--- js/id/modes/draw_line.js | 26 +++++++++++--------------- test/spec/graph/history.js | 15 +++++++++++++++ 4 files changed, 40 insertions(+), 19 deletions(-) diff --git a/js/id/graph/history.js b/js/id/graph/history.js index 3d027d042..142a9eeca 100644 --- a/js/id/graph/history.js +++ b/js/id/graph/history.js @@ -1,7 +1,7 @@ iD.History = function() { var stack, index, imagery_used = 'Bing', - dispatch = d3.dispatch('change'); + dispatch = d3.dispatch('change', 'undone', 'redone'); function perform(actions) { actions = Array.prototype.slice.call(actions); @@ -62,6 +62,7 @@ iD.History = function() { if (stack[index].annotation) break; } + dispatch.undone(); change(previous); }, @@ -73,6 +74,7 @@ iD.History = function() { if (stack[index].annotation) break; } + dispatch.redone(); change(previous); }, diff --git a/js/id/modes/draw_area.js b/js/id/modes/draw_area.js index 2fe02f37c..026aa5bbc 100644 --- a/js/id/modes/draw_area.js +++ b/js/id/modes/draw_area.js @@ -112,16 +112,22 @@ iD.modes.DrawArea = function(wayId) { d3.select(document) .call(keybinding); + + history.on('undone.drawline', function () { + controller.enter(iD.modes.Browse()); + }); }; mode.exit = function() { - var surface = mode.map.surface; + var map = mode.map, + surface = map.surface, + history = mode.history; surface.selectAll('.way, .node') .classed('active', false); - mode.map.tail(false); - mode.map.fastEnable(true); + map.tail(false); + map.fastEnable(true); surface .on('mousemove.drawarea', null) @@ -129,6 +135,8 @@ iD.modes.DrawArea = function(wayId) { keybinding.off(); + history.on('undone.drawline', null); + window.setTimeout(function() { mode.map.dblclickEnable(true); }, 1000); diff --git a/js/id/modes/draw_line.js b/js/id/modes/draw_line.js index e857bafe4..95719a6a9 100644 --- a/js/id/modes/draw_line.js +++ b/js/id/modes/draw_line.js @@ -126,11 +126,6 @@ iD.modes.DrawLine = function(wayId, direction) { controller.enter(iD.modes.Select(way, true)); } - function undo() { - history.undo(); - controller.enter(iD.modes.Browse()); - } - surface .on('mousemove.drawline', mousemove) .on('click.drawline', click); @@ -139,26 +134,27 @@ iD.modes.DrawLine = function(wayId, direction) { .on('⌫', backspace) .on('⌦', del) .on('⎋', ret) - .on('↩', ret) - .on('⌘-Z', undo) - .on('⌃-Z', undo); + .on('↩', ret); d3.select(document) .call(keybinding); - d3.select('#undo') - .on('click.drawline', undo); + history.on('undone.drawline', function () { + controller.enter(iD.modes.Browse()); + }); }; mode.exit = function() { - var surface = mode.map.surface; + var map = mode.map, + surface = map.surface, + history = mode.history; surface.selectAll('.way, .node') .classed('active', false); - mode.map.tail(false); - mode.map.fastEnable(true); - mode.map.minzoom(0); + map.tail(false); + map.fastEnable(true); + map.minzoom(0); surface .on('mousemove.drawline', null) @@ -166,7 +162,7 @@ iD.modes.DrawLine = function(wayId, direction) { keybinding.off(); - d3.select('#undo').on('click.drawline', null); + history.on('undone.drawline', null); window.setTimeout(function() { mode.map.dblclickEnable(true); diff --git a/test/spec/graph/history.js b/test/spec/graph/history.js index 554b22a82..309d396a1 100644 --- a/test/spec/graph/history.js +++ b/test/spec/graph/history.js @@ -83,6 +83,13 @@ describe("iD.History", function () { expect(history.redoAnnotation()).to.equal("annotation"); }); + it("emits an undone event", function () { + history.perform(action); + history.on('undone', spy); + history.undo(); + expect(spy).to.have.been.called; + }); + it("emits a change event", function () { history.perform(action); history.on('change', spy); @@ -92,6 +99,14 @@ describe("iD.History", function () { }); describe("#redo", function () { + it("emits an redone event", function () { + history.perform(action); + history.undo(); + history.on('change', spy); + history.redo(); + expect(spy).to.have.been.called; + }); + it("emits a change event", function () { history.perform(action); history.undo(); From 2e6c23d7ef6f3405ea2aeb33e9568b683b24b069 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Tue, 22 Jan 2013 14:58:45 -0500 Subject: [PATCH 05/32] Fix namespace --- js/id/modes/draw_area.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/js/id/modes/draw_area.js b/js/id/modes/draw_area.js index 026aa5bbc..a90d13c9c 100644 --- a/js/id/modes/draw_area.js +++ b/js/id/modes/draw_area.js @@ -113,7 +113,7 @@ iD.modes.DrawArea = function(wayId) { d3.select(document) .call(keybinding); - history.on('undone.drawline', function () { + history.on('undone.drawarea', function () { controller.enter(iD.modes.Browse()); }); }; @@ -135,7 +135,7 @@ iD.modes.DrawArea = function(wayId) { keybinding.off(); - history.on('undone.drawline', null); + history.on('undone.drawarea', null); window.setTimeout(function() { mode.map.dblclickEnable(true); From dbd5fc65435e65f44b832e42ede4fb734fdc42f7 Mon Sep 17 00:00:00 2001 From: Saman Bemel-Benrud Date: Tue, 22 Jan 2013 15:15:41 -0500 Subject: [PATCH 06/32] minor style tweak to forms. --- css/app.css | 1 - 1 file changed, 1 deletion(-) diff --git a/css/app.css b/css/app.css index 312f958a8..466183132 100644 --- a/css/app.css +++ b/css/app.css @@ -106,7 +106,6 @@ input[type=text] { background-color: black; border:1px solid rgba(255, 255, 255, .25); color: white; - border-radius: 4px; } textarea:focus, From f2c6227cb372b0d3f5e4e944a55e53d503f35f7e Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Tue, 22 Jan 2013 17:35:58 -0500 Subject: [PATCH 07/32] Fix build Failing due to a bug in PhantomJS (a.k.a Webkit), see https://github.com/tmcw/happen/issues/5 --- test/lib/happen.js | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/test/lib/happen.js b/test/lib/happen.js index 8777d883a..2f7f5e6cb 100644 --- a/test/lib/happen.js +++ b/test/lib/happen.js @@ -40,6 +40,23 @@ o.keyCode || 0, // in unsigned long keyCodeArg, o.charCode || 0 // in unsigned long charCodeArg); ); + + // Workaround for https://bugs.webkit.org/show_bug.cgi?id=16735 + if (evt.ctrlKey != (o.ctrlKey || 0) || + evt.altKey != (o.altKey || 0) || + evt.shiftKey != (o.shiftKey || 0) || + evt.metaKey != (o.metaKey || 0) || + evt.keyCode != (o.keyCode || 0) || + evt.charCode != (o.charCode || 0)) { + evt = document.createEvent('Event'); + evt.initEvent(o.type, true, true); + evt.ctrlKey = o.ctrlKey || false; + evt.altKey = o.altKey || false; + evt.shiftKey = o.shiftKey || false; + evt.metaKey = o.metaKey || false; + evt.keyCode = o.keyCode || 0; + evt.charCode = o.charCode || 0; + } } } else { evt = document.createEvent('MouseEvents'); From d74bf1e39a64650ba9965604ab9665113d8977b9 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Fri, 11 Jan 2013 15:17:21 -0800 Subject: [PATCH 08/32] Use proper prototypal inheritance and less dynamic new --- js/id/connection.js | 11 ++--------- js/id/graph/entity.js | 13 ------------- js/id/graph/node.js | 12 +++++++++++- js/id/graph/relation.js | 12 +++++++++++- js/id/graph/way.js | 12 +++++++++++- 5 files changed, 35 insertions(+), 25 deletions(-) diff --git a/js/id/connection.js b/js/id/connection.js index 003750ff2..7b9f98a27 100644 --- a/js/id/connection.js +++ b/js/id/connection.js @@ -77,15 +77,8 @@ iD.Connection = function() { delete o.lat; } o.id = iD.Entity.id.fromOSM(o.type, o.id); - switch (o.type) { - case 'node': - o._poi = !refNodes[o.id]; - return iD.Node(o); - case 'way': - return iD.Way(o); - case 'relation': - return iD.Relation(o); - } + if (o.type === 'node') o._poi = !refNodes[o.id]; + return new iD.Entity[o.type](o); } function parse(dom) { diff --git a/js/id/graph/entity.js b/js/id/graph/entity.js index 8c8f9a6fb..45d0d0c5c 100644 --- a/js/id/graph/entity.js +++ b/js/id/graph/entity.js @@ -109,16 +109,3 @@ iD.Entity.prototype = { return n.length === 0 ? 'unknown' : n.join('; '); } }; - -iD.Entity.extend = function(properties) { - var Subclass = function() { - if (this instanceof Subclass) return; - return (new Subclass()).initialize(arguments); - }; - - Subclass.prototype = new iD.Entity(); - _.extend(Subclass.prototype, properties); - iD.Entity[properties.type] = Subclass; - - return Subclass; -}; diff --git a/js/id/graph/node.js b/js/id/graph/node.js index fd3a1cd69..d72bf6442 100644 --- a/js/id/graph/node.js +++ b/js/id/graph/node.js @@ -1,4 +1,14 @@ -iD.Node = iD.Entity.extend({ +iD.Node = iD.Entity.node = function iD_Node() { + if (!(this instanceof iD_Node)) { + return (new iD_Node()).initialize(arguments); + } else if (arguments.length) { + this.initialize(arguments); + } +}; + +iD.Node.prototype = Object.create(iD.Entity.prototype); + +_.extend(iD.Node.prototype, { type: "node", extent: function() { diff --git a/js/id/graph/relation.js b/js/id/graph/relation.js index dc298ecbd..e7d4afd0a 100644 --- a/js/id/graph/relation.js +++ b/js/id/graph/relation.js @@ -1,4 +1,14 @@ -iD.Relation = iD.Entity.extend({ +iD.Relation = iD.Entity.relation = function iD_Relation() { + if (!(this instanceof iD_Relation)) { + return (new iD_Relation()).initialize(arguments); + } else if (arguments.length) { + this.initialize(arguments); + } +}; + +iD.Relation.prototype = Object.create(iD.Entity.prototype); + +_.extend(iD.Relation.prototype, { type: "relation", members: [], diff --git a/js/id/graph/way.js b/js/id/graph/way.js index 9efd4fe6c..35834e761 100644 --- a/js/id/graph/way.js +++ b/js/id/graph/way.js @@ -1,4 +1,14 @@ -iD.Way = iD.Entity.extend({ +iD.Way = iD.Entity.way = function iD_Way() { + if (!(this instanceof iD_Way)) { + return (new iD_Way()).initialize(arguments); + } else if (arguments.length) { + this.initialize(arguments); + } +}; + +iD.Way.prototype = Object.create(iD.Entity.prototype); + +_.extend(iD.Way.prototype, { type: "way", nodes: [], From 381794e7a164f5bdb94fd1a76800d1bb57e6f9f3 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Tue, 22 Jan 2013 17:54:57 -0500 Subject: [PATCH 09/32] Fix Relation#multipolygon for unmatched inner rings (fixes #461) --- js/id/graph/relation.js | 2 +- test/spec/graph/relation.js | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/js/id/graph/relation.js b/js/id/graph/relation.js index e7d4afd0a..a09d6dac0 100644 --- a/js/id/graph/relation.js +++ b/js/id/graph/relation.js @@ -140,7 +140,7 @@ _.extend(iD.Relation.prototype, { if (o !== undefined) result[o].push(inners[i]); else - result.push(inners[i]); // Invalid geometry + result.push([inners[i]]); // Invalid geometry } return result; diff --git a/test/spec/graph/relation.js b/test/spec/graph/relation.js index f8b49a213..58b1767f4 100644 --- a/test/spec/graph/relation.js +++ b/test/spec/graph/relation.js @@ -298,6 +298,17 @@ describe('iD.Relation', function () { expect(r.multipolygon(graph)).to.eql([[[a, b, c, a], [d, e, f, d]], [[g, h, i, g]]]); }); + specify("invalid geometry: unmatched inner", function () { + var a = iD.Node(), + b = iD.Node(), + c = iD.Node(), + w = iD.Way({nodes: [a.id, b.id, c.id, a.id]}), + r = iD.Relation({members: [{id: w.id, role: 'inner', type: 'way'}]}), + g = iD.Graph([a, b, c, w, r]); + + expect(r.multipolygon(g)).to.eql([[[a, b, c, a]]]); + }); + specify("incomplete relation", function () { var a = iD.Node(), b = iD.Node(), From 7b18674e91a38fd463f13e0f99b4899921f77326 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Tue, 22 Jan 2013 18:15:04 -0500 Subject: [PATCH 10/32] This is maybe working on PhantomJS now --- test/spec/modes/add_point.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/spec/modes/add_point.js b/test/spec/modes/add_point.js index 26cfcd013..725a7a396 100644 --- a/test/spec/modes/add_point.js +++ b/test/spec/modes/add_point.js @@ -33,7 +33,7 @@ describe("iD.modes.AddPoint", function () { }); describe("pressing ⎋", function () { - xit("exits to browse mode", function () { + it("exits to browse mode", function () { happen.keydown(document, {keyCode: 27}); expect(controller.mode.id).to.equal('browse'); }); From 7f8ff43a0fc4d58c069cb6260c4299d9a25fc3b4 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Tue, 22 Jan 2013 18:16:52 -0500 Subject: [PATCH 11/32] Fix namespaces and unbinding --- js/id/modes/select.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/js/id/modes/select.js b/js/id/modes/select.js index b735352aa..2f203b450 100644 --- a/js/id/modes/select.js +++ b/js/id/modes/select.js @@ -131,7 +131,7 @@ iD.modes.Select = function(entity, initial) { } surface.on('click.select', click) - .on('dblclick.browse', dblclick); + .on('dblclick.select', dblclick); keybinding.on('⌫', remove); @@ -169,7 +169,9 @@ iD.modes.Select = function(entity, initial) { keybinding.off(); - surface.on("click.select", null); + surface.on('click.select', null) + .on('dblclick.select', null); + mode.history.on('change.entity-undone', null); surface.selectAll(".selected") From c263ebd4dc0b6b9ee6e24fdf9a3c1f736a502b38 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Tue, 22 Jan 2013 18:20:20 -0500 Subject: [PATCH 12/32] iD.util.geo => iD.geo --- js/id/geo.js | 74 ++++++++++++++++++++++++++++++++++++++ js/id/graph/relation.js | 4 +-- js/id/modes/add_line.js | 2 +- js/id/modes/draw_line.js | 2 +- js/id/modes/select.js | 2 +- js/id/renderer/map.js | 2 +- js/id/svg.js | 2 +- js/id/svg/midpoints.js | 4 +-- js/id/util.js | 76 ---------------------------------------- test/spec/util.js | 30 ++++++++-------- 10 files changed, 98 insertions(+), 100 deletions(-) diff --git a/js/id/geo.js b/js/id/geo.js index 06b63ae14..b05a185fa 100644 --- a/js/id/geo.js +++ b/js/id/geo.js @@ -1 +1,75 @@ iD.geo = {}; + +iD.geo.roundCoords = function(c) { + return [Math.floor(c[0]), Math.floor(c[1])]; +}; + +iD.geo.interp = function(p1, p2, t) { + return [p1[0] + (p2[0] - p1[0]) * t, + p1[1] + (p2[1] - p1[1]) * t]; +}; + +iD.geo.dist = function(a, b) { + return Math.sqrt(Math.pow(a[0] - b[0], 2) + + Math.pow(a[1] - b[1], 2)); +}; + +iD.geo.chooseIndex = function(way, point, map) { + var dist = iD.geo.dist, + projNodes = way.nodes.map(function(n) { + return map.projection(n.loc); + }); + + for (var i = 0, changes = []; i < projNodes.length - 1; i++) { + changes[i] = + (dist(projNodes[i], point) + dist(point, projNodes[i + 1])) / + dist(projNodes[i], projNodes[i + 1]); + } + + var idx = _.indexOf(changes, _.min(changes)), + ratio = dist(projNodes[idx], point) / dist(projNodes[idx], projNodes[idx + 1]), + loc = iD.geo.interp(way.nodes[idx].loc, way.nodes[idx + 1].loc, ratio); + + return { + index: idx + 1, + loc: loc + }; +}; + +// Return whether point is contained in polygon. +// +// `point` should be a 2-item array of coordinates. +// `polygon` should be an array of 2-item arrays of coordinates. +// +// From https://github.com/substack/point-in-polygon. +// ray-casting algorithm based on +// http://www.ecse.rpi.edu/Homepages/wrf/Research/Short_Notes/pnpoly.html +// +iD.geo.pointInPolygon = function(point, polygon) { + var x = point[0], + y = point[1], + inside = false; + + for (var i = 0, j = polygon.length - 1; i < polygon.length; j = i++) { + var xi = polygon[i][0], yi = polygon[i][1]; + var xj = polygon[j][0], yj = polygon[j][1]; + + var intersect = ((yi > y) != (yj > y)) && + (x < (xj - xi) * (y - yi) / (yj - yi) + xi); + if (intersect) inside = !inside; + } + + return inside; +}; + +iD.geo.polygonContainsPolygon = function(outer, inner) { + return _.every(inner, function (point) { + return iD.geo.pointInPolygon(point, outer); + }); +}; + +iD.geo.polygonIntersectsPolygon = function(outer, inner) { + return _.some(inner, function (point) { + return iD.geo.pointInPolygon(point, outer); + }); +}; diff --git a/js/id/graph/relation.js b/js/id/graph/relation.js index a09d6dac0..2dd325438 100644 --- a/js/id/graph/relation.js +++ b/js/id/graph/relation.js @@ -120,13 +120,13 @@ _.extend(iD.Relation.prototype, { for (o = 0; o < outers.length; o++) { outer = _.pluck(outers[o], 'loc'); - if (iD.util.geo.polygonContainsPolygon(outer, inner)) + if (iD.geo.polygonContainsPolygon(outer, inner)) return o; } for (o = 0; o < outers.length; o++) { outer = _.pluck(outers[o], 'loc'); - if (iD.util.geo.polygonIntersectsPolygon(outer, inner)) + if (iD.geo.polygonIntersectsPolygon(outer, inner)) return o; } } diff --git a/js/id/modes/add_line.js b/js/id/modes/add_line.js index db4e88e18..8baa8e30e 100644 --- a/js/id/modes/add_line.js +++ b/js/id/modes/add_line.js @@ -40,7 +40,7 @@ iD.modes.AddLine = function() { } else if (datum.type === 'way') { // begin a new way starting from an existing way - var choice = iD.util.geo.chooseIndex(datum, d3.mouse(map.surface.node()), map); + var choice = iD.geo.chooseIndex(datum, d3.mouse(map.surface.node()), map); node = iD.Node({ loc: choice.loc }); history.perform( diff --git a/js/id/modes/draw_line.js b/js/id/modes/draw_line.js index 95719a6a9..3b3892aa6 100644 --- a/js/id/modes/draw_line.js +++ b/js/id/modes/draw_line.js @@ -80,7 +80,7 @@ iD.modes.DrawLine = function(wayId, direction) { datum.id = datum.way; choice = datum; } else { - choice = iD.util.geo.chooseIndex(datum, d3.mouse(surface.node()), map); + choice = iD.geo.chooseIndex(datum, d3.mouse(surface.node()), map); } history.replace( diff --git a/js/id/modes/select.js b/js/id/modes/select.js index 2f203b450..3a3750d36 100644 --- a/js/id/modes/select.js +++ b/js/id/modes/select.js @@ -116,7 +116,7 @@ iD.modes.Select = function(entity, initial) { var datum = d3.select(d3.event.target).datum(); if (datum instanceof iD.Entity && (datum.geometry() === 'area' || datum.geometry() === 'line')) { - var choice = iD.util.geo.chooseIndex(datum, + var choice = iD.geo.chooseIndex(datum, d3.mouse(mode.map.surface.node()), mode.map), node = iD.Node({ loc: choice.loc }); diff --git a/js/id/renderer/map.js b/js/id/renderer/map.js index 7e74d7b66..43785e3da 100644 --- a/js/id/renderer/map.js +++ b/js/id/renderer/map.js @@ -270,7 +270,7 @@ iD.Map = function() { map.centerEase = function(loc) { var from = map.center().slice(), t = 0; d3.timer(function() { - map.center(iD.util.geo.interp(from, loc, (t += 1) / 10)); + map.center(iD.geo.interp(from, loc, (t += 1) / 10)); return t == 10; }, 20); }; diff --git a/js/id/svg.js b/js/id/svg.js index e2b1dc2ad..a3b174ddb 100644 --- a/js/id/svg.js +++ b/js/id/svg.js @@ -1,7 +1,7 @@ iD.svg = { RoundProjection: function (projection) { return function (d) { - return iD.util.geo.roundCoords(projection(d)); + return iD.geo.roundCoords(projection(d)); }; }, diff --git a/js/id/svg/midpoints.js b/js/id/svg/midpoints.js index 7c7c7a0c3..a04526f83 100644 --- a/js/id/svg/midpoints.js +++ b/js/id/svg/midpoints.js @@ -11,9 +11,9 @@ iD.svg.Midpoints = function(projection) { for (var j = 0; j < entity.nodes.length - 1; j++) { var a = projection(entity.nodes[j].loc); var b = projection(entity.nodes[j + 1].loc); - if (iD.util.geo.dist(a, b) > 40) { + if (iD.geo.dist(a, b) > 40) { midpoints.push({ - loc: iD.util.geo.interp(entity.nodes[j].loc, entity.nodes[j + 1].loc, 0.5), + loc: iD.geo.interp(entity.nodes[j].loc, entity.nodes[j + 1].loc, 0.5), way: entity.id, index: j + 1, midpoint: true diff --git a/js/id/util.js b/js/id/util.js index b3ae537ec..2e6e3453d 100644 --- a/js/id/util.js +++ b/js/id/util.js @@ -70,79 +70,3 @@ iD.util.prefixCSSProperty = function(property) { return false; }; - -iD.util.geo = {}; - -iD.util.geo.roundCoords = function(c) { - return [Math.floor(c[0]), Math.floor(c[1])]; -}; - -iD.util.geo.interp = function(p1, p2, t) { - return [p1[0] + (p2[0] - p1[0]) * t, - p1[1] + (p2[1] - p1[1]) * t]; -}; - -iD.util.geo.dist = function(a, b) { - return Math.sqrt(Math.pow(a[0] - b[0], 2) + - Math.pow(a[1] - b[1], 2)); -}; - -iD.util.geo.chooseIndex = function(way, point, map) { - var dist = iD.util.geo.dist, - projNodes = way.nodes.map(function(n) { - return map.projection(n.loc); - }); - - for (var i = 0, changes = []; i < projNodes.length - 1; i++) { - changes[i] = - (dist(projNodes[i], point) + dist(point, projNodes[i + 1])) / - dist(projNodes[i], projNodes[i + 1]); - } - - var idx = _.indexOf(changes, _.min(changes)), - ratio = dist(projNodes[idx], point) / dist(projNodes[idx], projNodes[idx + 1]), - loc = iD.util.geo.interp(way.nodes[idx].loc, way.nodes[idx + 1].loc, ratio); - - return { - index: idx + 1, - loc: loc - }; -}; - -// Return whether point is contained in polygon. -// -// `point` should be a 2-item array of coordinates. -// `polygon` should be an array of 2-item arrays of coordinates. -// -// From https://github.com/substack/point-in-polygon. -// ray-casting algorithm based on -// http://www.ecse.rpi.edu/Homepages/wrf/Research/Short_Notes/pnpoly.html -// -iD.util.geo.pointInPolygon = function(point, polygon) { - var x = point[0], - y = point[1], - inside = false; - - for (var i = 0, j = polygon.length - 1; i < polygon.length; j = i++) { - var xi = polygon[i][0], yi = polygon[i][1]; - var xj = polygon[j][0], yj = polygon[j][1]; - - var intersect = ((yi > y) != (yj > y)) && - (x < (xj - xi) * (y - yi) / (yj - yi) + xi); - if (intersect) inside = !inside; - } - - return inside; -}; - -iD.util.geo.polygonContainsPolygon = function(outer, inner) { - return _.every(inner, function (point) { - return iD.util.geo.pointInPolygon(point, outer); - }); -}; - -iD.util.geo.polygonIntersectsPolygon = function(outer, inner) { - return _.some(inner, function (point) { - return iD.util.geo.pointInPolygon(point, outer); - }); -}; diff --git a/test/spec/util.js b/test/spec/util.js index 2cfa8dc16..6c12ccc0f 100644 --- a/test/spec/util.js +++ b/test/spec/util.js @@ -26,21 +26,21 @@ describe('Util', function() { describe('geo', function() { describe('#roundCoords', function() { - expect(iD.util.geo.roundCoords([0.1, 1])).to.eql([0, 1]); - expect(iD.util.geo.roundCoords([0, 1])).to.eql([0, 1]); - expect(iD.util.geo.roundCoords([0, 1.1])).to.eql([0, 1]); + expect(iD.geo.roundCoords([0.1, 1])).to.eql([0, 1]); + expect(iD.geo.roundCoords([0, 1])).to.eql([0, 1]); + expect(iD.geo.roundCoords([0, 1.1])).to.eql([0, 1]); }); describe('#interp', function() { it('interpolates halfway', function() { var a = [0, 0], b = [10, 10]; - expect(iD.util.geo.interp(a, b, 0.5)).to.eql([5, 5]); + expect(iD.geo.interp(a, b, 0.5)).to.eql([5, 5]); }); it('interpolates to one side', function() { var a = [0, 0], b = [10, 10]; - expect(iD.util.geo.interp(a, b, 0)).to.eql([0, 0]); + expect(iD.geo.interp(a, b, 0)).to.eql([0, 0]); }); }); @@ -48,17 +48,17 @@ describe('Util', function() { it('distance between two same points is zero', function() { var a = [0, 0], b = [0, 0]; - expect(iD.util.geo.dist(a, b)).to.eql(0); + expect(iD.geo.dist(a, b)).to.eql(0); }); it('a straight 10 unit line is 10', function() { var a = [0, 0], b = [10, 0]; - expect(iD.util.geo.dist(a, b)).to.eql(10); + expect(iD.geo.dist(a, b)).to.eql(10); }); it('a pythagorean triangle is right', function() { var a = [0, 0], b = [4, 3]; - expect(iD.util.geo.dist(a, b)).to.eql(5); + expect(iD.geo.dist(a, b)).to.eql(5); }); }); @@ -66,7 +66,7 @@ describe('Util', function() { it('says a point in a polygon is on a polygon', function() { var poly = [[0, 0], [0, 1], [1, 1], [1, 0], [0, 0]]; var point = [0.5, 0.5]; - expect(iD.util.geo.pointInPolygon(point, poly)).to.be.true; + expect(iD.geo.pointInPolygon(point, poly)).to.be.true; }); it('says a point outside of a polygon is outside', function() { var poly = [ @@ -76,7 +76,7 @@ describe('Util', function() { [1, 0], [0, 0]]; var point = [0.5, 1.5]; - expect(iD.util.geo.pointInPolygon(point, poly)).to.be.false; + expect(iD.geo.pointInPolygon(point, poly)).to.be.false; }); }); @@ -84,12 +84,12 @@ describe('Util', function() { it('says a polygon in a polygon is in', function() { var outer = [[0, 0], [0, 3], [3, 3], [3, 0], [0, 0]]; var inner = [[1, 1], [1, 2], [2, 2], [2, 1], [1, 1]]; - expect(iD.util.geo.polygonContainsPolygon(outer, inner)).to.be.true; + expect(iD.geo.polygonContainsPolygon(outer, inner)).to.be.true; }); it('says a polygon outside of a polygon is out', function() { var outer = [[0, 0], [0, 3], [3, 3], [3, 0], [0, 0]]; var inner = [[1, 1], [1, 9], [2, 2], [2, 1], [1, 1]]; - expect(iD.util.geo.polygonContainsPolygon(outer, inner)).to.be.false; + expect(iD.geo.polygonContainsPolygon(outer, inner)).to.be.false; }); }); @@ -97,19 +97,19 @@ describe('Util', function() { it('says a polygon in a polygon intersects it', function() { var outer = [[0, 0], [0, 3], [3, 3], [3, 0], [0, 0]]; var inner = [[1, 1], [1, 2], [2, 2], [2, 1], [1, 1]]; - expect(iD.util.geo.polygonIntersectsPolygon(outer, inner)).to.be.true; + expect(iD.geo.polygonIntersectsPolygon(outer, inner)).to.be.true; }); it('says a polygon that partially intersects does', function() { var outer = [[0, 0], [0, 3], [3, 3], [3, 0], [0, 0]]; var inner = [[-1, -1], [1, 2], [2, 2], [2, 1], [1, 1]]; - expect(iD.util.geo.polygonIntersectsPolygon(outer, inner)).to.be.true; + expect(iD.geo.polygonIntersectsPolygon(outer, inner)).to.be.true; }); it('says totally disjoint polygons do not intersect', function() { var outer = [[0, 0], [0, 3], [3, 3], [3, 0], [0, 0]]; var inner = [[-1, -1], [-1, -2], [-2, -2], [-2, -1], [-1, -1]]; - expect(iD.util.geo.polygonIntersectsPolygon(outer, inner)).to.be.false; + expect(iD.geo.polygonIntersectsPolygon(outer, inner)).to.be.false; }); }); }); From ce7e6ff8311f32acbeb2823f02413245eb8f55e1 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Tue, 22 Jan 2013 18:23:18 -0500 Subject: [PATCH 13/32] Consistent iD prefix in tests --- test/spec/connection.js | 2 +- test/spec/format/geojson.js | 2 +- test/spec/format/xml.js | 2 +- test/spec/renderer/background.js | 2 +- test/spec/renderer/hash.js | 2 +- test/spec/renderer/map.js | 2 +- test/spec/util.js | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) diff --git a/test/spec/connection.js b/test/spec/connection.js index 70f018322..dbf7722bb 100644 --- a/test/spec/connection.js +++ b/test/spec/connection.js @@ -1,4 +1,4 @@ -describe('Connection', function() { +describe('iD.Connection', function() { var c; beforeEach(function() { diff --git a/test/spec/format/geojson.js b/test/spec/format/geojson.js index f9b58f66f..e17c8573b 100644 --- a/test/spec/format/geojson.js +++ b/test/spec/format/geojson.js @@ -1,4 +1,4 @@ -describe('GeoJSON', function() { +describe('iD.format.GeoJSON', function() { describe('#mapping', function() { it('should be able to map a node to geojson', function() { diff --git a/test/spec/format/xml.js b/test/spec/format/xml.js index db0e3226b..e70ed464f 100644 --- a/test/spec/format/xml.js +++ b/test/spec/format/xml.js @@ -1,4 +1,4 @@ -describe('XML', function() { +describe('iD.format.XML', function() { var node = iD.Node({ id: 'n-1', type: 'node', loc: [-77, 38] }), way = iD.Way({ id: 'w-1', type: 'way', nodes: [] }); diff --git a/test/spec/renderer/background.js b/test/spec/renderer/background.js index 0d7b0de10..6ba871d11 100644 --- a/test/spec/renderer/background.js +++ b/test/spec/renderer/background.js @@ -1,4 +1,4 @@ -describe('Background', function() { +describe('iD.Background', function() { var c, d; beforeEach(function() { diff --git a/test/spec/renderer/hash.js b/test/spec/renderer/hash.js index 1304602f1..bece2e155 100644 --- a/test/spec/renderer/hash.js +++ b/test/spec/renderer/hash.js @@ -1,4 +1,4 @@ -describe("hash", function () { +describe("iD.Hash", function () { var hash, map, controller; beforeEach(function () { diff --git a/test/spec/renderer/map.js b/test/spec/renderer/map.js index 611a2d1ff..a0e89f578 100644 --- a/test/spec/renderer/map.js +++ b/test/spec/renderer/map.js @@ -1,4 +1,4 @@ -describe('Map', function() { +describe('iD.Map', function() { var container, map; beforeEach(function() { diff --git a/test/spec/util.js b/test/spec/util.js index 6c12ccc0f..c2c645673 100644 --- a/test/spec/util.js +++ b/test/spec/util.js @@ -1,4 +1,4 @@ -describe('Util', function() { +describe('iD.Util', function() { var util; it('#trueObj', function() { From 9331cd3def746f4118ea596cc1444a55cac444f5 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Tue, 22 Jan 2013 18:23:53 -0500 Subject: [PATCH 14/32] Delete obsolete tests --- test/spec/integration/create_poi.js | 22 ---------------------- test/spec/renderer/style.js | 17 ----------------- 2 files changed, 39 deletions(-) delete mode 100644 test/spec/integration/create_poi.js delete mode 100644 test/spec/renderer/style.js diff --git a/test/spec/integration/create_poi.js b/test/spec/integration/create_poi.js deleted file mode 100644 index f1d08d011..000000000 --- a/test/spec/integration/create_poi.js +++ /dev/null @@ -1,22 +0,0 @@ -describe("iD", function () { - var container, editor; - - beforeEach(function() { - container = d3.select('body').append('div'); - editor = iD() - editor.map().background.source(null); - editor.call(container); - }); - - afterEach(function() { - container.remove(); - }); - - it("allows an editor to add a place", function () { - // click 'Add Place' - // click on map - // select tags - // save - // check uploaded XML - }); -}); diff --git a/test/spec/renderer/style.js b/test/spec/renderer/style.js deleted file mode 100644 index dad18866b..000000000 --- a/test/spec/renderer/style.js +++ /dev/null @@ -1,17 +0,0 @@ -describe('iD.Style', function() { - describe('#waystack', function() { - it('stacks bridges over non-bridges', function() { - var a = { tags: { bridge: 'yes' } }, - b = { tags: {} }; - expect(iD.Style.waystack(a, b)).to.equal(1); - expect(iD.Style.waystack(b, a)).to.equal(-1); - }); - - it('stacks layers', function() { - var a = { tags: { layer: 1 } }, - b = { tags: { layer: 0 } }; - expect(iD.Style.waystack(a, b)).to.equal(1); - expect(iD.Style.waystack(b, a)).to.equal(-1); - }); - }); -}); From 80e2c3d7be802cadf441aeeeacd258df0aa541ed Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Tue, 22 Jan 2013 18:37:38 -0500 Subject: [PATCH 15/32] Don't make the non-selected layers appear to be disabled --- css/app.css | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/css/app.css b/css/app.css index 3c9f4fbf9..56e87ccd7 100644 --- a/css/app.css +++ b/css/app.css @@ -180,17 +180,13 @@ ul li { list-style: none;} ul.toggle-list li a { font-weight: bold; - color: #c1c1c1; + color: #333; padding: 10px; border-top: 1px solid white; display:block; border-top: 1px solid rgba(0, 0, 0, .5); } -ul.toggle-list li a.selected { - color: #333; -} - ul.toggle-list .icon { float: left; margin-right: 5px; From df061483554439183e03b19603fc8d5f9dbebe08 Mon Sep 17 00:00:00 2001 From: Tom MacWright Date: Tue, 22 Jan 2013 18:56:09 -0500 Subject: [PATCH 16/32] Look farther up for up tiles --- js/id/renderer/background.js | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/js/id/renderer/background.js b/js/id/renderer/background.js index f861d2226..c63362356 100644 --- a/js/id/renderer/background.js +++ b/js/id/renderer/background.js @@ -38,6 +38,12 @@ iD.Background = function() { return Math.ceil(256 * Math.pow(2, z - d[2])) / 256; } + function lookUp(d) { + for (var up = -1; up > -d[2]; up--) { + if (cache[atZoom(d, up)] !== false) return atZoom(d, up); + } + } + // derive the tiles onscreen, remove those offscreen and position tiles // correctly for the currentstate of `projection` function background() { @@ -70,16 +76,13 @@ iD.Background = function() { // if this tile has not finished, req the one above } else if (cache[d] === undefined && + lookUp(d)) { - // but the tile above is in the cache - cache[atZoom(d, -1)] && - - // and another tile has not already requested the - // tile above - !ups[atZoom(d, -1)]) { - - ups[atZoom(d, -1)] = true; - tiles.push(atZoom(d, -1)); + var upTile = lookUp(d); + if (!ups[upTile]) { + ups[upTile] = true; + tiles.push(upTile); + } // if this tile has not yet completed, try keeping the // tiles below it @@ -98,8 +101,6 @@ iD.Background = function() { .selectAll('img') .data(tiles, function(d) { return d; }); - image.exit().remove(); - function load(d) { cache[d.slice(0, 3)] = true; d3.select(this).on('load', null); @@ -116,6 +117,8 @@ iD.Background = function() { .on('error', error) .on('load', load); + image.exit().remove(); + image.style(transformProp, function(d) { var _ts = 256 * Math.pow(2, z - d[2]); var scale = tileSize(d, z); From 29d608970bb0573bee1abfa2b0c41c7cce9b6b8e Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Wed, 23 Jan 2013 10:03:05 -0500 Subject: [PATCH 17/32] UnjoinNode action (fixes #442) --- js/id/actions/unjoin_node.js | 40 +++++++++++++++++ js/id/modes/select.js | 5 +++ js/id/ui/inspector.js | 11 ++++- test/index.html | 2 + test/index_packaged.html | 1 + test/spec/actions/unjoin_node.js | 77 ++++++++++++++++++++++++++++++++ 6 files changed, 135 insertions(+), 1 deletion(-) create mode 100644 js/id/actions/unjoin_node.js create mode 100644 test/spec/actions/unjoin_node.js diff --git a/js/id/actions/unjoin_node.js b/js/id/actions/unjoin_node.js new file mode 100644 index 000000000..4bc7a5ca7 --- /dev/null +++ b/js/id/actions/unjoin_node.js @@ -0,0 +1,40 @@ +// Unjoin the ways at the given node. +// +// For testing convenience, accepts an ID to assign to the (first) new node. +// Normally, this will be undefined and the way will automatically +// be assigned a new ID. +// +// Reference: +// https://github.com/openstreetmap/potlatch2/blob/master/net/systemeD/halcyon/connection/actions/UnjoinNodeAction.as +// https://github.com/openstreetmap/josm/blob/mirror/src/org/openstreetmap/josm/actions/UnGlueAction.java +// +iD.actions.UnjoinNode = function(nodeId, newNodeId) { + var action = function(graph) { + if (!action.permitted(graph)) + return graph; + + var node = graph.entity(nodeId); + + graph.parentWays(node).forEach(function(parent, i) { + if (i === 0) + return; + + var index = parent.nodes.indexOf(nodeId), + newNode = iD.Node({id: newNodeId, loc: node.loc, tags: node.tags}), + nodes = parent.nodes.slice(); + + nodes.splice(index, 1, newNode.id); + + graph = graph.replace(newNode); + graph = graph.replace(parent.update({nodes: nodes})); + }); + + return graph; + }; + + action.permitted = function(graph) { + return graph.parentWays(graph.entity(nodeId)).length >= 2; + }; + + return action; +}; diff --git a/js/id/modes/select.js b/js/id/modes/select.js index 3a3750d36..2b92cfedd 100644 --- a/js/id/modes/select.js +++ b/js/id/modes/select.js @@ -85,6 +85,11 @@ iD.modes.Select = function(entity, initial) { iD.actions.SplitWay(d.id), 'split a way'); + }).on('unjoin', function(d) { + mode.history.perform( + iD.actions.UnjoinNode(d.id), + 'unjoined ways'); + }).on('remove', function() { remove(); diff --git a/js/id/ui/inspector.js b/js/id/ui/inspector.js index be9db53f0..058f318df 100644 --- a/js/id/ui/inspector.js +++ b/js/id/ui/inspector.js @@ -1,6 +1,6 @@ iD.ui.inspector = function() { var event = d3.dispatch('changeTags', 'reverseWay', - 'update', 'remove', 'close', 'splitWay'), + 'update', 'remove', 'close', 'splitWay', 'unjoin'), taginfo = iD.taginfo(), initial = false, tagList; @@ -58,8 +58,10 @@ iD.ui.inspector = function() { function drawButtons(selection) { var entity = selection.datum(); + var inspectorButtonWrap = selection.append('div') .attr('class','button-wrap joined fl'); + var inspectorButton1 = inspectorButtonWrap.append('button') .attr('class', 'apply col6 action') .on('click', apply); @@ -80,17 +82,24 @@ iD.ui.inspector = function() { .attr('href', 'http://www.openstreetmap.org/browse/' + entity.type + '/' + entity.osmId()) .attr('target', '_blank') .text('View on OSM'); + if (entity.type === 'way') { minorButtons.append('a') .attr('href', '#') .text('Reverse Direction') .on('click', function() { event.reverseWay(entity); }); } + if (entity.geometry() === 'vertex') { minorButtons.append('a') .attr('href', '#') .text('Split Way') .on('click', function() { event.splitWay(entity); }); + + minorButtons.append('a') + .attr('href', '#') + .text('Unjoin') + .on('click', function() { event.unjoin(entity); }); } } diff --git a/test/index.html b/test/index.html index 4099f2786..6df3944d9 100644 --- a/test/index.html +++ b/test/index.html @@ -79,6 +79,7 @@ + @@ -140,6 +141,7 @@ + diff --git a/test/index_packaged.html b/test/index_packaged.html index 8def4c0da..1432e6686 100644 --- a/test/index_packaged.html +++ b/test/index_packaged.html @@ -44,6 +44,7 @@ + diff --git a/test/spec/actions/unjoin_node.js b/test/spec/actions/unjoin_node.js new file mode 100644 index 000000000..247239f5c --- /dev/null +++ b/test/spec/actions/unjoin_node.js @@ -0,0 +1,77 @@ +describe("iD.actions.UnjoinNode", function () { + describe("#permitted", function () { + it("returns false for a node shared by less than two ways", function () { + var graph = iD.Graph({'a': iD.Node()}); + + expect(iD.actions.UnjoinNode('a').permitted(graph)).to.equal(false); + }); + + it("returns true for a node shared by two or more ways", function () { + // a ---- b ---- c + // | + // d + var graph = iD.Graph({ + 'a': iD.Node({id: 'a'}), + 'b': iD.Node({id: 'b'}), + 'c': iD.Node({id: 'c'}), + 'd': iD.Node({id: 'd'}), + '-': iD.Way({id: '-', nodes: ['a', 'b', 'c']}), + '|': iD.Way({id: '|', nodes: ['d', 'b']}) + }); + + expect(iD.actions.UnjoinNode('b').permitted(graph)).to.equal(true); + }); + }); + + it("replaces the node with a new node in all but the first way", function () { + // Situation: + // a ---- b ---- c + // | + // d + // Split at b. + // + // Expected result: + // a ---- b ---- c + // + // e + // | + // d + // + var graph = iD.Graph({ + 'a': iD.Node({id: 'a'}), + 'b': iD.Node({id: 'b'}), + 'c': iD.Node({id: 'c'}), + 'd': iD.Node({id: 'd'}), + '-': iD.Way({id: '-', nodes: ['a', 'b', 'c']}), + '|': iD.Way({id: '|', nodes: ['d', 'b']}) + }); + + graph = iD.actions.UnjoinNode('b', 'e')(graph); + + expect(graph.entity('-').nodes).to.eql(['a', 'b', 'c']); + expect(graph.entity('|').nodes).to.eql(['d', 'e']); + }); + + it("copies location and tags to the new nodes", function () { + var tags = {highway: 'traffic_signals'}, + loc = [1, 2], + graph = iD.Graph({ + 'a': iD.Node({id: 'a'}), + 'b': iD.Node({id: 'b', loc: loc, tags: tags}), + 'c': iD.Node({id: 'c'}), + 'd': iD.Node({id: 'd'}), + '-': iD.Way({id: '-', nodes: ['a', 'b', 'c']}), + '|': iD.Way({id: '|', nodes: ['d', 'b']}) + }); + + graph = iD.actions.UnjoinNode('b', 'e')(graph); + + // Immutable loc => should be shared by identity. + expect(graph.entity('b').loc).to.equal(loc); + expect(graph.entity('e').loc).to.equal(loc); + + // Immutable tags => should be shared by identity. + expect(graph.entity('b').tags).to.equal(tags); + expect(graph.entity('e').tags).to.equal(tags); + }); +}); From 8e92fffa1a824a3cfbc54b879c41b01965a86238 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Wed, 23 Jan 2013 10:05:14 -0500 Subject: [PATCH 18/32] Freeze loc --- js/id/graph/entity.js | 1 + 1 file changed, 1 insertion(+) diff --git a/js/id/graph/entity.js b/js/id/graph/entity.js index 45d0d0c5c..254607a7b 100644 --- a/js/id/graph/entity.js +++ b/js/id/graph/entity.js @@ -50,6 +50,7 @@ iD.Entity.prototype = { Object.freeze(this); Object.freeze(this.tags); + if (this.loc) Object.freeze(this.loc); if (this.nodes) Object.freeze(this.nodes); if (this.members) Object.freeze(this.members); } From b47bb517cf5312d0026e52a359d393bf820943a0 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Wed, 23 Jan 2013 10:06:53 -0500 Subject: [PATCH 19/32] Sync packaged tests --- test/index_packaged.html | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/index_packaged.html b/test/index_packaged.html index 1432e6686..6dc1391a9 100644 --- a/test/index_packaged.html +++ b/test/index_packaged.html @@ -30,6 +30,8 @@ + + @@ -77,6 +79,11 @@ + + + + + From 6f0b715c9f2d145da2760d41ec84fe8018ac89ee Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Wed, 23 Jan 2013 10:35:01 -0500 Subject: [PATCH 20/32] Include new file --- index.html | 1 + 1 file changed, 1 insertion(+) diff --git a/index.html b/index.html index 5853a8a50..93c5171a6 100644 --- a/index.html +++ b/index.html @@ -84,6 +84,7 @@ + From 75ea4d318e21b239877557fb01d5dbebf158a843 Mon Sep 17 00:00:00 2001 From: Tom MacWright Date: Wed, 23 Jan 2013 11:22:23 -0500 Subject: [PATCH 21/32] Test centerZoom --- test/spec/renderer/map.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/spec/renderer/map.js b/test/spec/renderer/map.js index a0e89f578..efe2c54b0 100644 --- a/test/spec/renderer/map.js +++ b/test/spec/renderer/map.js @@ -52,6 +52,15 @@ describe('iD.Map', function() { }); }); + describe('#centerZoom', function() { + it('gets and sets center and zoom', function() { + expect(map.centerZoom([20, 25], 4)).to.equal(map); + expect(map.center()[0]).to.be.closeTo(20, 0.5); + expect(map.center()[1]).to.be.closeTo(25, 0.5); + expect(map.zoom()).to.be.equal(4); + }); + }); + describe('#extent', function() { it('gets and sets extent', function() { map.size([100, 100]) From 5c65af512d9907ac60b68a797ed4a817c10b3d4b Mon Sep 17 00:00:00 2001 From: Tom MacWright Date: Wed, 23 Jan 2013 11:35:31 -0500 Subject: [PATCH 22/32] Test and fixup centerEase --- js/id/renderer/map.js | 1 + test/spec/renderer/map.js | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/js/id/renderer/map.js b/js/id/renderer/map.js index 43785e3da..f5f2a2dd5 100644 --- a/js/id/renderer/map.js +++ b/js/id/renderer/map.js @@ -273,6 +273,7 @@ iD.Map = function() { map.center(iD.geo.interp(from, loc, (t += 1) / 10)); return t == 10; }, 20); + return map; }; map.extent = function(_) { diff --git a/test/spec/renderer/map.js b/test/spec/renderer/map.js index efe2c54b0..894451299 100644 --- a/test/spec/renderer/map.js +++ b/test/spec/renderer/map.js @@ -42,6 +42,12 @@ describe('iD.Map', function() { }); }); + describe('#minzoom', function() { + it('is zero by default', function() { + expect(map.minzoom()).to.equal(0); + }); + }); + describe('#center', function() { it('gets and sets center', function() { expect(map.center([0, 0])).to.equal(map); @@ -52,6 +58,18 @@ describe('iD.Map', function() { }); }); + describe('#centerEase', function() { + it('sets center', function(done) { + expect(map.center([10, 10])).to.equal(map); + expect(map.centerEase([20, 20])).to.equal(map); + window.setTimeout(function() { + expect(map.center()[0]).to.be.closeTo(20, 0.5); + expect(map.center()[1]).to.be.closeTo(20, 0.5); + done(); + }, 500); + }); + }); + describe('#centerZoom', function() { it('gets and sets center and zoom', function() { expect(map.centerZoom([20, 25], 4)).to.equal(map); From eefa22531c44f752a0d2ef96d31b81bf4e53e395 Mon Sep 17 00:00:00 2001 From: Tom MacWright Date: Wed, 23 Jan 2013 12:30:22 -0500 Subject: [PATCH 23/32] Simplify indentation, add blocking modals. Fixes #412 --- js/id/ui/loading.js | 4 +- js/id/ui/modal.js | 6 +-- js/id/ui/save.js | 101 +++++++++++++++++++------------------- test/spec/renderer/map.js | 2 +- 4 files changed, 57 insertions(+), 56 deletions(-) diff --git a/js/id/ui/loading.js b/js/id/ui/loading.js index 6a98a917a..23d5b7c2d 100644 --- a/js/id/ui/loading.js +++ b/js/id/ui/loading.js @@ -1,5 +1,5 @@ -iD.ui.loading = function(message) { - var modal = iD.ui.modal(); +iD.ui.loading = function(message, blocking) { + var modal = iD.ui.modal(blocking); var loadertext = modal.select('.content') .classed('loading-modal', true) diff --git a/js/id/ui/modal.js b/js/id/ui/modal.js index fed97a0a1..498f1e241 100644 --- a/js/id/ui/modal.js +++ b/js/id/ui/modal.js @@ -1,4 +1,4 @@ -iD.ui.modal = function() { +iD.ui.modal = function(blocking) { var animate = d3.select('div.modal').empty(); d3.select('div.modal').transition() @@ -9,7 +9,7 @@ iD.ui.modal = function() { .attr('class', 'shaded') .style('opacity', 0) .on('click.remove-modal', function() { - if (d3.event.target == this) d3.select(this).remove(); + if (d3.event.target == this && !blocking) d3.select(this).remove(); }); var modal = shaded.append('div') @@ -18,7 +18,7 @@ iD.ui.modal = function() { modal.append('button') .attr('class', 'icon remove close-modal') .on('click', function() { - shaded.remove(); + if (!blocking) shaded.remove(); }); modal.append('div') diff --git a/js/id/ui/save.js b/js/id/ui/save.js index afaab5023..c0036d334 100644 --- a/js/id/ui/save.js +++ b/js/id/ui/save.js @@ -14,60 +14,61 @@ iD.ui.save = function() { .placement('bottom')) .on('click', function() { - function commit(e) { - d3.select('.shaded').remove(); - var l = iD.ui.loading('Uploading changes to OpenStreetMap.'); - connection.putChangeset(history.changes(), e.comment, history.imagery_used(), function(err, changeset_id) { - l.remove(); - history.reset(); - map.flush().redraw(); - if (err) { - var desc = iD.ui.confirm() - .select('.description'); - desc.append('h2') - .text('An error occurred while trying to save'); - desc.append('p').text(err.responseText); - } else { - var modal = iD.ui.modal(); - modal.select('.content') - .classed('success-modal', true) - .datum({ - id: changeset_id, - comment: e.comment - }) - .call(iD.ui.success() - .on('cancel', function() { - modal.remove(); - })); - } - }); - } - - if (history.hasChanges()) { - connection.authenticate(function(err) { + function commit(e) { + d3.select('.shaded').remove(); + var l = iD.ui.loading('Uploading changes to OpenStreetMap.', true); + connection.putChangeset(history.changes(), e.comment, history.imagery_used(), function(err, changeset_id) { + l.remove(); + history.reset(); + map.flush().redraw(); + if (err) { + var desc = iD.ui.confirm() + .select('.description'); + desc.append('h2') + .text('An error occurred while trying to save'); + desc.append('p').text(err.responseText); + } else { var modal = iD.ui.modal(); - var changes = history.changes(); - changes.connection = connection; modal.select('.content') - .classed('commit-modal', true) - .datum(changes) - .call(iD.ui.commit() + .classed('success-modal', true) + .datum({ + id: changeset_id, + comment: e.comment + }) + .call(iD.ui.success() .on('cancel', function() { modal.remove(); - }) - .on('fix', function(d) { - map.extent(d.entity.extent(map.history().graph())); - if (map.zoom() > 19) map.zoom(19); - controller.enter(iD.modes.Select(d.entity)); - modal.remove(); - }) - .on('save', commit)); - }); - } else { - iD.ui.confirm().select('.description') - .append('h3').text('You don\'t have any changes to save.'); - } - }); + })); + } + }); + } + + if (history.hasChanges()) { + connection.authenticate(function(err) { + var modal = iD.ui.modal(); + var changes = history.changes(); + changes.connection = connection; + modal.select('.content') + .classed('commit-modal', true) + .datum(changes) + .call(iD.ui.commit() + .on('cancel', function() { + modal.remove(); + }) + .on('fix', function(d) { + map.extent(d.entity.extent(map.history().graph())); + if (map.zoom() > 19) map.zoom(19); + controller.enter(iD.modes.Select(d.entity)); + modal.remove(); + }) + .on('save', commit)); + }); + } else { + iD.ui.confirm().select('.description') + .append('h3').text('You don\'t have any changes to save.'); + } + + }); selection.append('span') .attr('class', 'count'); diff --git a/test/spec/renderer/map.js b/test/spec/renderer/map.js index 894451299..5070bdffa 100644 --- a/test/spec/renderer/map.js +++ b/test/spec/renderer/map.js @@ -66,7 +66,7 @@ describe('iD.Map', function() { expect(map.center()[0]).to.be.closeTo(20, 0.5); expect(map.center()[1]).to.be.closeTo(20, 0.5); done(); - }, 500); + }, 1000); }); }); From 292c916cb143a4de102d3eb3d44281c6e9cbb7ef Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Wed, 23 Jan 2013 10:36:02 -0500 Subject: [PATCH 24/32] Converting some actions to entity methods The guidelines here are: Entity methods: return a modified entity don't necessarily maintain whole-graph consistency Actions: return a modified graph always maintain whole-graph consistency call entity methods liberally generally don't call other actions --- index.html | 4 -- js/id/actions/add_relation_member.js | 9 ---- js/id/actions/add_way_node.js | 6 +-- js/id/actions/delete_node.js | 4 +- js/id/actions/delete_way.js | 4 +- js/id/actions/move_node.js | 3 +- js/id/actions/move_way.js | 2 +- js/id/actions/remove_relation_member.js | 9 ---- js/id/actions/remove_way_node.js | 17 -------- js/id/actions/reverse_way.js | 3 +- js/id/actions/split_way.js | 14 ++---- js/id/actions/unjoin_node.js | 7 +-- js/id/actions/update_relation_member.js | 9 ---- js/id/graph/node.js | 4 ++ js/id/graph/relation.js | 17 ++++++++ js/id/graph/way.js | 23 ++++++++++ test/index.html | 9 ---- test/index_packaged.html | 4 -- test/spec/actions/add_relation_member.js | 37 ---------------- test/spec/actions/add_way_node.js | 29 ------------- test/spec/actions/remove_relation_member.js | 8 ---- test/spec/actions/remove_way_node.js | 8 ---- test/spec/actions/update_relation_member.js | 8 ---- test/spec/graph/relation.js | 36 ++++++++++++++++ test/spec/graph/way.js | 48 +++++++++++++++++++++ 25 files changed, 143 insertions(+), 179 deletions(-) delete mode 100644 js/id/actions/add_relation_member.js delete mode 100644 js/id/actions/remove_relation_member.js delete mode 100644 js/id/actions/remove_way_node.js delete mode 100644 js/id/actions/update_relation_member.js delete mode 100644 test/spec/actions/add_relation_member.js delete mode 100644 test/spec/actions/add_way_node.js delete mode 100644 test/spec/actions/remove_relation_member.js delete mode 100644 test/spec/actions/remove_way_node.js delete mode 100644 test/spec/actions/update_relation_member.js diff --git a/index.html b/index.html index 93c5171a6..69f83f76b 100644 --- a/index.html +++ b/index.html @@ -71,7 +71,6 @@ - @@ -80,12 +79,9 @@ - - - diff --git a/js/id/actions/add_relation_member.js b/js/id/actions/add_relation_member.js deleted file mode 100644 index 45e24a0b0..000000000 --- a/js/id/actions/add_relation_member.js +++ /dev/null @@ -1,9 +0,0 @@ -iD.actions.AddRelationMember = function(relationId, member, index) { - return function(graph) { - var relation = graph.entity(relationId), - members = relation.members.slice(); - - members.splice((index === undefined) ? members.length : index, 0, member); - return graph.replace(relation.update({members: members})); - }; -}; diff --git a/js/id/actions/add_way_node.js b/js/id/actions/add_way_node.js index d685c3295..3e4fffcfc 100644 --- a/js/id/actions/add_way_node.js +++ b/js/id/actions/add_way_node.js @@ -1,10 +1,6 @@ // https://github.com/openstreetmap/potlatch2/blob/master/net/systemeD/halcyon/connection/actions/AddNodeToWayAction.as iD.actions.AddWayNode = function(wayId, nodeId, index) { return function(graph) { - var way = graph.entity(wayId), - node = graph.entity(nodeId), - nodes = way.nodes.slice(); - nodes.splice((index === undefined) ? nodes.length : index, 0, nodeId); - return graph.replace(way.update({nodes: nodes})); + return graph.replace(graph.entity(wayId).addNode(nodeId, index)); }; }; diff --git a/js/id/actions/delete_node.js b/js/id/actions/delete_node.js index a2d54fecb..d600bc5fc 100644 --- a/js/id/actions/delete_node.js +++ b/js/id/actions/delete_node.js @@ -5,12 +5,12 @@ iD.actions.DeleteNode = function(nodeId) { graph.parentWays(node) .forEach(function(parent) { - graph = iD.actions.RemoveWayNode(parent.id, nodeId)(graph); + graph = graph.replace(parent.removeNode(nodeId)); }); graph.parentRelations(node) .forEach(function(parent) { - graph = iD.actions.RemoveRelationMember(parent.id, nodeId)(graph); + graph = graph.replace(parent.removeMember(nodeId)); }); return graph.remove(node); diff --git a/js/id/actions/delete_way.js b/js/id/actions/delete_way.js index c2065a756..19e9e7c2a 100644 --- a/js/id/actions/delete_way.js +++ b/js/id/actions/delete_way.js @@ -5,7 +5,7 @@ iD.actions.DeleteWay = function(wayId) { graph.parentRelations(way) .forEach(function(parent) { - graph = iD.actions.RemoveRelationMember(parent.id, wayId)(graph); + graph = graph.replace(parent.removeMember(wayId)); }); way.nodes.forEach(function (nodeId) { @@ -15,7 +15,7 @@ iD.actions.DeleteWay = function(wayId) { // can be deleted on earlier iterations of this loop. if (!node) return; - graph = iD.actions.RemoveWayNode(wayId, nodeId)(graph); + graph = graph.replace(way.removeNode(nodeId)); if (!graph.parentWays(node).length && !graph.parentRelations(node).length) { diff --git a/js/id/actions/move_node.js b/js/id/actions/move_node.js index 9204fcb67..433242ff6 100644 --- a/js/id/actions/move_node.js +++ b/js/id/actions/move_node.js @@ -2,7 +2,6 @@ // https://github.com/openstreetmap/potlatch2/blob/master/net/systemeD/halcyon/connection/actions/MoveNodeAction.as iD.actions.MoveNode = function(nodeId, loc) { return function(graph) { - var node = graph.entity(nodeId); - return graph.replace(node.update({loc: loc})); + return graph.replace(graph.entity(nodeId).move(loc)); }; }; diff --git a/js/id/actions/move_way.js b/js/id/actions/move_way.js index ea547cc44..09ab8fd50 100644 --- a/js/id/actions/move_way.js +++ b/js/id/actions/move_way.js @@ -6,7 +6,7 @@ iD.actions.MoveWay = function(wayId, delta, projection) { var node = graph.entity(id), start = projection(node.loc), end = projection.invert([start[0] + delta[0], start[1] + delta[1]]); - graph = iD.actions.MoveNode(id, end)(graph); + graph = graph.replace(node.move(end)); }); return graph; diff --git a/js/id/actions/remove_relation_member.js b/js/id/actions/remove_relation_member.js deleted file mode 100644 index e72dd7ca0..000000000 --- a/js/id/actions/remove_relation_member.js +++ /dev/null @@ -1,9 +0,0 @@ -iD.actions.RemoveRelationMember = function(relationId, memberId) { - return function(graph) { - var relation = graph.entity(relationId), - members = _.reject(relation.members, function(r) { - return r.id === memberId; - }); - return graph.replace(relation.update({members: members})); - }; -}; diff --git a/js/id/actions/remove_way_node.js b/js/id/actions/remove_way_node.js deleted file mode 100644 index 8c589a20c..000000000 --- a/js/id/actions/remove_way_node.js +++ /dev/null @@ -1,17 +0,0 @@ -iD.actions.RemoveWayNode = function(wayId, nodeId) { - return function(graph) { - var way = graph.entity(wayId), nodes; - // If this is the connecting node in a closed area - if (way.nodes.length > 1 && - _.indexOf(way.nodes, nodeId) === 0 && - _.lastIndexOf(way.nodes, nodeId) === way.nodes.length - 1) { - // Remove the node - nodes = _.without(way.nodes, nodeId); - // And reclose the way on the new first node. - nodes.push(nodes[0]); - } else { - nodes = _.without(way.nodes, nodeId); - } - return graph.replace(way.update({nodes: nodes})); - }; -}; diff --git a/js/id/actions/reverse_way.js b/js/id/actions/reverse_way.js index a845fe844..160017637 100644 --- a/js/id/actions/reverse_way.js +++ b/js/id/actions/reverse_way.js @@ -65,7 +65,8 @@ iD.actions.ReverseWay = function(wayId) { graph.parentRelations(way).forEach(function (relation) { relation.members.forEach(function (member, index) { if (member.id === way.id && (role = {forward: 'backward', backward: 'forward'}[member.role])) { - graph = iD.actions.UpdateRelationMember(relation.id, {role: role}, index)(graph); + relation = relation.updateMember({role: role}, index); + graph = graph.replace(relation); } }); }); diff --git a/js/id/actions/split_way.js b/js/id/actions/split_way.js index d716880cf..f6d502b97 100644 --- a/js/id/actions/split_way.js +++ b/js/id/actions/split_way.js @@ -37,11 +37,8 @@ iD.actions.SplitWay = function(nodeId, newWayId) { if (relation.isRestriction()) { var via = relation.memberByRole('via'); if (via && newWay.contains(via.id)) { - graph = iD.actions.UpdateRelationMember( - relation.id, - {id: newWay.id}, - relation.memberById(way.id).index - )(graph); + relation = relation.updateMember({id: newWay.id}, relation.memberById(way.id).index); + graph = graph.replace(relation); } } else { var role = relation.memberById(way.id).role, @@ -55,11 +52,8 @@ iD.actions.SplitWay = function(nodeId, newWayId) { } } - graph = iD.actions.AddRelationMember( - relation.id, - {id: newWay.id, type: 'way', role: role}, - i <= j ? i + 1 : i - )(graph); + relation = relation.addMember({id: newWay.id, type: 'way', role: role}, i <= j ? i + 1 : i); + graph = graph.replace(relation); } }); diff --git a/js/id/actions/unjoin_node.js b/js/id/actions/unjoin_node.js index 4bc7a5ca7..037c56137 100644 --- a/js/id/actions/unjoin_node.js +++ b/js/id/actions/unjoin_node.js @@ -20,13 +20,10 @@ iD.actions.UnjoinNode = function(nodeId, newNodeId) { return; var index = parent.nodes.indexOf(nodeId), - newNode = iD.Node({id: newNodeId, loc: node.loc, tags: node.tags}), - nodes = parent.nodes.slice(); - - nodes.splice(index, 1, newNode.id); + newNode = iD.Node({id: newNodeId, loc: node.loc, tags: node.tags}); graph = graph.replace(newNode); - graph = graph.replace(parent.update({nodes: nodes})); + graph = graph.replace(parent.updateNode(newNode.id, index)); }); return graph; diff --git a/js/id/actions/update_relation_member.js b/js/id/actions/update_relation_member.js deleted file mode 100644 index 29eb79afe..000000000 --- a/js/id/actions/update_relation_member.js +++ /dev/null @@ -1,9 +0,0 @@ -iD.actions.UpdateRelationMember = function(relationId, properties, index) { - return function(graph) { - var relation = graph.entity(relationId), - members = relation.members.slice(); - - members.splice(index, 1, _.extend({}, members[index], properties)); - return graph.replace(relation.update({members: members})); - }; -}; diff --git a/js/id/graph/node.js b/js/id/graph/node.js index d72bf6442..df9c253a4 100644 --- a/js/id/graph/node.js +++ b/js/id/graph/node.js @@ -17,5 +17,9 @@ _.extend(iD.Node.prototype, { geometry: function() { return this._poi ? 'point' : 'vertex'; + }, + + move: function(loc) { + return this.update({loc: loc}); } }); diff --git a/js/id/graph/relation.js b/js/id/graph/relation.js index 2dd325438..48a86c920 100644 --- a/js/id/graph/relation.js +++ b/js/id/graph/relation.js @@ -48,6 +48,23 @@ _.extend(iD.Relation.prototype, { } }, + addMember: function(member, index) { + var members = this.members.slice(); + members.splice(index === undefined ? members.length : index, 0, member); + return this.update({members: members}); + }, + + updateMember: function(member, index) { + var members = this.members.slice(); + members.splice(index, 1, _.extend({}, members[index], member)); + return this.update({members: members}); + }, + + removeMember: function(id) { + var members = _.reject(this.members, function(m) { return m.id === id; }); + return this.update({members: members}); + }, + isRestriction: function() { return !!(this.tags.type && this.tags.type.match(/^restriction:?/)); }, diff --git a/js/id/graph/way.js b/js/id/graph/way.js index 35834e761..eb3cf4087 100644 --- a/js/id/graph/way.js +++ b/js/id/graph/way.js @@ -60,5 +60,28 @@ _.extend(iD.Way.prototype, { geometry: function() { return this.isArea() ? 'area' : 'line'; + }, + + addNode: function(id, index) { + var nodes = this.nodes.slice(); + nodes.splice(index === undefined ? nodes.length : index, 0, id); + return this.update({nodes: nodes}); + }, + + updateNode: function(id, index) { + var nodes = this.nodes.slice(); + nodes.splice(index, 1, id); + return this.update({nodes: nodes}); + }, + + removeNode: function(id) { + var nodes = _.without(this.nodes, id); + + // Preserve circularity + if (this.nodes.length > 1 && this.first() === id && this.last() === id) { + nodes.push(nodes[0]); + } + + return this.update({nodes: nodes}); } }); diff --git a/test/index.html b/test/index.html index 6df3944d9..e7e6701a6 100644 --- a/test/index.html +++ b/test/index.html @@ -66,7 +66,6 @@ - @@ -75,12 +74,9 @@ - - - @@ -128,21 +124,16 @@ - - - - - diff --git a/test/index_packaged.html b/test/index_packaged.html index 6dc1391a9..5a013b45c 100644 --- a/test/index_packaged.html +++ b/test/index_packaged.html @@ -33,7 +33,6 @@ - @@ -42,12 +41,9 @@ - - - diff --git a/test/spec/actions/add_relation_member.js b/test/spec/actions/add_relation_member.js deleted file mode 100644 index aeae8e353..000000000 --- a/test/spec/actions/add_relation_member.js +++ /dev/null @@ -1,37 +0,0 @@ -describe("iD.actions.AddRelationMember", function () { - it("adds a member at the end of the relation", function () { - var relation = iD.Relation(), - graph = iD.Graph([relation]); - - graph = iD.actions.AddRelationMember(relation.id, {id: '1'})(graph); - - expect(graph.entity(relation.id).members).to.eql([{id: '1'}]); - }); - - it("adds a member at index 0", function () { - var relation = iD.Relation({members: [{id: '1'}]}), - graph = iD.Graph([relation]); - - graph = iD.actions.AddRelationMember(relation.id, {id: '2'}, 0)(graph); - - expect(graph.entity(relation.id).members).to.eql([{id: '2'}, {id: '1'}]); - }); - - it("adds a member at a positive index", function () { - var relation = iD.Relation({members: [{id: '1'}, {id: '3'}]}), - graph = iD.Graph([relation]); - - graph = iD.actions.AddRelationMember(relation.id, {id: '2'}, 1)(graph); - - expect(graph.entity(relation.id).members).to.eql([{id: '1'}, {id: '2'}, {id: '3'}]); - }); - - it("adds a member at a negative index", function () { - var relation = iD.Relation({members: [{id: '1'}, {id: '3'}]}), - graph = iD.Graph([relation]); - - graph = iD.actions.AddRelationMember(relation.id, {id: '2'}, -1)(graph); - - expect(graph.entity(relation.id).members).to.eql([{id: '1'}, {id: '2'}, {id: '3'}]); - }); -}); diff --git a/test/spec/actions/add_way_node.js b/test/spec/actions/add_way_node.js deleted file mode 100644 index 5c774e5e4..000000000 --- a/test/spec/actions/add_way_node.js +++ /dev/null @@ -1,29 +0,0 @@ -describe("iD.actions.AddWayNode", function () { - it("adds a node to the end of a way", function () { - var way = iD.Way(), - node = iD.Node({id: "n1"}), - graph = iD.actions.AddWayNode(way.id, node.id)(iD.Graph([way, node])); - expect(graph.entity(way.id).nodes).to.eql(["n1"]); - }); - - it("adds a node to a way at index 0", function () { - var way = iD.Way({nodes: ["n1", "n3"]}), - node = iD.Node({id: "n2"}), - graph = iD.actions.AddWayNode(way.id, node.id, 0)(iD.Graph([way, node])); - expect(graph.entity(way.id).nodes).to.eql(["n2", "n1", "n3"]); - }); - - it("adds a node to a way at a positive index", function () { - var way = iD.Way({nodes: ["n1", "n3"]}), - node = iD.Node({id: "n2"}), - graph = iD.actions.AddWayNode(way.id, node.id, 1)(iD.Graph([way, node])); - expect(graph.entity(way.id).nodes).to.eql(["n1", "n2", "n3"]); - }); - - it("adds a node to a way at a negative index", function () { - var way = iD.Way({nodes: ["n1", "n3"]}), - node = iD.Node({id: "n2"}), - graph = iD.actions.AddWayNode(way.id, node.id, -1)(iD.Graph([way, node])); - expect(graph.entity(way.id).nodes).to.eql(["n1", "n2", "n3"]); - }); -}); diff --git a/test/spec/actions/remove_relation_member.js b/test/spec/actions/remove_relation_member.js deleted file mode 100644 index d989b749b..000000000 --- a/test/spec/actions/remove_relation_member.js +++ /dev/null @@ -1,8 +0,0 @@ -describe("iD.actions.RemoveRelationMember", function () { - it("removes a member from a relation", function () { - var node = iD.Node(), - relation = iD.Way({members: [{ id: node.id }]}), - graph = iD.actions.RemoveRelationMember(relation.id, node.id)(iD.Graph([node, relation])); - expect(graph.entity(relation.id).members).to.eql([]); - }); -}); diff --git a/test/spec/actions/remove_way_node.js b/test/spec/actions/remove_way_node.js deleted file mode 100644 index 6e5751b76..000000000 --- a/test/spec/actions/remove_way_node.js +++ /dev/null @@ -1,8 +0,0 @@ -describe("iD.actions.RemoveWayNode", function () { - it("removes a node from a way", function () { - var node = iD.Node({id: "n1"}), - way = iD.Way({id: "w1", nodes: ["n1"]}), - graph = iD.actions.RemoveWayNode(way.id, node.id)(iD.Graph({n1: node, w1: way})); - expect(graph.entity(way.id).nodes).to.eql([]); - }); -}); diff --git a/test/spec/actions/update_relation_member.js b/test/spec/actions/update_relation_member.js deleted file mode 100644 index c85d72b34..000000000 --- a/test/spec/actions/update_relation_member.js +++ /dev/null @@ -1,8 +0,0 @@ -describe("iD.actions.UpdateRelationMember", function () { - it("updates the properties of the relation member at the specified index", function () { - var node = iD.Node(), - relation = iD.Relation({members: [{id: node.id, role: 'forward'}]}), - graph = iD.actions.UpdateRelationMember(relation.id, {role: 'backward'}, 0)(iD.Graph([node, relation])); - expect(graph.entity(relation.id).members).to.eql([{id: node.id, role: 'backward'}]); - }); -}); diff --git a/test/spec/graph/relation.js b/test/spec/graph/relation.js index 58b1767f4..d8619726a 100644 --- a/test/spec/graph/relation.js +++ b/test/spec/graph/relation.js @@ -104,6 +104,42 @@ describe('iD.Relation', function () { }); }); + describe("#addMember", function () { + it("adds a member at the end of the relation", function () { + var r = iD.Relation(); + expect(r.addMember({id: '1'}).members).to.eql([{id: '1'}]); + }); + + it("adds a member at index 0", function () { + var r = iD.Relation({members: [{id: '1'}]}); + expect(r.addMember({id: '2'}, 0).members).to.eql([{id: '2'}, {id: '1'}]); + }); + + it("adds a member at a positive index", function () { + var r = iD.Relation({members: [{id: '1'}, {id: '3'}]}); + expect(r.addMember({id: '2'}, 1).members).to.eql([{id: '1'}, {id: '2'}, {id: '3'}]); + }); + + it("adds a member at a negative index", function () { + var r = iD.Relation({members: [{id: '1'}, {id: '3'}]}); + expect(r.addMember({id: '2'}, -1).members).to.eql([{id: '1'}, {id: '2'}, {id: '3'}]); + }); + }); + + describe("#updateMember", function () { + it("updates the properties of the relation member at the specified index", function () { + var r = iD.Relation({members: [{role: 'forward'}]}); + expect(r.updateMember({role: 'backward'}, 0).members).to.eql([{role: 'backward'}]); + }); + }); + + describe("#removeMember", function () { + it("removes a member", function () { + var r = iD.Relation({members: [{id: 'a'}]}); + expect(r.removeMember('a').members).to.eql([]); + }); + }); + describe("#multipolygon", function () { specify("single polygon consisting of a single way", function () { var a = iD.Node(), diff --git a/test/spec/graph/way.js b/test/spec/graph/way.js index 37df6b648..0c70ea692 100644 --- a/test/spec/graph/way.js +++ b/test/spec/graph/way.js @@ -126,4 +126,52 @@ describe('iD.Way', function() { expect(iD.Way({tags: { area: 'yes' }}).geometry()).to.equal('area'); }); }); + + describe("#addNode", function () { + it("adds a node to the end of a way", function () { + var w = iD.Way(); + expect(w.addNode('a').nodes).to.eql(['a']); + }); + + it("adds a node to a way at index 0", function () { + var w = iD.Way({nodes: ['a', 'b']}); + expect(w.addNode('c', 0).nodes).to.eql(['c', 'a', 'b']); + }); + + it("adds a node to a way at a positive index", function () { + var w = iD.Way({nodes: ['a', 'b']}); + expect(w.addNode('c', 1).nodes).to.eql(['a', 'c', 'b']); + }); + + it("adds a node to a way at a negative index", function () { + var w = iD.Way({nodes: ['a', 'b']}); + expect(w.addNode('c', -1).nodes).to.eql(['a', 'c', 'b']); + }); + }); + + describe("#updateNode", function () { + it("updates the node id at the specified index", function () { + var w = iD.Way({nodes: ['a', 'b', 'c']}); + expect(w.updateNode('d', 1).nodes).to.eql(['a', 'd', 'c']); + }); + }); + + describe("#removeNode", function () { + it("removes the node", function () { + var a = iD.Node({id: 'a'}), + w = iD.Way({nodes: ['a']}); + + expect(w.removeNode('a').nodes).to.eql([]); + }); + + it("preserves circularity", function () { + var a = iD.Node({id: 'a'}), + b = iD.Node({id: 'b'}), + c = iD.Node({id: 'c'}), + d = iD.Node({id: 'd'}), + w = iD.Way({nodes: ['a', 'b', 'c', 'd', 'a']}); + + expect(w.removeNode('a').nodes).to.eql(['b', 'c', 'd', 'b']); + }); + }); }); From 278bb4a51c4026718e7d68403e345c833f8217d2 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Wed, 23 Jan 2013 12:08:25 -0500 Subject: [PATCH 25/32] Fix some cases where a degenerate way was created --- js/id/actions/delete_node.js | 7 +++++- js/id/graph/way.js | 4 ++++ js/id/modes/draw_area.js | 37 +++++++++++++++++++++----------- js/id/modes/draw_line.js | 33 +++++++++++++++++++--------- js/id/modes/select.js | 10 +++------ test/spec/actions/delete_node.js | 18 ++++++++++++++++ test/spec/graph/way.js | 25 +++++++++++++++++++++ 7 files changed, 103 insertions(+), 31 deletions(-) diff --git a/js/id/actions/delete_node.js b/js/id/actions/delete_node.js index d600bc5fc..a93129ce3 100644 --- a/js/id/actions/delete_node.js +++ b/js/id/actions/delete_node.js @@ -5,7 +5,12 @@ iD.actions.DeleteNode = function(nodeId) { graph.parentWays(node) .forEach(function(parent) { - graph = graph.replace(parent.removeNode(nodeId)); + parent = parent.removeNode(nodeId); + graph = graph.replace(parent); + + if (parent.isDegenerate()) { + graph = iD.actions.DeleteWay(parent.id)(graph); + } }); graph.parentRelations(node) diff --git a/js/id/graph/way.js b/js/id/graph/way.js index eb3cf4087..0de0e55a1 100644 --- a/js/id/graph/way.js +++ b/js/id/graph/way.js @@ -58,6 +58,10 @@ _.extend(iD.Way.prototype, { !this.tags.barrier); }, + isDegenerate: function() { + return _.uniq(this.nodes).length < (this.isArea() ? 3 : 2); + }, + geometry: function() { return this.isArea() ? 'area' : 'line'; }, diff --git a/js/id/modes/draw_area.js b/js/id/modes/draw_area.js index a90d13c9c..494d1affb 100644 --- a/js/id/modes/draw_area.js +++ b/js/id/modes/draw_area.js @@ -12,9 +12,8 @@ iD.modes.DrawArea = function(wayId) { history = mode.history, controller = mode.controller, way = history.graph().entity(wayId), - headId = (way.nodes.length == 1) ? - way.nodes[0] : - way.nodes[way.nodes.length - 2], + index = way.nodes.length - 1, + headId = way.nodes[index - 1], tailId = way.first(), node = iD.Node({loc: map.mouseCoordinates()}); @@ -24,12 +23,20 @@ iD.modes.DrawArea = function(wayId) { history.perform( iD.actions.AddNode(node), - iD.actions.AddWayNode(way.id, node.id, -1)); + iD.actions.AddWayNode(way.id, node.id, index)); surface.selectAll('.way, .node') .filter(function (d) { return d.id === wayId || d.id === node.id; }) .classed('active', true); + function ReplaceTemporaryNode(replacementId) { + return function(graph) { + graph = graph.replace(graph.entity(wayId).updateNode(replacementId, index)); + graph = graph.remove(node); + return graph; + } + } + function mousemove() { history.replace(iD.actions.MoveNode(node.id, map.mouseCoordinates())); } @@ -55,8 +62,7 @@ iD.modes.DrawArea = function(wayId) { } else if (datum.type === 'node' && datum.id !== node.id) { // connect the way to an existing node history.replace( - iD.actions.DeleteNode(node.id), - iD.actions.AddWayNode(way.id, datum.id, -1), + ReplaceTemporaryNode(datum.id), way.nodes.length > 2 ? 'added to an area' : ''); controller.enter(iD.modes.DrawArea(wayId)); @@ -77,13 +83,11 @@ iD.modes.DrawArea = function(wayId) { iD.actions.DeleteNode(node.id), iD.actions.DeleteNode(headId)); - if (history.graph().fetch(wayId).nodes.length === 2) { - history.replace( - iD.actions.DeleteNode(way.nodes[0]), - iD.actions.DeleteWay(wayId)); - controller.enter(iD.modes.Browse()); - } else { + if (history.graph().entity(wayId)) { controller.enter(iD.modes.DrawArea(wayId)); + } else { + // The way was deleted because it had too few nodes. + controller.enter(iD.modes.Browse()); } } @@ -95,8 +99,15 @@ iD.modes.DrawArea = function(wayId) { function ret() { d3.event.preventDefault(); + history.replace(iD.actions.DeleteNode(node.id)); - controller.enter(iD.modes.Select(way, true)); + + if (history.graph().entity(wayId)) { + controller.enter(iD.modes.Select(way, true)); + } else { + // The way was deleted because it had too few nodes. + controller.enter(iD.modes.Browse()); + } } surface diff --git a/js/id/modes/draw_line.js b/js/id/modes/draw_line.js index 3b3892aa6..b8fabeca5 100644 --- a/js/id/modes/draw_line.js +++ b/js/id/modes/draw_line.js @@ -13,7 +13,7 @@ iD.modes.DrawLine = function(wayId, direction) { controller = mode.controller, way = history.graph().entity(wayId), node = iD.Node({loc: map.mouseCoordinates()}), - index = (direction === 'forward') ? undefined : 0, + index = (direction === 'forward') ? way.nodes.length : 0, headId = (direction === 'forward') ? way.last() : way.first(), tailId = (direction === 'forward') ? way.first() : way.last(); @@ -35,6 +35,14 @@ iD.modes.DrawLine = function(wayId, direction) { .filter(function (d) { return d.id === wayId || d.id === node.id; }) .classed('active', true); + function ReplaceTemporaryNode(replacementId) { + return function(graph) { + graph = graph.replace(graph.entity(wayId).updateNode(replacementId, index)); + graph = graph.remove(node); + return graph; + } + } + function mousemove() { history.replace(iD.actions.MoveNode(node.id, map.mouseCoordinates())); } @@ -46,8 +54,7 @@ iD.modes.DrawLine = function(wayId, direction) { // connect the way in a loop if (way.nodes.length > 2) { history.replace( - iD.actions.DeleteNode(node.id), - iD.actions.AddWayNode(wayId, tailId, index), + ReplaceTemporaryNode(tailId), 'added to a line'); controller.enter(iD.modes.Select(way, true)); @@ -66,8 +73,7 @@ iD.modes.DrawLine = function(wayId, direction) { } else if (datum.type === 'node' && datum.id !== node.id) { // connect the way to an existing node history.replace( - iD.actions.DeleteNode(node.id), - iD.actions.AddWayNode(wayId, datum.id, index), + ReplaceTemporaryNode(datum.id), 'added to a line'); controller.enter(iD.modes.DrawLine(wayId, direction)); @@ -106,11 +112,11 @@ iD.modes.DrawLine = function(wayId, direction) { iD.actions.DeleteNode(node.id), iD.actions.DeleteNode(headId)); - if (history.graph().fetch(wayId).nodes.length === 0) { - history.replace(iD.actions.DeleteWay(wayId)); - controller.enter(iD.modes.Browse()); - } else { + if (history.graph().entity(wayId)) { controller.enter(iD.modes.DrawLine(wayId, direction)); + } else { + // The way was deleted because it had too few nodes. + controller.enter(iD.modes.Browse()); } } @@ -122,8 +128,15 @@ iD.modes.DrawLine = function(wayId, direction) { function ret() { d3.event.preventDefault(); + history.replace(iD.actions.DeleteNode(node.id)); - controller.enter(iD.modes.Select(way, true)); + + if (history.graph().entity(wayId)) { + controller.enter(iD.modes.Select(way, true)); + } else { + // The way was deleted because it had too few nodes. + controller.enter(iD.modes.Browse()); + } } surface diff --git a/js/id/modes/select.js b/js/id/modes/select.js index 2b92cfedd..d80d6fffd 100644 --- a/js/id/modes/select.js +++ b/js/id/modes/select.js @@ -15,13 +15,9 @@ iD.modes.Select = function(entity, initial) { iD.actions.DeleteWay(entity.id), 'deleted a way'); } else if (entity.type === 'node') { - var parents = mode.history.graph().parentWays(entity), - operations = [iD.actions.DeleteNode(entity.id)]; - parents.forEach(function(parent) { - if (_.uniq(parent.nodes).length === 1) operations.push(iD.actions.DeleteWay(parent.id)); - }); - mode.history.perform.apply(mode.history, - operations.concat(['deleted a node'])); + mode.history.perform( + iD.actions.DeleteNode(entity.id), + 'deleted a node'); } mode.controller.exit(); diff --git a/test/spec/actions/delete_node.js b/test/spec/actions/delete_node.js index a73f7e33f..93860117f 100644 --- a/test/spec/actions/delete_node.js +++ b/test/spec/actions/delete_node.js @@ -24,4 +24,22 @@ describe("iD.actions.DeleteNode", function () { graph = action(iD.Graph([node1, node2, relation])); expect(graph.entity(relation.id).members).to.eql([{ id: node2.id }]); }); + + it("deletes parent ways that would otherwise have less than two nodes", function () { + var node1 = iD.Node(), + node2 = iD.Node(), + way = iD.Way({nodes: [node1.id, node2.id]}), + action = iD.actions.DeleteNode(node1.id), + graph = action(iD.Graph([node1, node2, way])); + expect(graph.entity(way.id)).to.be.undefined; + }); + + it("deletes degenerate circular ways", function () { + var node1 = iD.Node(), + node2 = iD.Node(), + way = iD.Way({nodes: [node1.id, node2.id, node1.id]}), + action = iD.actions.DeleteNode(node2.id), + graph = action(iD.Graph([node1, node2, way])); + expect(graph.entity(way.id)).to.be.undefined; + }); }); diff --git a/test/spec/graph/way.js b/test/spec/graph/way.js index 0c70ea692..87d7d0029 100644 --- a/test/spec/graph/way.js +++ b/test/spec/graph/way.js @@ -117,6 +117,31 @@ describe('iD.Way', function() { }); }); + describe("#isDegenerate", function() { + it("returns true for a linear way with zero or one nodes", function () { + expect(iD.Way({nodes: []}).isDegenerate()).to.equal(true); + expect(iD.Way({nodes: ['a']}).isDegenerate()).to.equal(true); + }); + + it("returns true for a circular way with only one unique node", function () { + expect(iD.Way({nodes: ['a', 'a']}).isDegenerate()).to.equal(true); + }); + + it("returns false for a linear way with two or more nodes", function () { + expect(iD.Way({nodes: ['a', 'b']}).isDegenerate()).to.equal(false); + }); + + it("returns true for an area with zero, one, or two unique nodes", function () { + expect(iD.Way({tags: {area: 'yes'}, nodes: []}).isDegenerate()).to.equal(true); + expect(iD.Way({tags: {area: 'yes'}, nodes: ['a', 'a']}).isDegenerate()).to.equal(true); + expect(iD.Way({tags: {area: 'yes'}, nodes: ['a', 'b', 'a']}).isDegenerate()).to.equal(true); + }); + + it("returns false for an area with three or more unique nodes", function () { + expect(iD.Way({tags: {area: 'yes'}, nodes: ['a', 'b', 'c', 'a']}).isDegenerate()).to.equal(false); + }); + }); + describe("#geometry", function() { it("returns 'line' when the way is not an area", function () { expect(iD.Way().geometry()).to.equal('line'); From 18b33310032e5df383c50a46016c51841b1d1d96 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Wed, 23 Jan 2013 12:39:45 -0500 Subject: [PATCH 26/32] Fix event reference --- js/id/id.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/id/id.js b/js/id/id.js index 9d7fb38bf..3646ba074 100644 --- a/js/id/id.js +++ b/js/id/id.js @@ -192,7 +192,7 @@ window.iD = function(container) { .on('⌃+Z', function() { history.undo(); }) .on('⌘+⇧+Z', function() { history.redo(); }) .on('⌃+⇧+Z', function() { history.redo(); }) - .on('⌫', function(e) { e.preventDefault(); }); + .on('⌫', function() { d3.event.preventDefault(); }); d3.select(document) .call(keybinding); From fcf15718d13384721af86cba6cf9082914741d2a Mon Sep 17 00:00:00 2001 From: Tom MacWright Date: Wed, 23 Jan 2013 15:27:47 -0500 Subject: [PATCH 27/32] Restyle flash modal. Fixes #458 --- css/app.css | 11 +++++++++++ js/id/ui/flash.js | 4 ++-- js/id/ui/geocoder.js | 1 + js/id/ui/inspector.js | 2 ++ 4 files changed, 16 insertions(+), 2 deletions(-) diff --git a/css/app.css b/css/app.css index 56e87ccd7..7227e6d82 100644 --- a/css/app.css +++ b/css/app.css @@ -940,6 +940,17 @@ div.typeahead a:first-child { display: block; } +.modal-flash .content { + box-shadow: none; + border-radius: 4px; + background: #111; + color: #eee; +} + +.modal-flash .close-modal { + display:none; +} + .changeset-list li { border-top:1px solid #ccc; padding:5px 10px; diff --git a/js/id/ui/flash.js b/js/id/ui/flash.js index 4ce6a3b0a..1e4d7379d 100644 --- a/js/id/ui/flash.js +++ b/js/id/ui/flash.js @@ -1,7 +1,7 @@ iD.ui.flash = function() { var modal = iD.ui.modal(); - modal.select('.modal').classed('modal-alert', true); + modal.select('.modal').classed('modal-flash', true); modal.select('.content') .classed('modal-section', true) @@ -13,7 +13,7 @@ iD.ui.flash = function() { setTimeout(function() { modal.remove(); return true; - }, 1000); + }, 1500); return modal; }; diff --git a/js/id/ui/geocoder.js b/js/id/ui/geocoder.js index ba3e0db2f..9a9c1e1d3 100644 --- a/js/id/ui/geocoder.js +++ b/js/id/ui/geocoder.js @@ -13,6 +13,7 @@ iD.ui.geocoder = function() { if (!resp.results.length) { return iD.ui.flash() .select('.content') + .append('h3') .text('No location found for "' + resp.query[0] + '"'); } var bounds = resp.results[0][0].bounds; diff --git a/js/id/ui/inspector.js b/js/id/ui/inspector.js index 058f318df..22fac522d 100644 --- a/js/id/ui/inspector.js +++ b/js/id/ui/inspector.js @@ -178,6 +178,7 @@ iD.ui.inspector = function() { } else { iD.ui.flash() .select('.content') + .append('h3') .text('This is no documentation available for this tag combination'); } }); @@ -195,6 +196,7 @@ iD.ui.inspector = function() { } else { iD.ui.flash() .select('.content') + .append('h3') .text('This is no documentation available for this key'); } }); From 671e138fcd83788ffcd06a3f060bd3b0211d7983 Mon Sep 17 00:00:00 2001 From: Tom MacWright Date: Wed, 23 Jan 2013 15:32:25 -0500 Subject: [PATCH 28/32] Fix flash test --- test/spec/ui/flash.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/spec/ui/flash.js b/test/spec/ui/flash.js index 2b43ef83e..b0314ffeb 100644 --- a/test/spec/ui/flash.js +++ b/test/spec/ui/flash.js @@ -11,7 +11,7 @@ describe("iD.ui.flash", function () { it('leaves after 1000 ms', function () { var flash = iD.ui.flash(); - clock.tick(1010); + clock.tick(1610); expect(flash.node().parentNode).to.be.null; }); }); From 72618574f371ff7ca8eadf18f3f52f9292bdff72 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Wed, 23 Jan 2013 15:05:00 -0500 Subject: [PATCH 29/32] Rename permitted -> enabled and add to more actions --- js/id/actions/delete_node.js | 8 +++++++- js/id/actions/delete_way.js | 8 +++++++- js/id/actions/reverse_way.js | 8 +++++++- js/id/actions/split_way.js | 4 ++-- js/id/actions/unjoin_node.js | 4 ++-- test/spec/actions/split_way.js | 8 ++++---- test/spec/actions/unjoin_node.js | 6 +++--- 7 files changed, 32 insertions(+), 14 deletions(-) diff --git a/js/id/actions/delete_node.js b/js/id/actions/delete_node.js index a93129ce3..48e704ac3 100644 --- a/js/id/actions/delete_node.js +++ b/js/id/actions/delete_node.js @@ -1,6 +1,6 @@ // https://github.com/openstreetmap/potlatch2/blob/master/net/systemeD/halcyon/connection/actions/DeleteNodeAction.as iD.actions.DeleteNode = function(nodeId) { - return function(graph) { + var action = function(graph) { var node = graph.entity(nodeId); graph.parentWays(node) @@ -20,4 +20,10 @@ iD.actions.DeleteNode = function(nodeId) { return graph.remove(node); }; + + action.enabled = function(graph) { + return true; + }; + + return action; }; diff --git a/js/id/actions/delete_way.js b/js/id/actions/delete_way.js index 19e9e7c2a..224537c81 100644 --- a/js/id/actions/delete_way.js +++ b/js/id/actions/delete_way.js @@ -1,6 +1,6 @@ // https://github.com/openstreetmap/potlatch2/blob/master/net/systemeD/halcyon/connection/actions/DeleteWayAction.as iD.actions.DeleteWay = function(wayId) { - return function(graph) { + var action = function(graph) { var way = graph.entity(wayId); graph.parentRelations(way) @@ -29,4 +29,10 @@ iD.actions.DeleteWay = function(wayId) { return graph.remove(way); }; + + action.enabled = function(graph) { + return true; + } + + return action; }; diff --git a/js/id/actions/reverse_way.js b/js/id/actions/reverse_way.js index 160017637..4a7d392b7 100644 --- a/js/id/actions/reverse_way.js +++ b/js/id/actions/reverse_way.js @@ -53,7 +53,7 @@ iD.actions.ReverseWay = function(wayId) { } } - return function(graph) { + var action = function(graph) { var way = graph.entity(wayId), nodes = way.nodes.slice().reverse(), tags = {}, key, role; @@ -73,4 +73,10 @@ iD.actions.ReverseWay = function(wayId) { return graph.replace(way.update({nodes: nodes, tags: tags})); }; + + action.enabled = function(graph) { + return true; + }; + + return action; }; diff --git a/js/id/actions/split_way.js b/js/id/actions/split_way.js index f6d502b97..6db8b57e0 100644 --- a/js/id/actions/split_way.js +++ b/js/id/actions/split_way.js @@ -19,7 +19,7 @@ iD.actions.SplitWay = function(nodeId, newWayId) { } var action = function(graph) { - if (!action.permitted(graph)) + if (!action.enabled(graph)) return graph; var way = candidateWays(graph)[0], @@ -60,7 +60,7 @@ iD.actions.SplitWay = function(nodeId, newWayId) { return graph; }; - action.permitted = function(graph) { + action.enabled = function(graph) { return candidateWays(graph).length === 1; }; diff --git a/js/id/actions/unjoin_node.js b/js/id/actions/unjoin_node.js index 037c56137..39e05a9cf 100644 --- a/js/id/actions/unjoin_node.js +++ b/js/id/actions/unjoin_node.js @@ -10,7 +10,7 @@ // iD.actions.UnjoinNode = function(nodeId, newNodeId) { var action = function(graph) { - if (!action.permitted(graph)) + if (!action.enabled(graph)) return graph; var node = graph.entity(nodeId); @@ -29,7 +29,7 @@ iD.actions.UnjoinNode = function(nodeId, newNodeId) { return graph; }; - action.permitted = function(graph) { + action.enabled = function(graph) { return graph.parentWays(graph.entity(nodeId)).length >= 2; }; diff --git a/test/spec/actions/split_way.js b/test/spec/actions/split_way.js index 09bacb82a..6074cbaa8 100644 --- a/test/spec/actions/split_way.js +++ b/test/spec/actions/split_way.js @@ -1,5 +1,5 @@ describe("iD.actions.SplitWay", function () { - describe("#permitted", function () { + describe("#enabled", function () { it("returns true for a non-end node of a single way", function () { var graph = iD.Graph({ 'a': iD.Node({id: 'a'}), @@ -8,7 +8,7 @@ describe("iD.actions.SplitWay", function () { '-': iD.Way({id: '-', nodes: ['a', 'b', 'c']}) }); - expect(iD.actions.SplitWay('b').permitted(graph)).to.be.true; + expect(iD.actions.SplitWay('b').enabled(graph)).to.be.true; }); it("returns false for the first node of a single way", function () { @@ -18,7 +18,7 @@ describe("iD.actions.SplitWay", function () { '-': iD.Way({id: '-', nodes: ['a', 'b']}) }); - expect(iD.actions.SplitWay('a').permitted(graph)).to.be.false; + expect(iD.actions.SplitWay('a').enabled(graph)).to.be.false; }); it("returns false for the last node of a single way", function () { @@ -28,7 +28,7 @@ describe("iD.actions.SplitWay", function () { '-': iD.Way({id: '-', nodes: ['a', 'b']}) }); - expect(iD.actions.SplitWay('b').permitted(graph)).to.be.false; + expect(iD.actions.SplitWay('b').enabled(graph)).to.be.false; }); }); diff --git a/test/spec/actions/unjoin_node.js b/test/spec/actions/unjoin_node.js index 247239f5c..5901e97da 100644 --- a/test/spec/actions/unjoin_node.js +++ b/test/spec/actions/unjoin_node.js @@ -1,9 +1,9 @@ describe("iD.actions.UnjoinNode", function () { - describe("#permitted", function () { + describe("#enabled", function () { it("returns false for a node shared by less than two ways", function () { var graph = iD.Graph({'a': iD.Node()}); - expect(iD.actions.UnjoinNode('a').permitted(graph)).to.equal(false); + expect(iD.actions.UnjoinNode('a').enabled(graph)).to.equal(false); }); it("returns true for a node shared by two or more ways", function () { @@ -19,7 +19,7 @@ describe("iD.actions.UnjoinNode", function () { '|': iD.Way({id: '|', nodes: ['d', 'b']}) }); - expect(iD.actions.UnjoinNode('b').permitted(graph)).to.equal(true); + expect(iD.actions.UnjoinNode('b').enabled(graph)).to.equal(true); }); }); From 3138ba28054337306ff1d1a33addceba5c61b409 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Wed, 23 Jan 2013 15:52:35 -0500 Subject: [PATCH 30/32] Handle incomplete relations in SplitWay (fixes #466) --- js/id/actions/split_way.js | 3 ++- test/spec/actions/split_way.js | 14 ++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/js/id/actions/split_way.js b/js/id/actions/split_way.js index 6db8b57e0..6fbd08595 100644 --- a/js/id/actions/split_way.js +++ b/js/id/actions/split_way.js @@ -47,7 +47,8 @@ iD.actions.SplitWay = function(nodeId, newWayId) { j; for (j = 0; j < relation.members.length; j++) { - if (relation.members[j].type === 'way' && graph.entity(relation.members[j].id).contains(last)) { + var entity = graph.entity(relation.members[j].id); + if (entity && entity.type === 'way' && entity.contains(last)) { break; } } diff --git a/test/spec/actions/split_way.js b/test/spec/actions/split_way.js index 6074cbaa8..2958521a9 100644 --- a/test/spec/actions/split_way.js +++ b/test/spec/actions/split_way.js @@ -175,6 +175,20 @@ describe("iD.actions.SplitWay", function () { expect(_.pluck(graph.entity('r').members, 'id')).to.eql(['~', '=', '-']); }); + it("handles incomplete relations", function () { + var graph = iD.Graph({ + 'a': iD.Node({id: 'a'}), + 'b': iD.Node({id: 'b'}), + 'c': iD.Node({id: 'c'}), + '-': iD.Way({id: '-', nodes: ['a', 'b', 'c']}), + 'r': iD.Relation({id: 'r', members: [{id: '~', type: 'way'}, {id: '-', type: 'way'}]}) + }); + + graph = iD.actions.SplitWay('b', '=')(graph); + + expect(_.pluck(graph.entity('r').members, 'id')).to.eql(['~', '-', '=']); + }); + ['restriction', 'restriction:bus'].forEach(function (type) { it("updates a restriction's 'from' role", function () { // Situation: From f605ff85ea4ae519a4d2d277ab510013e5e87a95 Mon Sep 17 00:00:00 2001 From: Tom MacWright Date: Wed, 23 Jan 2013 15:53:46 -0500 Subject: [PATCH 31/32] Prettier transition for notice, fix notice style. Fixes #438 --- css/app.css | 21 +++++++++++---------- js/id/id.js | 4 +--- js/id/ui/notice.js | 39 +++++++++++++++++++++++++++++---------- 3 files changed, 41 insertions(+), 23 deletions(-) diff --git a/css/app.css b/css/app.css index 7227e6d82..9e70092af 100644 --- a/css/app.css +++ b/css/app.css @@ -981,18 +981,19 @@ div.typeahead a:first-child { ------------------------------------------------------- */ .notice { - position:absolute; - top:11px; - left:11px; - width:278px; + float:left; + width:33.333%; + padding-right: 10px; +} + +.notice .notice-text { + height: 40px; text-align: center; - height:38px; - padding:10px 20px; - background:#fff; - color:#000; font-weight: normal; - line-height: 21px; - border-radius:5px; + border-radius: 5px; + line-height: 40px; + background: #fff; + color: #000; opacity: 0.9; } diff --git a/js/id/id.js b/js/id/id.js index 3646ba074..c0f64a415 100644 --- a/js/id/id.js +++ b/js/id/id.js @@ -50,9 +50,7 @@ window.iD = function(container) { } } - var notice = iD.ui.notice(limiter - .append('div') - .attr('class', 'notice')); + var notice = iD.ui.notice(limiter).message(null); map.on('move.disable-buttons', disableTooHigh) .on('move.contributors', _.debounce(function() { diff --git a/js/id/ui/notice.js b/js/id/ui/notice.js index 1c87c5553..67838c31e 100644 --- a/js/id/ui/notice.js +++ b/js/id/ui/notice.js @@ -2,21 +2,40 @@ iD.ui.notice = function(selection) { var message = '', notice = {}; + var div = selection.append('div') + .attr('class', 'notice') + .style('display', 'none'); + + var txt = div.append('div') + .attr('class', 'notice-text'); + + function replace(a, b) { + a.style('opacity', 1) + .transition() + .each('end', function() { + a.style('display', 'none'); + b.style('display', 'inline-block') + .style('opacity', 0) + .transition() + .style('opacity', 1); + }) + .style('opacity', 0); + } + notice.message = function(_) { - selection.attr('class', 'notice inner'); + div.attr('class', 'notice inner'); if (!arguments.length) return _; if (!message && _) { - selection - .text(_) - .transition() - .style('display', ''); + txt.text(_); + replace(selection.select('.button-wrap'), div); } else if (_ && message !== _) { - selection.text(_); + txt.text(_); + selection.select('.button-wrap').style('display', 'none'); } else if (!_) { - selection - .text('') - .transition() - .style('display', 'none'); + txt.text(''); + if (message) { + replace(div, selection.select('.button-wrap')); + } } message = _; return notice; From 64e8e85187589bed18fbf04047b2ec325a6e0243 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Wed, 23 Jan 2013 15:57:57 -0500 Subject: [PATCH 32/32] location.replace (fixes #462) --- js/id/modes/select.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/js/id/modes/select.js b/js/id/modes/select.js index d80d6fffd..dc298e36a 100644 --- a/js/id/modes/select.js +++ b/js/id/modes/select.js @@ -45,9 +45,9 @@ iD.modes.Select = function(entity, initial) { }); var q = iD.util.stringQs(location.hash.substring(1)); - location.hash = '#' + iD.util.qsString(_.assign(q, { + location.replace('#' + iD.util.qsString(_.assign(q, { id: entity.id - }), true); + }), true)); d3.select('.inspector-wrap') .style('display', 'block') @@ -166,7 +166,7 @@ iD.modes.Select = function(entity, initial) { }); var q = iD.util.stringQs(location.hash.substring(1)); - location.hash = '#' + iD.util.qsString(_.omit(q, 'id'), true); + location.replace('#' + iD.util.qsString(_.omit(q, 'id'), true)); keybinding.off();