Skip to content
Permalink
Browse files
Don't emit layerOrderChanged when removing layers
Otherwise it automatically enables the layer order panel
  • Loading branch information
nyalldawson committed Mar 22, 2017
1 parent b8fd1fd commit 746d288e33b66cad7a997a82e23a674b90b08b5b
Showing with 1 addition and 5 deletions.
  1. +0 −4 src/core/qgsproject.cpp
  2. +1 −1 tests/src/python/test_qgsproject.py
@@ -2171,7 +2171,6 @@ void QgsProject::removeMapLayers( const QList<QgsMapLayer *> &layers )
QStringList layerIds;
QList<QgsMapLayer *> layerList;

bool layerOrderHasChanged = false;
QList< QgsMapLayer * > currentOrder = layerOrder();
Q_FOREACH ( QgsMapLayer *layer, layers )
{
@@ -2180,7 +2179,6 @@ void QgsProject::removeMapLayers( const QList<QgsMapLayer *> &layers )
{
layerIds << layer->id();
layerList << layer;
layerOrderHasChanged = layerOrderHasChanged || currentOrder.contains( layer );
}
}

@@ -2204,8 +2202,6 @@ void QgsProject::removeMapLayers( const QList<QgsMapLayer *> &layers )
}

emit layersRemoved( layerIds );
if ( layerOrderHasChanged )
emit layerOrderChanged();
}

void QgsProject::removeMapLayer( const QString &layerId )
@@ -210,7 +210,7 @@ def testLayerOrder(self):
self.assertEqual(len(layer_order_changed_spy), 3)
prj.removeMapLayer(layer)
self.assertEqual(prj.layerOrder(), [layer2, layer3])
self.assertEqual(len(layer_order_changed_spy), 4)
self.assertEqual(len(layer_order_changed_spy), 3) # should be no signal

# save and restore
file_name = os.path.join(str(QDir.tempPath()), 'proj.qgs')

3 comments on commit 746d288

@m-kuhn

This comment has been minimized.

Copy link
Member

@m-kuhn m-kuhn replied Mar 22, 2017

Isn't the problem rather that the panel relies on the layer order change signal to be activated?

@nyalldawson

This comment has been minimized.

Copy link
Collaborator Author

@nyalldawson nyalldawson replied Mar 22, 2017

I was 50/50 on emitting that signal when a layer was removed in the first place. I'm not sure that removing a layer really consistutes reordering or not. But when I discovered it caused this bug it pushed me to just remove the signal.

If we did emit it for layer removal I can't see a clean way to detect that the signal was caused by layer removal and not by manual reordering. On the flipside if someone wants both conditions (manual layer reordering AND reordering via removal) they could just connect to both signals. Maybe I just need to make this clear in the docs.

@m-kuhn

This comment has been minimized.

Copy link
Member

@m-kuhn m-kuhn replied Mar 22, 2017

I'm rewriting this code anyway right now. hasCustomLayerOrder is still only in gui.

Please sign in to comment.