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

Bugfix - Layer not selected if clicked in objects dock view #1931

Merged
merged 2 commits into from Apr 10, 2018

Conversation

Projects
None yet
2 participants
@kralle333
Contributor

kralle333 commented Apr 9, 2018

No description provided.

@@ -342,7 +342,7 @@ void ObjectsView::mousePressEvent(QMouseEvent *event)
DocumentManager::instance()->centerMapViewOn(center + offset);
}
} else if (Layer *layer = mapObjectModel()->toLayer(index)) {
mMapDocument->setCurrentObject(layer);
mMapDocument->setCurrentLayer(layer);

This comment has been minimized.

@bjorn

bjorn Apr 10, 2018

Owner

Please keep the call to setCurrentObject, since changing the current layer will only automatically change the current object when either there's isn't a current object or it is a layer. But when the user clicks a layer he may expect to see its properties, for which we need to change the current object.

In any case I agree it would be expected that the current layer changes as well. I do not remember why this code was not doing that in the first place. Maybe there was no good reason, or maybe we'll find out later. :-)

@bjorn bjorn merged commit c71178f into bjorn:master Apr 10, 2018

0 of 2 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
@bjorn

This comment has been minimized.

Owner

bjorn commented Apr 10, 2018

Thanks!

Did you check how this works together with multiple selected layers?

@kralle333

This comment has been minimized.

Contributor

kralle333 commented Apr 10, 2018

Haven't seen issues when selecting multiple layers.

@bjorn

This comment has been minimized.

Owner

bjorn commented Apr 17, 2018

Haven't seen issues when selecting multiple layers.

Alright, indeed there isn't really a problem, it just changes the selection to that one clicked object layer.

It is however possible to select multiple object layers also in the Objects view, but this currently does not lead to multiple selected layers. I'm not sure if we want to go that far though, since layer selection in general is currently not reflected in the Objects view, and when doing so we would mix object and layer selection in a single view. It actually makes little sense to deselect all objects when clicking on a layer, and nor would it make sense to deselect the current layers when clicking on an object. But that is exactly what the Qt selection behavior would do if we stick it into a single view. It may of course be feasible to customize this behavior, but I think that's only worth looking into when we go as far as to unify the Objects and Layers views into a single view (though that raises additional questions).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment