When I try to square a building that is not completely in view, and note comes up, and the building is not square (this is good). When I move the map to include the whole building, however, the building is still considered "not completely visible" by iD. This seems to be something with iD not updating the status (completely visible or not) when I move the map.
@BjornRasmussen Could you post an example location? I couldn't reproduce this.
Sure @quincylvania!
To reproduce:
1) Go to 35.94648/-79.05480
2) Try to square the building to remove the excess node.
3) Observe that the building, which is not completely visible, can't be squared.
4) Move the map to include the entire building while keeping the building selected
5) Try to square the building (using Q, not a right click) now that it is completely visible. iD will prevent the building from being squared because "not enough of it is currently visible"
iD should recheck if a building is visible when the map is moved.
@BjornRasmussen Thanks for the detailed steps! This makes sense, the operations are loaded upon entering modeSelect, so if you don't deselect the building then they're not updated.
@BjornRasmussen I fixed this for squaring and for other affected operations. I just removed cacheing of the disable state since it's not particularly expensive to calculate and is called infrequently.
I was caching these because of https://github.com/openstreetmap/iD/commit/81127d71f3cdc431f292952006dc1751d34b8365
I do think it's important for the tooltip message to match the state of the operation. (i.e. it's not ok for the tooltip to say "Not available because it's not downloaded" when the operation is available and obviously not greyed out. )
I'm reopening just so that I can look into that above issue.. I'm sure there is a better way to do it than caching the status (maybe just for "undownloaded" status, poll again a little while later).
@bhousel Sounds good. I actually left the disabled cacheing for the straighten operation since it doesn't look at the extent, but I agree there's probably a better way to address that.
@bhousel Sounds good. I actually left the
disabledcacheing for the straighten operation since it doesn't look at the extent, but I agree there's probably a better way to address that.
Cool - yeah I think this incongruity can happen for any operation that can possibly request more data from another tile. It's very easy to trigger for operationStraighten and a lot harder (but possible) for all the other ones too. I'll figure something out better than caching.
Most helpful comment
Cool - yeah I think this incongruity can happen for any operation that can possibly request more data from another tile. It's very easy to trigger for
operationStraightenand a lot harder (but possible) for all the other ones too. I'll figure something out better than caching.