Skip to content
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

Self intersection check is still too aggressive #4787

Closed
hikemaniac opened this issue Feb 11, 2018 · 10 comments
Closed

Self intersection check is still too aggressive #4787

hikemaniac opened this issue Feb 11, 2018 · 10 comments
Labels
considering Not Actionable - still considering if this is something we want

Comments

@hikemaniac
Copy link
Contributor

Recently I stumbled upon some cases where the self intersection check complicates mapping.

self intersection problems

I actually just wanted to continue mapping this field and connect its new nodes to the scrub area but iD wouldn't let me.

@manfredbrandl
Copy link
Contributor

May I propose a different strategy to avoid self intersecting lines and areas? iD could check modified objects after the user unselected them. If there are Warnings or Errors detected a new field right to the Save Button could appear and show that the next Save will have Warning/Errors and maybe how many in colour and/or number. Clicking on this field could switch to the list of warnings presented after Save is clicked. The change of cursor could appear while editing to warn burn other to block.

@bhousel
Copy link
Member

bhousel commented Feb 12, 2018

If there are Warnings or Errors detected a new field right to the Save Button could appear and show that the next Save will have Warning/Errors and maybe how many in

I'm ok with this, especially combined with small on map warning icon like described here ?
#4776 (comment)

I really think I could use that UI pattern to just solve a lot of the most common "iD makes it too easy to / iD makes it too hard to" issues that keep coming up.

@bhousel bhousel added the considering Not Actionable - still considering if this is something we want label Feb 12, 2018
@bhousel bhousel changed the title Self intersection check is still to aggresive Self intersection check is still too aggressive Feb 12, 2018
@bhousel
Copy link
Member

bhousel commented Feb 12, 2018

@hikemaniac you probably already know this already but - in the mean time until decide on the appropriate fix, you can just move your node here, and then move the other nodes wherever you want to.

Yes avoiding self intersections is an extra step, but I don't think the current checks really prevent you from doing what you are trying to do...

screenshot 2018-02-11 21 28 03

@hikemaniac
Copy link
Contributor Author

You probably already know this already but - in the mean time until decide on the appropriate fix, you can just move your node here, and then move the other nodes wherever you want to.

Yep, I already did this and successfully uploaded my changeset 😊 but I think that the self-intersection checks complicate mapping in such cases.
By the way (as already reported in #4731) the performance when drawing large areas and lines becomes really bad at some point and this even crashed my browser once. So I believe that @manfredbrandl's idea would really help a lot.

@manfredbrandl
Copy link
Contributor

I'm ok with this, especially combined with small on map warning icon like described here ?
#4776 (comment)

I appreciate warning icons on the map, they point you to the exact position of the problem and adds another usefull feature with and without the new field right to the Save button.

@bhousel
Copy link
Member

bhousel commented Feb 12, 2018

One idea: we put them at the top next to save, and also on screen
screenshot 2018-02-12 11 17 42

Another idea (I like this better)
Add a panel on the side, maybe w/badge showing the number of warnings.. Users can click on the list to see and take them to the thing to fix. The panel gives us more opportunity to show/hide certain things (let user control what they want to see). Badges are now an accepted UI pattern.

screenshot 2018-02-12 10 22 56

@slhh
Copy link
Contributor

slhh commented Feb 13, 2018

I actually just wanted to continue mapping this field and connect its new nodes to the scrub area but iD wouldn't let me.

You can override the intersection check by holding the Alt key down. Its fair to require this because a new error is at least temporarily introduced. Maybe, we can show a hint to use the Alt modifier, e.g. where the shortcut errors are displayed, when placing of the node is disabled due to an intersection.

In addition dragging of a node doesn't seem to be the right tool for the usecase. We should have a partial redraw tool for a part of a way.
Idea for the UI:
Drag the midpoint at an edge to indicate that this edge should be reshaped, and drag it on one of the nodes of the edge to start redrawing from that node. This should put the user in the drawWay mode with the other node of the original edge being the target. Further nodes of the way in the same direction can serve as alternate targets to finish, which would remove the skipped nodes of the original way.

@bhousel I like the on map warning concept in general, but we should not overuse it.

@manfredbrandl
Copy link
Contributor

Another idea (I like this better)

That's really better. @bhousel do you plan to have the same UI for resolving warnings from the panel on the side and from the Save button? I think that would make the UI simpler.

@bhousel
Copy link
Member

bhousel commented Feb 13, 2018

That's really better. @bhousel do you plan to have the same UI for resolving warnings from the panel on the side and from the Save button? I think that would make the UI simpler.

Yeah, I hadn't actually thought it through very much, and I just threw those screenshots together pretty quickly. I don't know what I'm doing 😄 I initially thought a button at the top, then the idea changed to another pane on the side, which opens up a lot more options to explain / navigate to / filter the warnings.

@bhousel
Copy link
Member

bhousel commented Feb 6, 2019

Something like the issue manager is coming soon in #5830 so I'm going to close here, and we can open a separate issue for validating self-crossing lines.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
considering Not Actionable - still considering if this is something we want
Projects
None yet
Development

No branches or pull requests

4 participants