Skip to content

Commit

Permalink
Cache disabled() results in straighten action for consistent response
Browse files Browse the repository at this point in the history
What could happen was:
- user could right click on a line
- this would trigger `disabled()` checks for each operation buttons
- the line was not fully downloaded, so would return `disabled()` 'not_downloaded'
  (and also start download of the missing tiles)
- then the tooltip would pop into existence, calling `tooltip()`
- which calls `disabled()` again
- but this time it's fine and the `disabled()` is false
- so you'd see a greyed out button but the tooltip said everyting is ok and
  you can click the button anyway

I fixed this by just caching the disabled test.  This is probably ok anyway
because these tests can be expensive, and then the user will see a consistent
message like "The line is not yet fully downloaded".

If the user clicks off the line and back onto it, iD will reenter select mode,
rebuild the menu, redo the disabled test, and they will see the button enabled.
  • Loading branch information
bhousel committed Apr 10, 2019
1 parent e300909 commit 81127d7
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 65 deletions.
51 changes: 24 additions & 27 deletions modules/core/context.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,62 +108,59 @@ export function coreContext() {
};


function wrapcb(callback, cid) {
function afterLoad(cid, callback) {
return function(err, result) {
if (err) {
// 400 Bad Request, 401 Unauthorized, 403 Forbidden..
if (err.status === 400 || err.status === 401 || err.status === 403) {
connection.logout();
if (connection) {
connection.logout();
}
}
return callback.call(context, err);
if (typeof callback === 'function') {
callback(err);
}
return;

} else if (connection.getConnectionId() !== cid) {
return callback.call(context, { message: 'Connection Switched', status: -1 });
} else if (connection && connection.getConnectionId() !== cid) {
if (typeof callback === 'function') {
callback({ message: 'Connection Switched', status: -1 });
}
return;

} else {
return callback.call(context, err, result);
history.merge(result.data, result.extent);
if (typeof callback === 'function') {
callback(err, result);
}
return;
}
};
}


context.loadTiles = function(projection, callback) {
var cid;
function done(err, result) {
if (!err) history.merge(result.data, result.extent);
if (callback) callback(err, result);
}
if (connection && context.editable()) {
cid = connection.getConnectionId();
var cid = connection.getConnectionId();
utilCallWhenIdle(function() {
connection.loadTiles(projection, wrapcb(done, cid));
connection.loadTiles(projection, afterLoad(cid, callback));
})();
}
};

context.loadTileAtLoc = function(loc, callback) {
var cid;
function done(err, result) {
if (!err) history.merge(result.data, result.extent);
if (callback) callback(err, result);
}
if (connection && context.editable()) {
cid = connection.getConnectionId();
var cid = connection.getConnectionId();
utilCallWhenIdle(function() {
connection.loadTileAtLoc(loc, wrapcb(done, cid));
connection.loadTileAtLoc(loc, afterLoad(cid, callback));
})();
}
};

context.loadEntity = function(entityID, callback) {
var cid;
function done(err, result) {
if (!err) history.merge(result.data, result.extent);
if (callback) callback(err, result);
}
if (connection) {
cid = connection.getConnectionId();
connection.loadEntity(entityID, wrapcb(done, cid));
var cid = connection.getConnectionId();
connection.loadEntity(entityID, afterLoad(cid, callback));
}
};

Expand Down
18 changes: 4 additions & 14 deletions modules/modes/select.js
Original file line number Diff line number Diff line change
@@ -1,21 +1,12 @@
import {
event as d3_event,
select as d3_select
} from 'd3-selection';
import { event as d3_event, select as d3_select } from 'd3-selection';

import { t } from '../util/locale';

import { actionAddMidpoint } from '../actions';

import {
behaviorBreathe,
behaviorCopy,
behaviorHover,
behaviorLasso,
behaviorPaste,
behaviorSelect
behaviorBreathe, behaviorCopy, behaviorHover,
behaviorLasso, behaviorPaste, behaviorSelect
} from '../behavior';

import { geoExtent, geoChooseEdge, geoPointInPolygon } from '../geo';
import { modeBrowse } from './browse';
import { modeDragNode } from './drag_node';
Expand All @@ -24,7 +15,6 @@ import { osmNode, osmWay } from '../osm';
import * as Operations from '../operations/index';
import { uiEditMenu, uiSelectionList } from '../ui';
import { uiCmd } from '../ui/cmd';

import {
utilArrayIntersection, utilEntityOrMemberSelector,
utilEntitySelector, utilKeybinding
Expand Down Expand Up @@ -148,7 +138,7 @@ export function modeSelect(context, selectedIDs) {


function positionMenu() {
if (!editMenu) { return; }
if (!editMenu) return;

var entity = singular();
if (entity && context.geometry(entity.id) === 'relation') {
Expand Down
18 changes: 12 additions & 6 deletions modules/operations/straighten.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { utilArrayDifference, utilGetAllNodes } from '../util/index';


export function operationStraighten(selectedIDs, context) {
var _disabled;
var action = actionStraighten(selectedIDs, context.projection);
var wayIDs = selectedIDs.filter(function(id) { return id.charAt(0) === 'w'; });
var nodes = utilGetAllNodes(wayIDs, context.graph());
Expand Down Expand Up @@ -63,16 +64,21 @@ export function operationStraighten(selectedIDs, context) {


operation.disabled = function() {
var reason = action.disabled(context.graph());
if (reason) {
return reason;
if (_disabled !== undefined) return _disabled;

_disabled = action.disabled(context.graph());
if (_disabled) {
return _disabled;
} else if (someMissing()) {
return 'not_downloaded';
_disabled = 'not_downloaded';
return _disabled;
} else if (selectedIDs.some(context.hasHiddenConnections)) {
return 'connected_to_hidden';
_disabled = 'connected_to_hidden';
return _disabled;
}

return false;
_disabled = false;
return _disabled;


function someMissing() {
Expand Down
37 changes: 19 additions & 18 deletions modules/ui/edit_menu.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
import {
event as d3_event,
select as d3_select
} from 'd3-selection';
import { event as d3_event, select as d3_select } from 'd3-selection';

import { geoVecFloor } from '../geo';
import { geoVecAdd, geoVecFloor } from '../geo';
import { textDirection } from '../util/locale';
import { uiTooltipHtml } from './tooltipHtml';

Expand Down Expand Up @@ -51,7 +48,7 @@ export function uiEditMenu(context, operations) {
offset[1] = -1 * (center[1] + menuHeight - viewport.height + vpBottomMargin);
}

var origin = [ center[0] + offset[0], center[1] + offset[1] ];
var origin = geoVecAdd(center, offset);

menu = selection
.append('g')
Expand All @@ -75,19 +72,17 @@ export function uiEditMenu(context, operations) {


var button = menu.selectAll('.edit-menu-item')
.data(operations)
.enter()
.data(operations);

// enter
var buttonEnter = button.enter()
.append('g')
.attr('class', function (d) { return 'edit-menu-item edit-menu-item-' + d.id; })
.classed('disabled', function (d) { return d.disabled(); })
.attr('transform', function (d, i) {
return 'translate(' + geoVecFloor([
0,
m + i * buttonHeight
]).join(',') + ')';
.attr('transform', function(d, i) {
return 'translate(' + geoVecFloor([0, m + i * buttonHeight]).join(',') + ')';
});

button
buttonEnter
.append('rect')
.attr('x', 4)
.attr('width', buttonWidth)
Expand All @@ -97,13 +92,19 @@ export function uiEditMenu(context, operations) {
.on('mouseover', mouseover)
.on('mouseout', mouseout);

button
buttonEnter
.append('use')
.attr('width', '20')
.attr('height', '20')
.attr('transform', function () { return 'translate(' + [2 * p, 5] + ')'; })
.attr('xlink:href', function (d) { return '#iD-operation-' + d.id; });

// update
button = buttonEnter
.merge(button)
.classed('disabled', function(d) { return d.disabled(); });


tooltip = d3_select('#id-container')
.append('div')
.attr('class', 'tooltip-inner edit-menu-tooltip');
Expand Down Expand Up @@ -171,9 +172,9 @@ export function uiEditMenu(context, operations) {
};


editMenu.center = function (_) {
editMenu.center = function(val) {
if (!arguments.length) return center;
center = _;
center = val;
return editMenu;
};

Expand Down

0 comments on commit 81127d7

Please sign in to comment.