Skip to content

Marker drag check #594

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

Merged
merged 8 commits into from
Jun 20, 2020
Merged

Conversation

Falke-Design
Copy link
Collaborator

Check if marker has dragging property before enable / disable dragging

Fix #567

When a marker is inactive (interactive: false) nothing can be done with it, but other layers still snapping to it. Maybe snapping should be also removed.

Copy link
Contributor

@codeofsumit codeofsumit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Two things I'm missing

  • Add a test to the marker test suite if dragging was enabled/not enabled to a marker set to interactive: false
  • Resolve merge conflicts

Great work 👍

@Falke-Design
Copy link
Collaborator Author

@codeofsumit Both things added

Copy link
Contributor

@codeofsumit codeofsumit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand was is being tested in that test 🤔
Can you explain it?

@Falke-Design
Copy link
Collaborator Author

Yes it is difficult to understand.
Before this changes, when a interactive Marker was added to map and then edit mode was enabled, it thrown a error because the marker was not draggable.

With this test it only checks if a error is thrown when goes into edit mode.

@Falke-Design
Copy link
Collaborator Author

Can a cypress test on github restarted? It stopped before tested ...
Local all tests are passed.

@codeofsumit codeofsumit merged commit 6ab4ff5 into geoman-io:develop Jun 20, 2020
@Falke-Design Falke-Design deleted the marker_drag_check branch July 5, 2020 13:32
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