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

show deleted ways #310

Merged
merged 1 commit into from
Jun 9, 2020
Merged

Conversation

geohacker
Copy link
Member

This PR completes #296
cc @LanesGood

@geohacker geohacker requested a review from LanesGood June 8, 2020 12:49
@LanesGood
Copy link
Member

@geohacker it appears the last two nodes of the way still exist, even though the way itself is deleted. Can you confirm whether this is expected behavior?

@geohacker
Copy link
Member Author

@LanesGood yes, that's right. That's how deleted POIs also show up so I just matched the same. There'll be pendingDeleteUpload = true property on these features. So we could style these differently.

@LanesGood
Copy link
Member

@geohacker this is the current case of deleting two nodes from a 4-node closed polygon:

way-poly-delete

Should we aim instead for the iD behavior for deleting a closed feature entirely if there are not three nodes?

@geohacker
Copy link
Member Author

Should we aim instead for the iD behavior for deleting a closed feature entirely if there are not three nodes?

Thanks for pointing this out @LanesGood. I checked iD again and it's doing what you recommended. Deleting the way if it's a polygon and less than 3 nodes. @batpad do you think this will fit within the workflow of how we manage lines?

@geohacker geohacker changed the base branch from edit-ways to fix/del-invalid-polys June 9, 2020 13:08
@geohacker geohacker merged commit 7fc9673 into fix/del-invalid-polys Jun 9, 2020
@geohacker geohacker deleted the fix/show-deleted-ways branch June 9, 2020 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants