diff --git a/css/80_app.css b/css/80_app.css index 2e14ac1d0..e10487025 100644 --- a/css/80_app.css +++ b/css/80_app.css @@ -713,7 +713,7 @@ button.save.has-count .count::before { position: absolute; right: 0; top: 0; - height: 60px; + height: 59px; z-index: 50; } [dir='rtl'] .modal > button { diff --git a/data/core.yaml b/data/core.yaml index 9261276af..08021a33f 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -470,6 +470,7 @@ en: status_code: "Server returned status code {code}" unknown_error_details: "Please ensure you are connected to the internet." uploading: Uploading changes to OpenStreetMap... + conflict_progress: "Checking for conflicts: {num} of {total}" unsaved_changes: You have unsaved changes conflict: header: Resolve conflicting edits diff --git a/dist/locales/en.json b/dist/locales/en.json index 1e13481be..976862a90 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -582,6 +582,7 @@ "status_code": "Server returned status code {code}", "unknown_error_details": "Please ensure you are connected to the internet.", "uploading": "Uploading changes to OpenStreetMap...", + "conflict_progress": "Checking for conflicts: {num} of {total}", "unsaved_changes": "You have unsaved changes", "conflict": { "header": "Resolve conflicting edits", diff --git a/modules/modes/save.js b/modules/modes/save.js index 4fb3908a0..f5198b41b 100644 --- a/modules/modes/save.js +++ b/modules/modes/save.js @@ -1,7 +1,5 @@ import _clone from 'lodash-es/clone'; -import _difference from 'lodash-es/difference'; import _filter from 'lodash-es/filter'; -import _each from 'lodash-es/each'; import _map from 'lodash-es/map'; import _reduce from 'lodash-es/reduce'; import _union from 'lodash-es/union'; @@ -44,18 +42,31 @@ import { } from '../util'; +var _isSaving = false; + export function modeSave(context) { - var mode = { - id: 'save' - }; + var mode = { id: 'save' }; + var keybinding = d3_keybinding('save'); - var keybinding = d3_keybinding('select'); + var loading = uiLoading(context) + .message(t('save.uploading')) + .blocking(true); var commit = uiCommit(context) .on('cancel', cancel) .on('save', save); + var _toCheck = []; + var _toLoad = []; + var _loaded = {}; + var _toLoadCount = 0; + var _toLoadTotal = 0; + + var _conflicts = []; + var _errors = []; + var _origChanges; + function cancel(selectedID) { if (selectedID) { @@ -66,42 +77,85 @@ export function modeSave(context) { } - function save(changeset, tryAgain) { + function save(changeset, tryAgain, checkConflicts) { + // Guard against accidentally entering save code twice - #4641 + if (_isSaving && !tryAgain) { + return; + } - var osm = context.connection(), - loading = uiLoading(context).message(t('save.uploading')).blocking(true), - history = context.history(), - origChanges = history.changes(actionDiscardTags(history.difference())), - localGraph = context.graph(), - remoteGraph = coreGraph(history.base(), true), - modified = _filter(history.difference().summary(), {changeType: 'modified'}), - toCheck = _map(_map(modified, 'entity'), 'id'), - toLoad = withChildNodes(toCheck, localGraph), - conflicts = [], - errors = []; + var osm = context.connection(); + if (!osm) { + cancel(); + return; + } - if (!osm) return; + // If user somehow got logged out mid-save, try to reauthenticate.. + // This can happen if they were logged in from before, but the tokens are no longer valid. + if (!osm.authenticated()) { + osm.authenticate(function(err) { + if (err) { + cancel(); // quit save mode.. + } else { + save(changeset, tryAgain, checkConflicts); // continue where we left off.. + } + }); + return; + } + if (!_isSaving) { + keybindingOff(); + context.container().call(loading); // block input + _isSaving = true; + } + + var history = context.history(); + var localGraph = context.graph(); + var remoteGraph = coreGraph(history.base(), true); + + _conflicts = []; + _errors = []; + + // Store original changes, in case user wants to download them as an .osc file + _origChanges = history.changes(actionDiscardTags(history.difference())); + + // First time, `history.perform` a no-op action. + // Any conflict resolutions will be done as `history.replace` if (!tryAgain) { - history.perform(actionNoop()); // checkpoint + history.perform(actionNoop()); } - context.container().call(loading); + // Attempt a fast upload.. If there are conflicts, re-enter with `checkConflicts = true` + if (!checkConflicts) { + upload(changeset); - if (toCheck.length) { - osm.loadMultiple(toLoad, loaded); + // Do the full (slow) conflict check.. } else { - upload(); + var modified = _filter(history.difference().summary(), { changeType: 'modified' }); + _toCheck = _map(_map(modified, 'entity'), 'id'); + _toLoad = withChildNodes(_toCheck, localGraph); + _loaded = {}; + _toLoadCount = 0; + _toLoadTotal = _toLoad.length; + + if (_toCheck.length) { + showProgress(_toLoadCount, _toLoadTotal); + _toLoad.forEach(function(id) { _loaded[id] = false; }); + osm.loadMultiple(_toLoad, loaded); + } else { + upload(changeset); + } } + return; + function withChildNodes(ids, graph) { return _uniq(_reduce(ids, function(result, id) { var entity = graph.entity(id); if (entity.type === 'way') { try { - var cn = graph.childNodes(entity); - result.push.apply(result, _map(_filter(cn, 'version'), 'id')); + var children = graph.childNodes(entity); + result.push.apply(result, _map(_filter(children, 'version'), 'id')); } catch (err) { /* eslint-disable no-console */ if (typeof console !== 'undefined') console.error(err); @@ -115,46 +169,64 @@ export function modeSave(context) { // Reload modified entities into an alternate graph and check for conflicts.. function loaded(err, result) { - if (errors.length) return; + if (_errors.length) return; if (err) { - errors.push({ - msg: err.responseText, + _errors.push({ + msg: err.message || err.responseText, details: [ t('save.status_code', { code: err.status }) ] }); showErrors(); } else { var loadMore = []; - _each(result.data, function(entity) { + + result.data.forEach(function(entity) { remoteGraph.replace(entity); - toLoad = _without(toLoad, entity.id); + _loaded[entity.id] = true; + _toLoad = _without(_toLoad, entity.id); + + if (!entity.visible) return; // Because loadMultiple doesn't download /full like loadEntity, // need to also load children that aren't already being checked.. - if (!entity.visible) return; + var i, id; if (entity.type === 'way') { - loadMore.push.apply(loadMore, - _difference(entity.nodes, toCheck, toLoad, loadMore)); + for (i = 0; i < entity.nodes.length; i++) { + id = entity.nodes[i]; + if (_loaded[id] === undefined) { + _loaded[id] = false; + loadMore.push(id); + } + } } else if (entity.type === 'relation' && entity.isMultipolygon()) { - loadMore.push.apply(loadMore, - _difference(_map(entity.members, 'id'), toCheck, toLoad, loadMore)); + for (i = 0; i < entity.members.length; i++) { + id = entity.members[i].id; + if (_loaded[id] === undefined) { + _loaded[id] = false; + loadMore.push(id); + } + } } }); + _toLoadCount += result.data.length; + _toLoadTotal += loadMore.length; + showProgress(_toLoadCount, _toLoadTotal); + if (loadMore.length) { - toLoad.push.apply(toLoad, loadMore); + _toLoad.push.apply(_toLoad, loadMore); osm.loadMultiple(loadMore, loaded); } - if (!toLoad.length) { - checkConflicts(); + if (!_toLoad.length) { + detectConflicts(); } } } - function checkConflicts() { + function detectConflicts() { function choice(id, text, action) { return { id: id, text: text, action: function() { history.replace(action); } }; } @@ -165,16 +237,14 @@ export function modeSave(context) { return utilDisplayName(entity) || (utilDisplayType(entity.id) + ' ' + entity.id); } - function compareVersions(local, remote) { + function sameVersions(local, remote) { if (local.version !== remote.version) return false; if (local.type === 'way') { var children = _union(local.nodes, remote.nodes); - for (var i = 0; i < children.length; i++) { - var a = localGraph.hasEntity(children[i]), - b = remoteGraph.hasEntity(children[i]); - + var a = localGraph.hasEntity(children[i]); + var b = remoteGraph.hasEntity(children[i]); if (a && b && a.version !== b.version) return false; } } @@ -182,26 +252,26 @@ export function modeSave(context) { return true; } - _each(toCheck, function(id) { - var local = localGraph.entity(id), - remote = remoteGraph.entity(id); + _toCheck.forEach(function(id) { + var local = localGraph.entity(id); + var remote = remoteGraph.entity(id); - if (compareVersions(local, remote)) return; + if (sameVersions(local, remote)) return; - var action = actionMergeRemoteChanges, - merge = action(id, localGraph, remoteGraph, formatUser); + var action = actionMergeRemoteChanges; + var merge = action(id, localGraph, remoteGraph, formatUser); history.replace(merge); var mergeConflicts = merge.conflicts(); if (!mergeConflicts.length) return; // merged safely - var forceLocal = action(id, localGraph, remoteGraph).withOption('force_local'), - forceRemote = action(id, localGraph, remoteGraph).withOption('force_remote'), - keepMine = t('save.conflict.' + (remote.visible ? 'keep_local' : 'restore')), - keepTheirs = t('save.conflict.' + (remote.visible ? 'keep_remote' : 'delete')); + var forceLocal = action(id, localGraph, remoteGraph).withOption('force_local'); + var forceRemote = action(id, localGraph, remoteGraph).withOption('force_remote'); + var keepMine = t('save.conflict.' + (remote.visible ? 'keep_local' : 'restore')); + var keepTheirs = t('save.conflict.' + (remote.visible ? 'keep_remote' : 'delete')); - conflicts.push({ + _conflicts.push({ id: id, name: entityName(local), details: mergeConflicts, @@ -213,163 +283,206 @@ export function modeSave(context) { }); }); - upload(); + upload(changeset); + } + } + + + function upload(changeset) { + var osm = context.connection(); + if (!osm) { + _errors.push({ msg: 'No OSM Service' }); } + if (_conflicts.length) { + _conflicts.sort(function(a, b) { return b.id.localeCompare(a.id); }); + showConflicts(changeset); - function upload() { - if (conflicts.length) { - conflicts.sort(function(a,b) { return b.id.localeCompare(a.id); }); - showConflicts(); - } else if (errors.length) { - showErrors(); - } else { - var changes = history.changes(actionDiscardTags(history.difference())); - if (changes.modified.length || changes.created.length || changes.deleted.length) { - osm.putChangeset(changeset, changes, uploadCallback); - } else { // changes were insignificant or reverted by user - d3_select('.inspector-wrap *').remove(); - loading.close(); - context.flush(); - cancel(); - } + } else if (_errors.length) { + showErrors(); + + } else { + var history = context.history(); + var changes = history.changes(actionDiscardTags(history.difference())); + if (changes.modified.length || changes.created.length || changes.deleted.length) { + osm.putChangeset(changeset, changes, uploadCallback); + } else { // changes were insignificant or reverted by user + d3_select('.inspector-wrap *').remove(); + loading.close(); + _isSaving = false; + context.flush(); + cancel(); } } + } - function uploadCallback(err, changeset) { - if (err) { - errors.push({ - msg: err.responseText, + function uploadCallback(err, changeset) { + if (err) { + if (err.status === 409) { // 409 Conflict + save(changeset, true, true); // tryAgain = true, checkConflicts = true + } else { + _errors.push({ + msg: err.message || err.responseText, details: [ t('save.status_code', { code: err.status }) ] }); showErrors(); - } else { - history.clearSaved(); - success(changeset); - // Add delay to allow for postgres replication #1646 #2678 - window.setTimeout(function() { - d3_select('.inspector-wrap *').remove(); - loading.close(); - context.flush(); - }, 2500); } + + } else { + context.history().clearSaved(); + success(changeset); + // Add delay to allow for postgres replication #1646 #2678 + window.setTimeout(function() { + d3_select('.inspector-wrap *').remove(); + loading.close(); + _isSaving = false; + context.flush(); + }, 2500); } + } - function showConflicts() { - var selection = context.container() - .select('#sidebar') - .append('div') - .attr('class','sidebar-component'); + function showProgress(num, total) { + var modal = context.container().select('.loading-modal .modal-section'); + var progress = modal.selectAll('.progress') + .data([0]); - loading.close(); + // enter/update + progress.enter() + .append('div') + .attr('class', 'progress') + .merge(progress) + .text(t('save.conflict_progress', { num: num, total: total })); + } - selection.call(uiConflicts(context) - .list(conflicts) - .origChanges(origChanges) - .on('cancel', function() { - history.pop(); - selection.remove(); - }) - .on('save', function() { - for (var i = 0; i < conflicts.length; i++) { - if (conflicts[i].chosen === 1) { // user chose "keep theirs" - var entity = context.hasEntity(conflicts[i].id); - if (entity && entity.type === 'way') { - var children = _uniq(entity.nodes); - for (var j = 0; j < children.length; j++) { - history.replace(actionRevert(children[j])); - } + + function showConflicts(changeset) { + var history = context.history(); + var selection = context.container() + .select('#sidebar') + .append('div') + .attr('class','sidebar-component'); + + loading.close(); + _isSaving = false; + + var ui = uiConflicts(context) + .conflictList(_conflicts) + .origChanges(_origChanges) + .on('cancel', function() { + history.pop(); + selection.remove(); + keybindingOn(); + }) + .on('save', function() { + for (var i = 0; i < _conflicts.length; i++) { + if (_conflicts[i].chosen === 1) { // user chose "keep theirs" + var entity = context.hasEntity(_conflicts[i].id); + if (entity && entity.type === 'way') { + var children = _uniq(entity.nodes); + for (var j = 0; j < children.length; j++) { + history.replace(actionRevert(children[j])); } - history.replace(actionRevert(conflicts[i].id)); } + history.replace(actionRevert(_conflicts[i].id)); } + } - selection.remove(); - save(changeset, true); - }) - ); - } + selection.remove(); + save(changeset, true, false); // tryAgain = true, checkConflicts = false + }); + + selection.call(ui); + } - function showErrors() { - var selection = uiConfirm(context.container()); + function showErrors() { + keybindingOn(); + context.history().pop(); + loading.close(); + _isSaving = false; - history.pop(); - loading.close(); + var selection = uiConfirm(context.container()); + selection + .select('.modal-section.header') + .append('h3') + .text(t('save.error')); - selection - .select('.modal-section.header') - .append('h3') - .text(t('save.error')); - - addErrors(selection, errors); - selection.okButton(); - } + addErrors(selection, _errors); + selection.okButton(); + } - function addErrors(selection, data) { - var message = selection - .select('.modal-section.message-text'); + function addErrors(selection, data) { + var message = selection + .select('.modal-section.message-text'); - var items = message - .selectAll('.error-container') - .data(data); + var items = message + .selectAll('.error-container') + .data(data); - var enter = items.enter() - .append('div') - .attr('class', 'error-container'); + var enter = items.enter() + .append('div') + .attr('class', 'error-container'); - enter - .append('a') - .attr('class', 'error-description') - .attr('href', '#') - .classed('hide-toggle', true) - .text(function(d) { return d.msg || t('save.unknown_error_details'); }) - .on('click', function() { - var error = d3_select(this), - detail = d3_select(this.nextElementSibling), - exp = error.classed('expanded'); + enter + .append('a') + .attr('class', 'error-description') + .attr('href', '#') + .classed('hide-toggle', true) + .text(function(d) { return d.msg || t('save.unknown_error_details'); }) + .on('click', function() { + d3_event.preventDefault(); - detail.style('display', exp ? 'none' : 'block'); - error.classed('expanded', !exp); + var error = d3_select(this); + var detail = d3_select(this.nextElementSibling); + var exp = error.classed('expanded'); - d3_event.preventDefault(); - }); + detail.style('display', exp ? 'none' : 'block'); + error.classed('expanded', !exp); + }); - var details = enter - .append('div') - .attr('class', 'error-detail-container') - .style('display', 'none'); + var details = enter + .append('div') + .attr('class', 'error-detail-container') + .style('display', 'none'); - details - .append('ul') - .attr('class', 'error-detail-list') - .selectAll('li') - .data(function(d) { return d.details || []; }) - .enter() - .append('li') - .attr('class', 'error-detail-item') - .text(function(d) { return d; }); - - items.exit() - .remove(); - } + details + .append('ul') + .attr('class', 'error-detail-list') + .selectAll('li') + .data(function(d) { return d.details || []; }) + .enter() + .append('li') + .attr('class', 'error-detail-item') + .text(function(d) { return d; }); + items.exit() + .remove(); } function success(changeset) { commit.reset(); - context.enter(modeBrowse(context) - .sidebar(uiSuccess(context) - .changeset(changeset) - .on('cancel', function() { - context.ui().sidebar.hide(); - }) - ) - ); + + var ui = uiSuccess(context) + .changeset(changeset) + .on('cancel', function() { context.ui().sidebar.hide(); }); + + context.enter(modeBrowse(context).sidebar(ui)); + } + + + function keybindingOn() { + d3_select(document) + .call(keybinding.on('⎋', cancel, true)); + } + + + function keybindingOff() { + d3_select(document) + .call(keybinding.off); } @@ -378,17 +491,16 @@ export function modeSave(context) { context.ui().sidebar.show(commit); } - keybinding - .on('⎋', cancel, true); - - d3_select(document) - .call(keybinding); + keybindingOn(); context.container().selectAll('#content') .attr('class', 'inactive'); var osm = context.connection(); - if (!osm) return; + if (!osm) { + cancel(); + return; + } if (osm.authenticated()) { done(); @@ -405,7 +517,9 @@ export function modeSave(context) { mode.exit = function() { - keybinding.off(); + _isSaving = false; + + keybindingOff(); context.container().selectAll('#content') .attr('class', 'active'); diff --git a/modules/renderer/map.js b/modules/renderer/map.js index a4ebac8a4..bac563eeb 100644 --- a/modules/renderer/map.js +++ b/modules/renderer/map.js @@ -344,7 +344,12 @@ export function rendererMap(context) { function editOff() { context.features().resetStats(); surface.selectAll('.layer-osm *').remove(); - context.enter(modeBrowse(context)); + + var mode = context.mode(); + if (mode && mode.id !== 'save') { + context.enter(modeBrowse(context)); + } + dispatch.call('drawn', this, {full: true}); } diff --git a/modules/services/osm.js b/modules/services/osm.js index af96685a3..9ead1f5b0 100644 --- a/modules/services/osm.js +++ b/modules/services/osm.js @@ -25,25 +25,26 @@ import { import { utilRebind, utilIdleWorker } from '../util'; -var dispatch = d3_dispatch('authLoading', 'authDone', 'change', 'loading', 'loaded'), - urlroot = 'https://www.openstreetmap.org', - blacklists = ['.*\.google(apis)?\..*/(vt|kh)[\?/].*([xyz]=.*){3}.*'], - inflight = {}, - loadedTiles = {}, - entityCache = {}, - connectionId = 1, - tileZoom = 16, - oauth = osmAuth({ - url: urlroot, - oauth_consumer_key: '5A043yRSEugj4DJ5TljuapfnrflWDte8jTOcWLlT', - oauth_secret: 'aB3jKq1TRsCOUrfOIZ6oQMEDmv2ptV76PA54NGLL', - loading: authLoading, - done: authDone - }), - rateLimitError, - userChangesets, - userDetails, - off; +var dispatch = d3_dispatch('authLoading', 'authDone', 'change', 'loading', 'loaded'); +var urlroot = 'https://www.openstreetmap.org'; +var oauth = osmAuth({ + url: urlroot, + oauth_consumer_key: '5A043yRSEugj4DJ5TljuapfnrflWDte8jTOcWLlT', + oauth_secret: 'aB3jKq1TRsCOUrfOIZ6oQMEDmv2ptV76PA54NGLL', + loading: authLoading, + done: authDone +}); + +var _blacklists = ['.*\.google(apis)?\..*/(vt|kh)[\?/].*([xyz]=.*){3}.*']; +var _tiles = { loaded: {}, inflight: {} }; +var _changeset = {}; +var _entityCache = {}; +var _connectionID = 1; +var _tileZoom = 16; +var _rateLimitError; +var _userChangesets; +var _userDetails; +var _off; function authLoading() { @@ -64,15 +65,15 @@ function abortRequest(i) { function getLoc(attrs) { - var lon = attrs.lon && attrs.lon.value, - lat = attrs.lat && attrs.lat.value; + var lon = attrs.lon && attrs.lon.value; + var lat = attrs.lat && attrs.lat.value; return [parseFloat(lon), parseFloat(lat)]; } function getNodes(obj) { - var elems = obj.getElementsByTagName('nd'), - nodes = new Array(elems.length); + var elems = obj.getElementsByTagName('nd'); + var nodes = new Array(elems.length); for (var i = 0, l = elems.length; i < l; i++) { nodes[i] = 'n' + elems[i].attributes.ref.value; } @@ -81,8 +82,8 @@ function getNodes(obj) { function getTags(obj) { - var elems = obj.getElementsByTagName('tag'), - tags = {}; + var elems = obj.getElementsByTagName('tag'); + var tags = {}; for (var i = 0, l = elems.length; i < l; i++) { var attrs = elems[i].attributes; tags[attrs.k.value] = attrs.v.value; @@ -93,8 +94,8 @@ function getTags(obj) { function getMembers(obj) { - var elems = obj.getElementsByTagName('member'), - members = new Array(elems.length); + var elems = obj.getElementsByTagName('member'); + var members = new Array(elems.length); for (var i = 0, l = elems.length; i < l; i++) { var attrs = elems[i].attributes; members[i] = { @@ -164,14 +165,14 @@ function parse(xml, callback, options) { options = _extend({ cache: true }, options); if (!xml || !xml.childNodes) return; - var root = xml.childNodes[0], - children = root.childNodes; + var root = xml.childNodes[0]; + var children = root.childNodes; function parseChild(child) { var parser = parsers[child.nodeName]; if (parser) { var uid = osmEntity.id.fromOSM(child.nodeName, child.attributes.id.value); - if (options.cache && entityCache[uid]) { + if (options.cache && _entityCache[uid]) { return null; } return parser(child, uid); @@ -190,20 +191,21 @@ export default { reset: function() { - connectionId++; - userChangesets = undefined; - userDetails = undefined; - rateLimitError = undefined; - _forEach(inflight, abortRequest); - entityCache = {}; - loadedTiles = {}; - inflight = {}; + _connectionID++; + _userChangesets = undefined; + _userDetails = undefined; + _rateLimitError = undefined; + _forEach(_tiles.inflight, abortRequest); + if (_changeset.inflight) abortRequest(_changeset.inflight); + _tiles = { loaded: {}, inflight: {} }; + _changeset = {}; + _entityCache = {}; return this; }, getConnectionId: function() { - return connectionId; + return _connectionID; }, @@ -239,7 +241,7 @@ export default { loadFromAPI: function(path, callback, options) { options = _extend({ cache: true }, options); var that = this; - var cid = connectionId; + var cid = _connectionID; function done(err, xml) { if (that.getConnectionId() !== cid) { @@ -251,8 +253,7 @@ export default { // 400 Bad Request, 401 Unauthorized, 403 Forbidden // Logout and retry the request.. - if (isAuthenticated && err && - (err.status === 400 || err.status === 401 || err.status === 403)) { + if (isAuthenticated && err && (err.status === 400 || err.status === 401 || err.status === 403)) { that.logout(); that.loadFromAPI(path, callback); @@ -260,9 +261,9 @@ export default { } else { // 509 Bandwidth Limit Exceeded, 429 Too Many Requests // Set the rateLimitError flag and trigger a warning.. - if (!isAuthenticated && !rateLimitError && err && + if (!isAuthenticated && !_rateLimitError && err && (err.status === 509 || err.status === 429)) { - rateLimitError = err; + _rateLimitError = err; dispatch.call('change'); } @@ -271,7 +272,7 @@ export default { parse(xml, function (entities) { if (options.cache) { for (var i in entities) { - entityCache[entities[i].id] = true; + _entityCache[entities[i].id] = true; } } callback(null, entities); @@ -290,9 +291,9 @@ export default { loadEntity: function(id, callback) { - var type = osmEntity.id.type(id), - osmID = osmEntity.id.toOSM(id), - options = { cache: false }; + var type = osmEntity.id.type(id); + var osmID = osmEntity.id.toOSM(id); + var options = { cache: false }; this.loadFromAPI( '/api/0.6/' + type + '/' + osmID + (type !== 'node' ? '/full' : ''), @@ -305,9 +306,9 @@ export default { loadEntityVersion: function(id, version, callback) { - var type = osmEntity.id.type(id), - osmID = osmEntity.id.toOSM(id), - options = { cache: false }; + var type = osmEntity.id.type(id); + var osmID = osmEntity.id.toOSM(id); + var options = { cache: false }; this.loadFromAPI( '/api/0.6/' + type + '/' + osmID + '/' + version, @@ -323,9 +324,9 @@ export default { var that = this; _forEach(_groupBy(_uniq(ids), osmEntity.id.type), function(v, k) { - var type = k + 's', - osmIDs = _map(v, osmEntity.id.toOSM), - options = { cache: false }; + var type = k + 's'; + var osmIDs = _map(v, osmEntity.id.toOSM); + var options = { cache: false }; _forEach(_chunk(osmIDs, 150), function(arr) { that.loadFromAPI( @@ -346,32 +347,46 @@ export default { putChangeset: function(changeset, changes, callback) { + if (_changeset.inflight) { + return callback({ message: 'Changeset already inflight', status: -2 }, changeset); + } + var that = this; - var cid = connectionId; + var cid = _connectionID; - // Create the changeset.. - oauth.xhr({ - method: 'PUT', - path: '/api/0.6/changeset/create', - options: { header: { 'Content-Type': 'text/xml' } }, - content: JXON.stringify(changeset.asJXON()) - }, createdChangeset); + if (_changeset.open) { // reuse existing open changeset.. + createdChangeset(null, _changeset.open); + } else { // open a new changeset.. + _changeset.inflight = oauth.xhr({ + method: 'PUT', + path: '/api/0.6/changeset/create', + options: { header: { 'Content-Type': 'text/xml' } }, + content: JXON.stringify(changeset.asJXON()) + }, createdChangeset); + } - function createdChangeset(err, changeset_id) { + function createdChangeset(err, changesetID) { + _changeset.inflight = null; + if (err) { - return callback(err); + // 400 Bad Request, 401 Unauthorized, 403 Forbidden.. + if (err.status === 400 || err.status === 401 || err.status === 403) { + that.logout(); + } + return callback(err, changeset); } if (that.getConnectionId() !== cid) { - return callback({ message: 'Connection Switched', status: -1 }); + return callback({ message: 'Connection Switched', status: -1 }, changeset); } - changeset = changeset.update({ id: changeset_id }); + _changeset.open = changesetID; + changeset = changeset.update({ id: changesetID }); // Upload the changeset.. - oauth.xhr({ + _changeset.inflight = oauth.xhr({ method: 'POST', - path: '/api/0.6/changeset/' + changeset_id + '/upload', + path: '/api/0.6/changeset/' + changesetID + '/upload', options: { header: { 'Content-Type': 'text/xml' } }, content: JXON.stringify(changeset.osmChangeJXON(changes)) }, uploadedChangeset); @@ -379,7 +394,9 @@ export default { function uploadedChangeset(err) { - if (err) return callback(err); + _changeset.inflight = null; + + if (err) return callback(err, changeset); // Upload was successful, safe to call the callback. // Add delay to allow for postgres replication #1646 #2678 @@ -387,6 +404,8 @@ export default { callback(null, changeset); }, 2500); + _changeset.open = null; + // At this point, we don't really care if the connection was switched.. // Only try to close the changeset if we're still talking to the same server. if (that.getConnectionId() === cid) { @@ -402,45 +421,50 @@ export default { userDetails: function(callback) { - if (userDetails) { - callback(undefined, userDetails); + if (_userDetails) { + callback(undefined, _userDetails); return; } var that = this; - var cid = connectionId; + var cid = _connectionID; function done(err, user_details) { if (err) { + // 400 Bad Request, 401 Unauthorized, 403 Forbidden.. + if (err.status === 400 || err.status === 401 || err.status === 403) { + that.logout(); + } return callback(err); } if (that.getConnectionId() !== cid) { return callback({ message: 'Connection Switched', status: -1 }); } - var u = user_details.getElementsByTagName('user')[0], - img = u.getElementsByTagName('img'), - image_url = ''; + + var u = user_details.getElementsByTagName('user')[0]; + var img = u.getElementsByTagName('img'); + var image_url = ''; if (img && img[0] && img[0].getAttribute('href')) { image_url = img[0].getAttribute('href'); } - var changesets = u.getElementsByTagName('changesets'), - changesets_count = 0; + var changesets = u.getElementsByTagName('changesets'); + var changesets_count = 0; if (changesets && changesets[0] && changesets[0].getAttribute('count')) { changesets_count = changesets[0].getAttribute('count'); } - userDetails = { + _userDetails = { id: u.attributes.id.value, display_name: u.attributes.display_name.value, image_url: image_url, changesets_count: changesets_count }; - callback(undefined, userDetails); + callback(undefined, _userDetails); } oauth.xhr({ method: 'GET', path: '/api/0.6/user/details' }, done); @@ -448,13 +472,13 @@ export default { userChangesets: function(callback) { - if (userChangesets) { - callback(undefined, userChangesets); + if (_userChangesets) { + callback(undefined, _userChangesets); return; } var that = this; - var cid = connectionId; + var cid = _connectionID; this.userDetails(function(err, user) { if (err) { @@ -466,13 +490,17 @@ export default { function done(err, changesets) { if (err) { + // 400 Bad Request, 401 Unauthorized, 403 Forbidden.. + if (err.status === 400 || err.status === 401 || err.status === 403) { + that.logout(); + } return callback(err); } if (that.getConnectionId() !== cid) { return callback({ message: 'Connection Switched', status: -1 }); } - userChangesets = Array.prototype.map.call( + _userChangesets = Array.prototype.map.call( changesets.getElementsByTagName('changeset'), function (changeset) { return { tags: getTags(changeset) }; @@ -482,7 +510,7 @@ export default { return comment && comment !== ''; }); - callback(undefined, userChangesets); + callback(undefined, _userChangesets); } oauth.xhr({ method: 'GET', path: '/api/0.6/changesets?user=' + user.id }, done); @@ -492,7 +520,7 @@ export default { status: function(callback) { var that = this; - var cid = connectionId; + var cid = _connectionID; function done(xml) { if (that.getConnectionId() !== cid) { @@ -509,12 +537,12 @@ export default { } } if (regexes.length) { - blacklists = regexes; + _blacklists = regexes; } - if (rateLimitError) { - callback(rateLimitError, 'rateLimited'); + if (_rateLimitError) { + callback(_rateLimitError, 'rateLimited'); } else { var apiStatus = xml.getElementsByTagName('status'); var val = apiStatus[0].getAttribute('api'); @@ -530,31 +558,31 @@ export default { imageryBlacklists: function() { - return blacklists; + return _blacklists; }, tileZoom: function(_) { - if (!arguments.length) return tileZoom; - tileZoom = _; + if (!arguments.length) return _tileZoom; + _tileZoom = _; return this; }, loadTiles: function(projection, dimensions, callback) { - if (off) return; + if (_off) return; var that = this; var s = projection.scale() * 2 * Math.PI; var z = Math.max(Math.log(s) / Math.log(2) - 8, 0); - var ts = 256 * Math.pow(2, z - tileZoom); + var ts = 256 * Math.pow(2, z - _tileZoom); var origin = [ s / 2 - projection.translate()[0], s / 2 - projection.translate()[1] ]; var tiles = d3_geoTile() - .scaleExtent([tileZoom, tileZoom]) + .scaleExtent([_tileZoom, _tileZoom]) .scale(s) .size(dimensions) .translate(projection.translate())() @@ -570,36 +598,36 @@ export default { }; }); - _filter(inflight, function(v, i) { + _filter(_tiles.inflight, function(v, i) { var wanted = _find(tiles, function(tile) { return i === tile.id; }); - if (!wanted) delete inflight[i]; + if (!wanted) delete _tiles.inflight[i]; return !wanted; }).map(abortRequest); tiles.forEach(function(tile) { var id = tile.id; - if (loadedTiles[id] || inflight[id]) return; + if (_tiles.loaded[id] || _tiles.inflight[id]) return; - if (_isEmpty(inflight)) { + if (_isEmpty(_tiles.inflight)) { dispatch.call('loading'); } - inflight[id] = that.loadFromAPI( + _tiles.inflight[id] = that.loadFromAPI( '/api/0.6/map?bbox=' + tile.extent.toParam(), function(err, parsed) { - delete inflight[id]; + delete _tiles.inflight[id]; if (!err) { - loadedTiles[id] = true; + _tiles.loaded[id] = true; } if (callback) { callback(err, _extend({ data: parsed }, tile)); } - if (_isEmpty(inflight)) { + if (_isEmpty(_tiles.inflight)) { dispatch.call('loaded'); } } @@ -625,21 +653,21 @@ export default { toggle: function(_) { - off = !_; + _off = !_; return this; }, loadedTiles: function(_) { - if (!arguments.length) return loadedTiles; - loadedTiles = _; + if (!arguments.length) return _tiles.loaded; + _tiles.loaded = _; return this; }, logout: function() { - userChangesets = undefined; - userDetails = undefined; + _userChangesets = undefined; + _userDetails = undefined; oauth.logout(); dispatch.call('change'); return this; @@ -648,9 +676,9 @@ export default { authenticate: function(callback) { var that = this; - var cid = connectionId; - userChangesets = undefined; - userDetails = undefined; + var cid = _connectionID; + _userChangesets = undefined; + _userDetails = undefined; function done(err, res) { if (err) { @@ -661,7 +689,7 @@ export default { if (callback) callback({ message: 'Connection Switched', status: -1 }); return; } - rateLimitError = undefined; + _rateLimitError = undefined; dispatch.call('change'); if (callback) callback(err, res); that.userChangesets(function() {}); // eagerly load user details/changesets diff --git a/modules/ui/commit.js b/modules/ui/commit.js index e7d3837f5..be1ef0e72 100644 --- a/modules/ui/commit.js +++ b/modules/ui/commit.js @@ -16,9 +16,9 @@ import { utilDetect } from '../util/detect'; import { utilRebind } from '../util'; -var changeset; +var _changeset; var readOnlyTags = [ - /^changesets_count$/, + /^_changesets_count$/, /^created_by$/, /^ideditor:/, /^imagery_used$/, @@ -32,9 +32,9 @@ var hashtagRegex = /(#[^\u2000-\u206F\u2E00-\u2E7F\s\\'!"#$%()*,.\/:;<=>?@\[\]^` export function uiCommit(context) { - var dispatch = d3_dispatch('cancel', 'save'), - userDetails, - _selection; + var dispatch = d3_dispatch('cancel', 'save'); + var _userDetails; + var _selection; var changesetEditor = uiChangesetEditor(context) .on('change', changeTags); @@ -51,16 +51,16 @@ export function uiCommit(context) { if (!osm) return; // expire stored comment and hashtags after cutoff datetime - #3947 - var commentDate = +context.storage('commentDate') || 0, - currDate = Date.now(), - cutoff = 2 * 86400 * 1000; // 2 days + var commentDate = +context.storage('commentDate') || 0; + var currDate = Date.now(); + var cutoff = 2 * 86400 * 1000; // 2 days if (commentDate > currDate || currDate - commentDate > cutoff) { context.storage('comment', null); context.storage('hashtags', null); } var tags; - if (!changeset) { + if (!_changeset) { var detected = utilDetect(); tags = { comment: context.storage('comment') || '', @@ -78,12 +78,12 @@ export function uiCommit(context) { tags.hashtags = hashtags; } - changeset = new osmChangeset({ tags: tags }); + _changeset = new osmChangeset({ tags: tags }); } - tags = _clone(changeset.tags); + tags = _clone(_changeset.tags); tags.imagery_used = context.history().imageryUsed().join(';').substr(0, 255); - changeset = changeset.update({ tags: tags }); + _changeset = _changeset.update({ tags: tags }); var header = selection.selectAll('.header') .data([0]); @@ -114,7 +114,7 @@ export function uiCommit(context) { changesetSection .call(changesetEditor - .changesetID(changeset.id) + .changesetID(_changeset.id) .tags(tags) ); @@ -146,7 +146,7 @@ export function uiCommit(context) { var userLink = d3_select(document.createElement('div')); - userDetails = user; + _userDetails = user; if (user.image_url) { userLink @@ -195,7 +195,7 @@ export function uiCommit(context) { .merge(requestReviewEnter); var requestReviewInput = requestReview.selectAll('input') - .property('checked', isReviewRequested(changeset.tags)) + .property('checked', isReviewRequested(_changeset.tags)) .on('change', toggleRequestReview); @@ -238,7 +238,8 @@ export function uiCommit(context) { return (n && n.value.length) ? null : true; }) .on('click.save', function() { - dispatch.call('save', this, changeset); + this.blur(); // avoid keeping focus on the button - #4641 + dispatch.call('save', this, _changeset); }); @@ -256,7 +257,7 @@ export function uiCommit(context) { .call(rawTagEditor .expanded(expanded) .readOnlyTags(readOnlyTags) - .tags(_clone(changeset.tags)) + .tags(_clone(_changeset.tags)) ); @@ -273,7 +274,7 @@ export function uiCommit(context) { .call(rawTagEditor .expanded(expanded) .readOnlyTags(readOnlyTags) - .tags(_clone(changeset.tags)) + .tags(_clone(_changeset.tags)) ); } } @@ -299,8 +300,8 @@ export function uiCommit(context) { function findHashtags(tags, commentOnly) { - var inComment = commentTags(), - inHashTags = hashTags(); + var inComment = commentTags(); + var inHashTags = hashTags(); if (inComment !== null) { // when hashtags are detected in comment... context.storage('hashtags', null); // always remove stored hashtags - #4304 @@ -340,7 +341,7 @@ export function uiCommit(context) { function updateChangeset(changed, onInput) { - var tags = _clone(changeset.tags); + var tags = _clone(_changeset.tags); _forEach(changed, function(v, k) { k = k.trim().substr(0, 255); @@ -371,8 +372,8 @@ export function uiCommit(context) { } // always update userdetails, just in case user reauthenticates as someone else - if (userDetails && userDetails.changesets_count !== undefined) { - var changesetsCount = parseInt(userDetails.changesets_count, 10) + 1; // #4283 + if (_userDetails && _userDetails.changesets_count !== undefined) { + var changesetsCount = parseInt(_userDetails.changesets_count, 10) + 1; // #4283 tags.changesets_count = String(changesetsCount); // first 100 edits - new user @@ -397,14 +398,14 @@ export function uiCommit(context) { delete tags.changesets_count; } - if (!_isEqual(changeset.tags, tags)) { - changeset = changeset.update({ tags: tags }); + if (!_isEqual(_changeset.tags, tags)) { + _changeset = _changeset.update({ tags: tags }); } } commit.reset = function() { - changeset = null; + _changeset = null; }; diff --git a/modules/ui/confirm.js b/modules/ui/confirm.js index aa1238a20..2fd0e4383 100644 --- a/modules/ui/confirm.js +++ b/modules/ui/confirm.js @@ -27,7 +27,9 @@ export function uiConfirm(selection) { .on('click.confirm', function() { modalSelection.remove(); }) - .text(t('confirm.okay')); + .text(t('confirm.okay')) + .node() + .focus(); return modalSelection; }; diff --git a/modules/ui/conflicts.js b/modules/ui/conflicts.js index 2cfe3448e..885b05479 100644 --- a/modules/ui/conflicts.js +++ b/modules/ui/conflicts.js @@ -5,60 +5,92 @@ import { select as d3_select } from 'd3-selection'; +import { d3keybinding as d3_keybinding } from '../lib/d3.keybinding.js'; + import { t } from '../util/locale'; import { JXON } from '../util/jxon'; import { geoExtent } from '../geo'; import { osmChangeset } from '../osm'; import { svgIcon } from '../svg'; import { utilDetect } from '../util/detect'; -import { utilEntityOrMemberSelector } from '../util'; -import { utilRebind } from '../util/rebind'; + +import { + utilEntityOrMemberSelector, + utilRebind, + utilWrap +} from '../util'; export function uiConflicts(context) { - var dispatch = d3_dispatch('cancel', 'save'), - origChanges, - conflictList; + var dispatch = d3_dispatch('cancel', 'save'); + var keybinding = d3_keybinding('conflicts'); + var _origChanges; + var _conflictList; + + + function keybindingOn() { + d3_select(document) + .call(keybinding.on('⎋', cancel, true)); + } + + function keybindingOff() { + d3_select(document) + .call(keybinding.off); + } + + function tryAgain() { + keybindingOff(); + dispatch.call('save'); + } + + function cancel() { + keybindingOff(); + dispatch.call('cancel'); + } function conflicts(selection) { - var header = selection + keybindingOn(); + + var headerEnter = selection.selectAll('.header') + .data([0]) + .enter() .append('div') .attr('class', 'header fillL'); - header + headerEnter .append('button') .attr('class', 'fr') - .on('click', function() { dispatch.call('cancel'); }) + .on('click', cancel) .call(svgIcon('#icon-close')); - header + headerEnter .append('h3') .text(t('save.conflict.header')); - var body = selection + var bodyEnter = selection.selectAll('.body') + .data([0]) + .enter() .append('div') .attr('class', 'body fillL'); - var conflictsHelp = body + var conflictsHelpEnter = bodyEnter .append('div') .attr('class', 'conflicts-help') .text(t('save.conflict.help')); // Download changes link - var detected = utilDetect(), - changeset = new osmChangeset(); + var detected = utilDetect(); + var changeset = new osmChangeset(); - delete changeset.id; // Export without chnageset_id + delete changeset.id; // Export without changeset_id - var data = JXON.stringify(changeset.osmChangeJXON(origChanges)), - blob = new Blob([data], {type: 'text/xml;charset=utf-8;'}), - fileName = 'changes.osc'; + var data = JXON.stringify(changeset.osmChangeJXON(_origChanges)); + var blob = new Blob([data], { type: 'text/xml;charset=utf-8;' }); + var fileName = 'changes.osc'; - var linkEnter = conflictsHelp.selectAll('.download-changes') - .data([0]) - .enter() + var linkEnter = conflictsHelpEnter.selectAll('.download-changes') .append('a') .attr('class', 'download-changes'); @@ -81,44 +113,44 @@ export function uiConflicts(context) { .text(t('save.conflict.download_changes')); - body + bodyEnter .append('div') .attr('class', 'conflict-container fillL3') .call(showConflict, 0); - body + bodyEnter .append('div') .attr('class', 'conflicts-done') .attr('opacity', 0) .style('display', 'none') .text(t('save.conflict.done')); - var buttons = body + var buttonsEnter = bodyEnter .append('div') .attr('class','buttons col12 joined conflicts-buttons'); - buttons + buttonsEnter .append('button') - .attr('disabled', conflictList.length > 1) + .attr('disabled', _conflictList.length > 1) .attr('class', 'action conflicts-button col6') .text(t('save.title')) - .on('click.try_again', function() { dispatch.call('save'); }); + .on('click.try_again', tryAgain); - buttons + buttonsEnter .append('button') .attr('class', 'secondary-action conflicts-button col6') .text(t('confirm.cancel')) - .on('click.cancel', function() { dispatch.call('cancel'); }); + .on('click.cancel', cancel); } function showConflict(selection, index) { - if (index < 0 || index >= conflictList.length) return; + index = utilWrap(index, _conflictList.length); var parent = d3_select(selection.node().parentNode); // enable save button if this is the last conflict being reviewed.. - if (index === conflictList.length - 1) { + if (index === _conflictList.length - 1) { window.setTimeout(function() { parent.select('.conflicts-button') .attr('disabled', null); @@ -130,30 +162,33 @@ export function uiConflicts(context) { }, 250); } - var item = selection + var conflict = selection .selectAll('.conflict') - .data([conflictList[index]]); + .data([_conflictList[index]]); - var enter = item.enter() + conflict.exit() + .remove(); + + var conflictEnter = conflict.enter() .append('div') .attr('class', 'conflict'); - enter + conflictEnter .append('h4') .attr('class', 'conflict-count') - .text(t('save.conflict.count', { num: index + 1, total: conflictList.length })); + .text(t('save.conflict.count', { num: index + 1, total: _conflictList.length })); - enter + conflictEnter .append('a') .attr('class', 'conflict-description') .attr('href', '#') .text(function(d) { return d.name; }) .on('click', function(d) { - zoomToEntity(d.id); d3_event.preventDefault(); + zoomToEntity(d.id); }); - var details = enter + var details = conflictEnter .append('div') .attr('class', 'conflict-detail-container'); @@ -183,11 +218,13 @@ export function uiConflicts(context) { .attr('class', 'conflict-nav-button action col6') .attr('disabled', function(d, i) { return (i === 0 && index === 0) || - (i === 1 && index === conflictList.length - 1) || null; + (i === 1 && index === _conflictList.length - 1) || null; }) .on('click', function(d, i) { - var container = parent.select('.conflict-container'), - sign = (i === 0 ? -1 : 1); + d3_event.preventDefault(); + + var container = parent.selectAll('.conflict-container'); + var sign = (i === 0 ? -1 : 1); container .selectAll('.conflict') @@ -195,12 +232,8 @@ export function uiConflicts(context) { container .call(showConflict, index + sign); - - d3_event.preventDefault(); }); - item.exit() - .remove(); } @@ -211,14 +244,15 @@ export function uiConflicts(context) { .selectAll('li') .data(function(d) { return d.choices || []; }); - var enter = choices.enter() + // enter + var choicesEnter = choices.enter() .append('li') .attr('class', 'layer'); - var label = enter + var labelEnter = choicesEnter .append('label'); - label + labelEnter .append('input') .attr('type', 'radio') .attr('name', function(d) { return d.id; }) @@ -228,14 +262,18 @@ export function uiConflicts(context) { choose(ul, d); }); - label + labelEnter .append('span') .text(function(d) { return d.text; }); - choices + // update + choicesEnter + .merge(choices) .each(function(d, i) { var ul = this.parentNode; - if (ul.__data__.chosen === i) choose(ul, d); + if (ul.__data__.chosen === i) { + choose(ul, d); + } }); } @@ -249,8 +287,8 @@ export function uiConflicts(context) { .selectAll('input') .property('checked', function(d) { return d === datum; }); - var extent = geoExtent(), - entity; + var extent = geoExtent(); + var entity; entity = context.graph().hasEntity(datum.id); if (entity) extent._extend(entity.extent(context.graph())); @@ -275,8 +313,7 @@ export function uiConflicts(context) { } else { context.map().zoomTo(entity); } - context.surface().selectAll( - utilEntityOrMemberSelector([entity.id], context.graph())) + context.surface().selectAll(utilEntityOrMemberSelector([entity.id], context.graph())) .classed('hover', true); } } @@ -293,16 +330,16 @@ export function uiConflicts(context) { // choice(id, keepTheirs, forceRemote) // ] // } - conflicts.list = function(_) { - if (!arguments.length) return conflictList; - conflictList = _; + conflicts.conflictList = function(_) { + if (!arguments.length) return _conflictList; + _conflictList = _; return conflicts; }; conflicts.origChanges = function(_) { - if (!arguments.length) return origChanges; - origChanges = _; + if (!arguments.length) return _origChanges; + _origChanges = _; return conflicts; }; diff --git a/modules/ui/intro/welcome.js b/modules/ui/intro/welcome.js index 8187998e7..81e1f9b09 100644 --- a/modules/ui/intro/welcome.js +++ b/modules/ui/intro/welcome.js @@ -137,7 +137,7 @@ export function uiIntroWelcome(context, reveal) { ); } - + chapter.enter = function() { welcome(); }; @@ -145,7 +145,7 @@ export function uiIntroWelcome(context, reveal) { chapter.exit = function() { listener.off(); - var tooltip = d3_select('.curtain-tooltip.intro-mouse') + d3_select('.curtain-tooltip.intro-mouse') .selectAll('.counter') .remove(); }; diff --git a/modules/ui/loading.js b/modules/ui/loading.js index 0b567fc71..899c56ae6 100644 --- a/modules/ui/loading.js +++ b/modules/ui/loading.js @@ -1,16 +1,17 @@ +import { select as d3_select } from 'd3-selection'; import { uiModal } from './modal'; export function uiLoading(context) { - var message = '', - blocking = false, - modalSelection; + var _modalSelection = d3_select(null); + var _message = ''; + var _blocking = false; var loading = function(selection) { - modalSelection = uiModal(selection, blocking); + _modalSelection = uiModal(selection, _blocking); - var loadertext = modalSelection.select('.content') + var loadertext = _modalSelection.select('.content') .classed('loading-modal', true) .append('div') .attr('class', 'modal-section fillL'); @@ -22,9 +23,9 @@ export function uiLoading(context) { loadertext .append('h3') - .text(message); + .text(_message); - modalSelection.select('button.close') + _modalSelection.select('button.close') .attr('class', 'hide'); return loading; @@ -32,21 +33,21 @@ export function uiLoading(context) { loading.message = function(_) { - if (!arguments.length) return message; - message = _; + if (!arguments.length) return _message; + _message = _; return loading; }; loading.blocking = function(_) { - if (!arguments.length) return blocking; - blocking = _; + if (!arguments.length) return _blocking; + _blocking = _; return loading; }; loading.close = function() { - modalSelection.remove(); + _modalSelection.remove(); };