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

Add topological editing toggle button (in edit mode) #939

Merged
merged 5 commits into from
Apr 18, 2020
Merged

Add topological editing toggle button (in edit mode) #939

merged 5 commits into from
Apr 18, 2020

Conversation

nirvn
Copy link
Member

@nirvn nirvn commented Apr 7, 2020

When QField has its overall navigate / edit button toggled to edit, a new topographic editing toggle button appears on the canvas:
Peek 2020-04-07 16-40

We could possibly add another button there to toggle snapping (if that's something that wouldn't overwhelm users).

@nirvn
Copy link
Member Author

nirvn commented Apr 7, 2020

Note: for it to actually work, we'll need to update the SDK to incorporate qgis/QGIS#35647

@qfield-fairy
Copy link
Collaborator

Uploaded test apks for

@nirvn
Copy link
Member Author

nirvn commented Apr 8, 2020

I wasn't entirely satisfied with the lack of a visual queue to differentiate buttons (i.e. bugger dashboard button) vs toggle buttons (i.e. topological editing). I've updated things so that when topological editing is toggled on, not only is the button background going from a semi-opaque dark gray to a fully opaque one, but the icon itself goes from white to QField green:
Peek 2020-04-08 11-42

IMHO, that gives users a much clearer visual statement. That also adopts what I did with the GPS button in the non-locked state (i.e. opaque dark gray + 'location' blue icon).

@qfield-fairy
Copy link
Collaborator

Uploaded test apks for

@signedav signedav mentioned this pull request Apr 8, 2020
7 tasks
@qfield-fairy
Copy link
Collaborator

Uploaded test apks for

@m-kuhn m-kuhn added this to the 1.6 milestone Apr 15, 2020
@qfield-fairy
Copy link
Collaborator

Uploaded test apks for

@nirvn
Copy link
Member Author

nirvn commented Apr 16, 2020

I see green everywhere 😉

Button {
id: topologyButton
round: true
visible: stateMachine.state === "digitize" && ( dashBoard.currentLayer.geometryType() === QgsWkbTypes.PolygonGeometry || dashBoard.currentLayer.geometryType() === QgsWkbTypes.LineGeometry )
Copy link
Member

Choose a reason for hiding this comment

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

As far as I understand this, it doesn't concern the selected layer but still is only visible when a layer is selected?
Shouldn't it be visible as soon as there are at least two layers with geometry line or polygon in the project - no matter if they are selected or not?

Copy link
Member Author

Choose a reason for hiding this comment

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

@signedav , Matthias wants to avoid obscuring the canvas as much as possible, hence hiding when possible.

Copy link
Member

Choose a reason for hiding this comment

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

how does it not concern the selected layer?

Copy link
Member

Choose a reason for hiding this comment

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

First - in browsing mode - it's not visible. Then with enable digitizing mode, it's still not visible. Then I select a layer and it becomes visible. I activate it, and because it popped out by selecting the layer, I expect, that I turn it on only for this layer. But it does not concern (only) this layer. It concerns all the polygon and line layers.

If I have e.g. several point layers. And I click on one, the + appears to digitize. I expect that this + concerns only this layer. There is not an option that I can select the layer after pressing + where to add a feature. This is QGIS behavior. And so I think the activation of the topological editing should be QGIS behavior as well. Means, not appearing on selecting a specific layer.

I suggest to make it visible all the time (as soon as there are polygon or line layers) or put it somewhere else (like e.g. in the menu like the measure tool).

Copy link
Member

Choose a reason for hiding this comment

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

I activate it, and because it popped out by selecting the layer, I expect, that I turn it on only for this layer.

I understand this assumption. But I also think it's obvious that when you change the layer and it stays the way it is it will be quite quickly visible to someone that it's not only for this layer.

For me, topological editing is an edge case. Interesting for some, a miracle for many. So it should take away users attention as little as possible.
It also will not do anything when editing a point layer, hence it doesn't make sense to show it there.

If it was for me, I'd even make it opt-in in the configuration of QField or the project to keep it hidden from the large portion of QField users that will never ever use it.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm -1 to adding a setting here, that's just an extra layer of complexity that's not worth it really IMHO.

I think hiding it as much as we can, like it's done ATM, is a good balance.

Arguably, the bonus of having it show up in the narrow circumstances we have atm does provide a collateral benefit: educating users about the presence of this feature.

Copy link
Member

@m-kuhn m-kuhn left a comment

Choose a reason for hiding this comment

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

I hope it's ok for you if I approve @signedav

@nirvn
Copy link
Member Author

nirvn commented Apr 18, 2020

Let's merge and see how we feel about it over the course of the coming days n weeks.

@nirvn nirvn merged commit 0e9225a into master Apr 18, 2020
@delete-merged-branch delete-merged-branch bot deleted the topo2 branch April 18, 2020 07:12
@signedav
Copy link
Member

I think, I can endure it 😉

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.

None yet

5 participants