Drag and drop of semicombos

[TAQ2]

#5728

Drag and drop working fine on full-width tags i.e. Destination but slightly different for the other inputs e.g. Destination Road Numbers. Please can I get some opinions on the code and the functionality

Questions:

1. Am I supposed to have added tests?
2. Am I supposed to have added documentation? It would make sense to have it somewhere when the i button is clicked
3. Is this the correct place to put this code? Its similar functionality to the drag and drop for relations. But seems a bit premature to abstract it out.
4. I haven't looked into testing mobile - is this something that iD supports?
5. I am trying to merge to master - not sure if this is correct as I've branched from 2.x . Is master version 3? If so I can't test because master is broken see below

[TAQ2]

Parse error at dist/iD.js:41858,7
	class CountryCoder {
	      ^
ERROR: Unexpected token: name «CountryCoder», expected: punc «;»
    at JS_Parse_Error.get (eval at <anonymous> (/home/travis/build/openstreetmap/iD/node_modules/uglify-js/tools/node.js:18:1),

I have not touched any variable named CountryCoder. Seems like even greenkeeper builds are failing on this error

@quincylvania mind helping me out :)

[quincylvania]

@TAQ2 Thanks for working on this! I'll have to go through the code later but it seems like a great start.

Am I supposed to have added tests?

Not necessarily, we don't have tests for most stuff in iD right now, particularly in the UI.

Am I supposed to have added documentation? It would make sense to have it somewhere when the i button is clicked

Nope, I think it's fine to let people discover this feature on their own.

Is this the correct place to put this code? Its similar functionality to the drag and drop for relations. But seems a bit premature to abstract it out.

Yes, seems fine. No need to abstract this.

I haven't looked into testing mobile - is this something that iD supports?

Mobile support is a nice-to-have in iD at the moment but not critical. We'll do a full push toward mobile support sometime in the future.

I am trying to merge to master - not sure if this is correct as I've branched from 2.x . Is master version 3? If so I can't test because master is broken see below

master is v3. 2.x is the more "stable" branch. There's no reason this has to wait for v3 so I'd recommend targeting 2.x.

Seems like even greenkeeper builds are failing on this error

Yes, this is because of an error unrelated to your work. This sometimes happens on the development branches. See #7009.

[TAQ2]

@quincylvania thanks for answering those for me. What is the status of this? Did you want me to move it to 2.x branch first? I assumed that you wanted to look at the code first perhaps?

[quincylvania]

@TAQ2 Hi, apologies for letting this go stale! I haven't reviewed your code in detail but the structure looks fine. Could you resubmit targeting the up-to-date 2.x branch? Then we can get it merged.

[TAQ2]

Ok done, I'm not sure how a few of you how a few of your commits landed on here. Please advise as I don't know how to get rid of them

[quincylvania]

Ok done, I'm not sure how a few of you how a few of your commits landed on here. Please advise as I don't know how to get rid of them

Hmm I don't know either! Looks like you're still targeting the master branch… you might try closing this PR and creating a new branch off of 2.x re-adding your changes.

[kymckay]

Those commits should disappear if you edit the PR on github (to right of the title) and target it to 2.x

It's because they're not in master

[quincylvania]

Those commits should disappear if you edit the PR on github (to right of the title) and target it to 2.x

It's because they're not in master

@SilentSpike Thanks for the tip!

[TAQ2]

Thanks for the help

[quincylvania]

@TAQ2 I tested this out and it looks pretty good! I merged it as-is but I'm going to make a few tweaks on my end. Thanks again for your work! 🎉