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.

Sign up for GitHub

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

Jump to bottom

Make vertices draggable and droppable #268

Open
wants to merge 28 commits into
base: main
Choose a base branch
from
Open

Make vertices draggable and droppable #268

wants to merge 28 commits into from

Conversation

merydian
Copy link
Collaborator

This includes all new changes, makes #222 obsolete, closes #204 and is related to #206.

@TheGreatRefrigerator
Copy link
Collaborator

Hey @merydian you leaked your API key in this branch. Best remove it from the commit, force push to the branch, delete the key from your account and create a new one :)

@MichaelsJP
Copy link
Member

Incredible work @merydian !

Only comment from my side, it would be cool if the preview feature can work on an existing list of points when toggled.

  • When enabled, the route preview is generated on the set points.
  • When disabled, the points remain but switch to the default flight of the crow representation.

@merydian merydian marked this pull request as ready for review November 13, 2024 13:40
@koebi
Copy link
Collaborator

koebi commented Nov 15, 2024

Hey,
I played around with this on QGIS 3.34.12, these are my findings so far:

  • Toggling LivePreview when no valid provider is set deletes all nodes.
  • Once a list is "final" (i.e. once I have double-clicked), is there a way to drag/drop nodes? I haven't found one…
  • The MapTool doesn't switch back after double-clicking. I figured that was fixed at some point?

@koebi
Copy link
Collaborator

koebi commented Nov 22, 2024

Two more comments:

  • Can we enable dragging points when the list is "final"?
  • Is there a way to display points while they are being dragged, and to update the RubberBand?

koebi
koebi requested changes Nov 22, 2024
View reviewed changes
Copy link
Collaborator

@koebi koebi left a comment

Choose a reason for hiding this comment

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

Two more general comments:

  • on profile change, the preview should be re-generated
  • tests are missing.

As for the translation:
As usually, you can provide the tr()-functions, I will provide the translation.

ORStools/gui/ORStoolsDialog.py Outdated Show resolved Hide resolved
ORStools/gui/ORStoolsDialog.py Outdated
Comment on lines 895 to 896
"Please use a different point",
"""Could not find routable point within a radius of 350.0 meters of specified coordinate. Use a different point closer to a road.""",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Translation is missing.

ORStools/gui/ORStoolsDialog.py Outdated
Comment on lines 893 to 894
"""Could not find routable point within a radius of 350.0 meters of specified coordinate.
Use a different point closer to a road.""", level=Qgis.MessageLevel.Warning, duration=3)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Translation is missing

ORStools/gui/ORStoolsDialog.py Outdated Show resolved Hide resolved
ORStools/gui/ORStoolsDialog.py Outdated
Comment on lines 894 to 898
"Please use a different point",
"""Could not find routable point within a radius of 350.0 meters of specified coordinate.
Use a different point closer to a road.""", level=Qgis.MessageLevel.Warning, duration=3)
Use a different point closer to a road.""",
level=Qgis.MessageLevel.Warning,
duration=3,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Translation is missing

ORStools/ORStoolsDialogUI.py Outdated Show resolved Hide resolved
ORStoolsDialogUI.py Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

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

See comment on ORStools/ORStoolsDialogUI.py.

ORStools/utils/router.py
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems to be the functionality currently found in run_gui_control in ORStoolsDialog.py.
Can these two functions be merged together?
There is a bit more error handling in the other as well, such as checking for duplicates.

ORStools/gui/ORStoolsDialog.py Outdated Show resolved Hide resolved
ORStools/gui/ORStoolsDialog.py Outdated
Comment on lines 916 to 918
"Please use a different point",
"""Could not find routable point within a radius of 350.0 meters of specified coordinate.
Use a different point closer to a road.""",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Translation missing

@koebi
Copy link
Collaborator

koebi commented Nov 22, 2024

Moreover, we should look at getting this combined with the ESC/Right-Click feat merged yesterday.

merydian and others added 20 commits December 2, 2024 12:56
@merydian
Copy from other branch and make it work
9d74f0f
@merydian
feat: remove rubberband when annotations are deleted
fbe34e3
@merydian
style: run ruff
d3c3c7a
@merydian
docs: add changelog entry
72b8df4
@merydian
feat: drop shapely dependency
72e291d
@merydian
fix: reset if api key not set
d2661c8
@merydian
style: some refactoring
ce58295
@merydian
fix: points not draggable after 2010 error
3929e8b
@merydian
feat: reload rubber band when live preview is toggled
f60ffa2
@merydian
refactor: some methods to reuse code
d788b3a
@merydian
fix: Use message bar instead of box
38d2348
@merydian
fix: error with livePreview toggle and empty routing list
3cd13d2
@merydian
style: run ruff
47bc9c4
@merydian
fix: message bar when connection refused
0c8f763
@merydian
fix: maptool not reset after double click
1c1351f
@merydian
feat: use tr method to translate user prompts
3e175de
@merydian
feat: reload rubber band on profile change
e85c3bb
@merydian
refactor: remove unused canvas variable
2a59f79
@merydian
refactor: make _toggle_preview more readable
429c212
@merydian
feat: drag & drop even after double click
8e624a4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@koebi koebi koebi requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
high priority
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Make vertex markers on map drag and droppable
4 participants
@merydian @TheGreatRefrigerator @MichaelsJP @koebi