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

[Feature] Use alt to select vertices by polygon #42250

Merged
merged 4 commits into from
Mar 30, 2021

Conversation

uclaros
Copy link
Contributor

@uclaros uclaros commented Mar 15, 2021

Description

Sometimes selecting the vertices you want with a rectangle using the vertex tool is hard and needs multiple passes using shift or ctrl to add or remove vertices from the selection or to creatively rotate the map canvas to align the vertices.

This PR should help in such situations, like for example selecting only one side's vertices of a curved road.

Usage:

  • Alt+click using the vertex tool to start digitizing a polygon.
  • Each subsequent click adds a new vertex to the rubberband polygon.
  • Backspace/delete removes last added rubberband vertex.
  • Esc cancels the polygon selection mode, as also does backspacing/deleting all of the rubberband's vertices.
  • Right click finalizes the polygon digitizing and selects all vertices within the rubberband polygon.

Peek 2021-03-15 19-08

@roya0045
Copy link
Contributor

That's nice, though I feel like I always forget contextual shortcuts, would it be possible to have a reminder popup when using tool to display their specific shortcuts. Do you know if you can come up with something for this?

@uclaros
Copy link
Contributor Author

uclaros commented Mar 15, 2021

That's nice, though I feel like I always forget contextual shortcuts, would it be possible to have a reminder popup when using tool to display their specific shortcuts. Do you know if you can come up with something for this?

It could be advertised on the tool's tooltip, but it would grow ugly big.
Another alternative would be to advertise it (and the cryptic shift-R mode) on the empty Vertex Editor Panel

@roya0045
Copy link
Contributor

Could be an option.

@github-actions github-actions bot added this to the 3.20.0 milestone Mar 15, 2021
@nyalldawson
Copy link
Collaborator

Great idea!

@nyalldawson
Copy link
Collaborator

It could be advertised on the tool's tooltip, but it would grow ugly big.

I don't think that's so much of a problem -- check the "Show/Hide labels" and "Pin labels" tooltips -- they are very explicit with describing the different behavior of the tools and yet work nicely in practice.

@nyalldawson nyalldawson reopened this Mar 15, 2021
@uclaros
Copy link
Contributor Author

uclaros commented Mar 18, 2021

Here's a take on the tooltip:
image

It doesn't look too bad after all indeed!
I'm not sure why the (All Layers) - (Current Layer) text is missing from the tooltip's title though...

Any suggestion as native speakers?

@nyalldawson
Copy link
Collaborator

@uclaros
I like the toolip, thanks!

@nyalldawson
Copy link
Collaborator

I'm not sure why the (All Layers) - (Current Layer) text is missing from the tooltip's title though...

From memory there's a regex in place which looks for ( .* ) and treats the contents inside the brackets as the shortcut key, and formats the tooltip accordingly.

If you change the text to just "Vertex Tool - All Layers" and remove the parenthesis it should work. Bonus points if you use the typographically correct dash symbol 😉

@roya0045
Copy link
Contributor

@uclaros Tooltip is great! Not sure about the black mode as opposed to the conventional black on notepad-yellow but its a minor nitpick.

@nyalldawson
Copy link
Collaborator

@roya0045 that's a operating system level theming thing, not anything QGIS is doing

@roya0045
Copy link
Contributor

@nyalldawson Ah good, wasn't sure if it was imposed or not.

@uclaros
Copy link
Contributor Author

uclaros commented Mar 20, 2021

If you change the text to just "Vertex Tool - All Layers" and remove the parenthesis it should work. Bonus points if you use the typographically correct dash symbol

I'm more of an ascii guy!

Not sure about the black mode as opposed to the conventional black on notepad-yellow but its a minor nitpick.

Yup, it's a kde breeze theme thing!

Copy link
Collaborator

@nyalldawson nyalldawson left a comment

Choose a reason for hiding this comment

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

Code looks good to me -- could we get a unit test too please?

@uclaros
Copy link
Contributor Author

uclaros commented Mar 29, 2021

Test added, sorry for the force push, I rebased to ease on my build time.

@nyalldawson nyalldawson merged commit 6bc879f into qgis:master Mar 30, 2021
@uclaros
Copy link
Contributor Author

uclaros commented Apr 2, 2021

@nyalldawson maybe also add the changelog and documentation tags?

@DelazJ DelazJ added Changelog Items that are queued to appear in the visual changelog - remove after harvesting Needs Documentation When merging a labeled PR, an issue will be created in the Doc repo. labels Jun 7, 2021
@github-actions
Copy link

github-actions bot commented Jun 7, 2021

@uclaros
This pull request has been tagged as requiring documentation.

A documentation ticket will be opened at https://github.com/qgis/QGIS-Documentation when this PR is merged.

Please update the description (not the comments) with helpful description and screenshot to help the work from documentors.
Also, any commit having [needs-doc] or [Needs Documentation] in will see its message pushed to the issue, so please be as verbose as you can.

Thank you!

@github-actions
Copy link

github-actions bot commented Jun 7, 2021

@uclaros
A documentation ticket has been opened at qgis/QGIS-Documentation#6742
It is your responsibility to visit this ticket and add as much detail as possible for the documentation team to correctly document this change.
Thank you!

@zacharlie zacharlie added ChangelogHarvested This PR description has been harvested in the Changelog already. and removed Changelog Items that are queued to appear in the visual changelog - remove after harvesting labels Jun 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ChangelogHarvested This PR description has been harvested in the Changelog already. Needs Documentation When merging a labeled PR, an issue will be created in the Doc repo.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants