Skip to content

Commit 08c7d66

Browse files
signedavsignedav
authoredMar 3, 2023
Merge pull request #52045 from signedav/yellow_index_lost
Fix lost index on selection change in Attribute Table
·
ltr-3_40final-3_30_0
2 parents 0c8dbb1 + c540c50 commit 08c7d66

File tree

3 files changed

+123
-34
lines changed

3 files changed

+123
-34
lines changed
 

‎src/gui/attributetable/qgsfeaturelistview.cpp

Lines changed: 53 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,22 @@ QgsFeatureListView::QgsFeatureListView( QWidget *parent )
3434
: QListView( parent )
3535
{
3636
setSelectionMode( QAbstractItemView::ExtendedSelection );
37+
38+
mUpdateEditSelectionTimerWithSelection.setSingleShot( true );
39+
connect( &mUpdateEditSelectionTimerWithSelection, &QTimer::timeout, this, [ this ]()
40+
{
41+
updateEditSelection( true );
42+
} );
43+
44+
mUpdateEditSelectionTimerWithSelection.setInterval( 0 );
45+
46+
mUpdateEditSelectionTimerWithoutSelection.setSingleShot( true );
47+
connect( &mUpdateEditSelectionTimerWithoutSelection, &QTimer::timeout, this, [ this ]()
48+
{
49+
updateEditSelection( false );
50+
} );
51+
52+
mUpdateEditSelectionTimerWithoutSelection.setInterval( 0 );
3753
}
3854

3955
QgsVectorLayerCache *QgsFeatureListView::layerCache()
@@ -406,6 +422,19 @@ void QgsFeatureListView::selectRow( const QModelIndex &index, bool anchor )
406422
}
407423

408424
void QgsFeatureListView::ensureEditSelection( bool inSelection )
425+
{
426+
427+
if ( inSelection )
428+
{
429+
mUpdateEditSelectionTimerWithSelection.start();
430+
}
431+
else
432+
{
433+
mUpdateEditSelectionTimerWithoutSelection.start();
434+
}
435+
}
436+
437+
void QgsFeatureListView::updateEditSelection( bool inSelection )
409438
{
410439
if ( !mModel->rowCount() )
411440
{
@@ -465,46 +494,37 @@ void QgsFeatureListView::ensureEditSelection( bool inSelection )
465494

466495
if ( editSelectionUpdateRequested )
467496
{
468-
if ( !mUpdateEditSelectionTimer.isSingleShot() )
469-
{
470-
mUpdateEditSelectionTimer.setSingleShot( true );
471-
connect( &mUpdateEditSelectionTimer, &QTimer::timeout, this, [ this, inSelection, validEditSelectionAvailable ]()
472-
{
473-
// The layer might have been removed between timer start and timer triggered
474-
// in this case there is nothing left for us to do.
475-
if ( !layerCache() )
476-
return;
497+
// The layer might have been removed between timer start and timer triggered
498+
// in this case there is nothing left for us to do.
499+
if ( !layerCache() )
500+
return;
477501

478-
int rowToSelect = -1;
502+
int rowToSelect = -1;
479503

480-
if ( inSelection )
504+
if ( inSelection )
505+
{
506+
const QgsFeatureIds selectedFids = layerCache()->layer()->selectedFeatureIds();
507+
const int rowCount = mModel->rowCount();
508+
509+
for ( int i = 0; i < rowCount; i++ )
510+
{
511+
if ( selectedFids.contains( mModel->idxToFid( mModel->index( i, 0 ) ) ) )
481512
{
482-
const QgsFeatureIds selectedFids = layerCache()->layer()->selectedFeatureIds();
483-
const int rowCount = mModel->rowCount();
484-
485-
for ( int i = 0; i < rowCount; i++ )
486-
{
487-
if ( selectedFids.contains( mModel->idxToFid( mModel->index( i, 0 ) ) ) )
488-
{
489-
rowToSelect = i;
490-
break;
491-
}
492-
493-
if ( rowToSelect == -1 && !validEditSelectionAvailable )
494-
rowToSelect = 0;
495-
}
513+
rowToSelect = i;
514+
break;
496515
}
497-
else
516+
517+
if ( rowToSelect == -1 && !validEditSelectionAvailable )
498518
rowToSelect = 0;
519+
}
520+
}
521+
else
522+
rowToSelect = 0;
499523

500-
if ( rowToSelect != -1 )
501-
{
502-
setEditSelection( mModel->mapToMaster( mModel->index( rowToSelect, 0 ) ), QItemSelectionModel::ClearAndSelect );
503-
}
504-
} );
505-
mUpdateEditSelectionTimer.setInterval( 0 );
524+
if ( rowToSelect != -1 )
525+
{
526+
setEditSelection( mModel->mapToMaster( mModel->index( rowToSelect, 0 ) ), QItemSelectionModel::ClearAndSelect );
506527
}
507-
mUpdateEditSelectionTimer.start();
508528
}
509529
}
510530

‎src/gui/attributetable/qgsfeaturelistview.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,8 @@ class GUI_EXPORT QgsFeatureListView : public QListView
234234
private:
235235
void selectRow( const QModelIndex &index, bool anchor );
236236

237+
void updateEditSelection( bool inSelection = false );
238+
237239
enum PositionInList
238240
{
239241
First,
@@ -264,7 +266,8 @@ class GUI_EXPORT QgsFeatureListView : public QListView
264266
int mRowAnchor = 0;
265267
QItemSelectionModel::SelectionFlags mCtrlDragSelectionFlag;
266268

267-
QTimer mUpdateEditSelectionTimer;
269+
QTimer mUpdateEditSelectionTimerWithSelection;
270+
QTimer mUpdateEditSelectionTimerWithoutSelection;
268271

269272
friend class QgsDualView;
270273
};

‎tests/src/app/testqgsattributetable.cpp

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@
3232
#include "qgsgui.h"
3333
#include "qgseditorwidgetregistry.h"
3434

35+
#include <QSignalSpy>
36+
3537
/**
3638
* \ingroup UnitTests
3739
* This is a unit test for the attribute table dialog
@@ -66,6 +68,7 @@ class TestQgsAttributeTable : public QObject
6668
void testStartMultiEditNoChanges();
6769
void testMultiEditMakeUncommittedChanges();
6870
void testInvalidView();
71+
void testEnsureEditSelection();
6972

7073
private:
7174
QgisApp *mQgisApp = nullptr;
@@ -830,5 +833,68 @@ void TestQgsAttributeTable::testInvalidView()
830833
QCOMPARE( dlg->mMainView->filteredFeatures(), QgsFeatureIds() << 1 << 3 );
831834
}
832835

836+
void TestQgsAttributeTable::testEnsureEditSelection()
837+
{
838+
std::unique_ptr< QgsVectorLayer > layer = std::make_unique< QgsVectorLayer >( QStringLiteral( "Point?field=col0:integer&field=col1:integer" ), QStringLiteral( "test" ), QStringLiteral( "memory" ) );
839+
QVERIFY( layer->isValid() );
840+
841+
QgsFeature ft1( layer->dataProvider()->fields(), 1 );
842+
ft1.setAttributes( QgsAttributes() << 1 << 2 );
843+
layer->dataProvider()->addFeature( ft1 );
844+
QgsFeature ft2( layer->dataProvider()->fields(), 2 );
845+
ft2.setAttributes( QgsAttributes() << 3 << 4 );
846+
layer->dataProvider()->addFeature( ft2 );
847+
QgsFeature ft3( layer->dataProvider()->fields(), 3 );
848+
ft3.setAttributes( QgsAttributes() << 5 << 6 );
849+
layer->dataProvider()->addFeature( ft3 );
850+
QgsFeature ft4( layer->dataProvider()->fields(), 4 );
851+
ft4.setAttributes( QgsAttributes() << 7 << 8 );
852+
layer->dataProvider()->addFeature( ft4 );
853+
854+
layer->removeSelection();
855+
856+
std::unique_ptr< QgsAttributeTableDialog > dlg( new QgsAttributeTableDialog( layer.get() ) );
857+
858+
//since the update is done by timer, we have to wait (at least one millisecond) or until the current edit selection changed
859+
qRegisterMetaType<QgsFeature>( "QgsFeature&" );
860+
QSignalSpy spy( dlg->mMainView->mFeatureListView, &QgsFeatureListView::currentEditSelectionChanged );
861+
862+
// we set the index to ft3
863+
dlg->mMainView->setCurrentEditSelection( {ft3.id()} );
864+
// ... and the currentEditSelection is on ft3
865+
QVERIFY( dlg->mMainView->mFeatureListView->currentEditSelection().contains( 3 ) );
866+
867+
// we make a featureselection on ft1, ft2 and ft3
868+
layer->selectByIds( QgsFeatureIds() << 1 << 2 << 3 );
869+
spy.wait( 1 );
870+
// ... and the currentEditSelection stays on ft3 (since it's in the featureselection)
871+
QVERIFY( dlg->mMainView->mFeatureListView->currentEditSelection().contains( 3 ) );
872+
873+
// we release the featureselection
874+
layer->removeSelection();
875+
spy.wait( 1 );
876+
// ... and the currentEditSelection persists on 3 (since it does not make an update)
877+
QVERIFY( dlg->mMainView->mFeatureListView->currentEditSelection().contains( 3 ) );
878+
879+
// we make afeatureselection on ft4
880+
layer->selectByIds( QgsFeatureIds() << 4 );
881+
spy.wait( 1 );
882+
// ... and the currentEditSelection goes to ft4
883+
QVERIFY( dlg->mMainView->mFeatureListView->currentEditSelection().contains( 4 ) );
884+
885+
// we make afeatureselection on ft2 and ft3
886+
layer->selectByIds( QgsFeatureIds() << 2 << 3 );
887+
spy.wait( 1 );
888+
// ... and the currentEditSelection goes to the first one of the featureselection (means ft2)
889+
QVERIFY( dlg->mMainView->mFeatureListView->currentEditSelection().contains( 2 ) );
890+
891+
// we reload the layer
892+
layer->reload();
893+
spy.wait( 1 );
894+
// ... and the currentEditSelection jumps to the first one (instead of staying at 2, since it's NOT persistend)
895+
QVERIFY( dlg->mMainView->mFeatureListView->currentEditSelection().contains( 1 ) );
896+
897+
}
898+
833899
QGSTEST_MAIN( TestQgsAttributeTable )
834900
#include "testqgsattributetable.moc"

1 commit comments

Comments
 (1)

jef-n commented on Mar 3, 2023

@jef-n
jef-n
on Mar 3, 2023edited by jef-n
Member

Hey, don't merge when the release is ongoing. Esp. when there are quotes in the release name…

Please sign in to comment.