-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Resolving Changeset already inflight error when editing with 2 layers w/in map #1427
base: hoot2x
Are you sure you want to change the base?
Changes from 6 commits
960b532
eb95c0b
ce6f3cf
8d9ab10
fac2925
14cf7f2
70c4f1d
7e89286
bd72f67
7faae87
ae33772
16e099a
16b2fd8
7a390cb
5c988cb
875eac7
d7182f4
87d9353
a741581
fd82f8f
ba9dda2
1f0ec99
42af31c
21d8219
76fabe0
305efc5
2b2693b
7e8a0de
2f78207
73f53a4
0ec1437
3207b64
44371a1
3819c61
1e3bda4
e2179df
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -327,6 +327,32 @@ export default class API { | |
} ); | ||
} | ||
|
||
|
||
/** | ||
* Get all id's necessary to create new map features | ||
* | ||
* @param {Promise|array} mapId + changesetId, nodeId, wayId, relationId | ||
*/ | ||
getAllIds( mapId ) { | ||
const params = { | ||
path: `/osm/api/0.6/map/${ mapId }/startingIndex`, | ||
method: 'GET' | ||
}; | ||
|
||
return this.request( params ) | ||
.then( resp => { | ||
|
||
let activeIds = {}; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think just return resp.data! |
||
activeIds = resp.data; | ||
return activeIds; | ||
} ) | ||
.catch( err => { | ||
const message = this.internalError( err ) || 'Unable to retrieve all map ids'; | ||
|
||
return Promise.reject( message ); | ||
} ); | ||
} | ||
|
||
/** | ||
* Get all layer links from the database | ||
* | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,6 +15,7 @@ import { | |
utilQsString, | ||
utilStringQs | ||
} from '../../../util'; | ||
import { diff3MergeIndices } from 'node-diff3'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. think we can remove this import! |
||
|
||
/** | ||
* Create the sidebar | ||
|
@@ -126,7 +127,7 @@ export default class Sidebar { | |
|
||
form.controller.update(); | ||
form.loadingLayerName = null; | ||
|
||
this.saveChanges(); | ||
this.conflateCheck(); | ||
} | ||
} ); | ||
|
@@ -168,6 +169,122 @@ export default class Sidebar { | |
this.adjustSize(); | ||
} | ||
|
||
saveChanges() { | ||
let loadedLayers = Object.values(Hoot.layers.loadedLayers); | ||
let selectReference = d3.selectAll('#reference'); | ||
let selectSecondary = d3.selectAll('#secondary'); | ||
if (loadedLayers.length === 2) { | ||
let referenceActive = loadedLayers[0]; | ||
let secondaryActive = loadedLayers[1]; | ||
let changeActive = new LayerAdd(); | ||
let referenceState; | ||
let secondaryState; | ||
|
||
|
||
if (d3.select('#reference button.select-active-layer').empty()) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. wonder if the on click callback can be generalized in a single function? seems like when we click one fo the active layer buttons, we make the one we click's classes all set to active, and the other one's set to false. so a function could have 2 params, one we want to set to active, one we want to disable and just use those accordingly. so naively, something like function update(active, inactive) {
//...update active button class
//...update inactive button
} |
||
selectReference | ||
.append('button') | ||
.classed('select-active-layer', true) | ||
.text('Set as active layer') | ||
.on('click', function () { | ||
|
||
d3.event.preventDefault(); | ||
|
||
d3.select('#reference') | ||
.classed('active-pulse', true); | ||
|
||
d3.select('#secondary') | ||
.classed('active-pulse', false); | ||
|
||
d3.select('#reference button.select-active-layer') | ||
.text('Active Layer') | ||
.classed('button-active', true); | ||
|
||
d3.select('#secondary button.select-active-layer') | ||
.text('Set as active layer') | ||
.classed('button-active', false); | ||
|
||
d3.selectAll('#secondary div.controller') | ||
.classed('disable-non-active', true); | ||
|
||
d3.selectAll('#reference div.controller') | ||
.classed('disable-non-active', false); | ||
|
||
d3.selectAll('#secondary button.delete-button') | ||
.classed('disable-non-active', true) | ||
.classed('no-click', true); | ||
|
||
d3.selectAll('#reference button.delete-button') | ||
.classed('disable-non-active', false) | ||
.classed('no-click', false); | ||
|
||
|
||
referenceState = referenceActive; | ||
|
||
if (secondaryState) { | ||
secondaryState.activeLayer = false; | ||
referenceState.activeLayer = true; | ||
} | ||
else { | ||
referenceState.activeLayer = true; | ||
} | ||
changeActive.selectedLayer = referenceState; | ||
}); | ||
} | ||
|
||
if (d3.select('#secondary button.select-active-layer').empty()) { | ||
selectSecondary | ||
.append('button') | ||
.classed('select-active-layer', true) | ||
.text('Set as active layer') | ||
.on('click', function () { | ||
|
||
d3.event.preventDefault(); | ||
d3.select('#secondary') | ||
.classed('active-pulse', true); | ||
|
||
d3.select('#reference') | ||
.classed('active-pulse', false); | ||
|
||
d3.select('#secondary button.select-active-layer') | ||
.text('Active Layer') | ||
.classed('button-active', true); | ||
|
||
d3.select('#reference button.select-active-layer') | ||
.text('Set as active layer') | ||
.classed('button-active', false); | ||
|
||
d3.selectAll('#reference div.controller') | ||
.classed('disable-non-active', true); | ||
|
||
d3.selectAll('#secondary div.controller') | ||
.classed('disable-non-active', false); | ||
|
||
d3.selectAll('#reference button.delete-button') | ||
.classed('disable-non-active', true) | ||
.classed('no-click', true); | ||
|
||
d3.selectAll('#secondary button.delete-button') | ||
.classed('disable-non-active', false) | ||
.classed('no-click', false); | ||
|
||
secondaryState = secondaryActive; | ||
|
||
if (referenceState) { | ||
referenceState.activeLayer = false; | ||
secondaryState.activeLayer = true; | ||
} | ||
else { | ||
secondaryState.activeLayer = true; | ||
} | ||
|
||
changeActive.selectedLayer = secondaryState; | ||
|
||
}); | ||
} | ||
} | ||
} | ||
|
||
conflateCheck() { | ||
let loadedLayers = Object.values( Hoot.layers.loadedLayers ), | ||
addControllers = d3.selectAll( '.add-controller' ); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,6 +16,7 @@ import { geoChooseEdge, geoHasSelfIntersections } from '../geo'; | |
import { modeBrowse, modeSelect } from '../modes'; | ||
import { osmNode } from '../osm'; | ||
import { utilKeybinding } from '../util'; | ||
import _find from 'lodash-es/find'; | ||
|
||
|
||
export function behaviorDrawWay(context, wayId, index, mode, startGraph) { | ||
|
@@ -73,6 +74,8 @@ export function behaviorDrawWay(context, wayId, index, mode, startGraph) { | |
function move(datum) { | ||
context.surface().classed('nope-disabled', d3_event.altKey); | ||
|
||
// add one more boolean at end, is activeLayer===mapId | ||
var _activeLayer = _find( Hoot.layers.loadedLayers, function(a, b) { return a.activeLayer; }); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we need to make use of this here, making sure to only apply a target loc if active ids match. here in iD a question is asked (is this target loc one that allows vertex). Our question needs to be |
||
var targetLoc = datum && datum.properties && datum.properties.entity && datum.properties.entity.loc; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we need to implement the commented out piece here, using the same split stuff we do in the draw.js file. |
||
var targetNodes = datum && datum.properties && datum.properties.nodes; | ||
var loc = context.map().mouseCoordinates(); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -140,6 +140,7 @@ export function behaviorHover(context) { | |
return; | ||
} | ||
|
||
// again, boolean, is entity.mapId === activeLayerId... | ||
var suppressed = _altDisables && d3_event && d3_event.altKey; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. need to do same |
||
_selection.selectAll(selector) | ||
.classed(suppressed ? 'hover-suppressed' : 'hover', true); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a reminder, let's update this comment!