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

Fix saving of the collapsed state for relations #3704

Merged
merged 1 commit into from
Nov 2, 2016

Conversation

pvalsecc
Copy link
Contributor

@pvalsecc pvalsecc commented Nov 1, 2016

In the attribute form, the collapsed state of RelationReference
was not loaded correctly and was interfering with the collapsed
state of the relation editor.

Plus, the state was saved globally for a reference. Meaning that
if a reference was used (through other references) from other
layers, it was sharing the same state.

@m-kuhn
Copy link
Member

m-kuhn commented Nov 1, 2016

Looks good to me

@@ -547,6 +549,9 @@ void QgsCollapsibleGroupBox::showEvent( QShowEvent * event )

QString QgsCollapsibleGroupBox::saveKey() const
{
if ( objectName().isEmpty() || ( mSettingGroup.isEmpty() && window()->objectName().isEmpty() ) )
return QString::null; // cannot get a valid key
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this have to be a null string or would QString() be ok? Qt docs recommend against using QString::isNull.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't care. Switched to empty string to make you happy ;-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

By the way, @nyalldawson, for your information, QString() builds a null string.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That's why the qt docs recommend not using it - it's confusing. QString() builds an empty but non null string, but a null QString is an empty string....!!

Copy link
Member

Choose a reason for hiding this comment

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

https://github.com/qt/qtbase/blob/dev/tests/auto/corelib/tools/qstring/tst_qstring.cpp#L1118-L1123

QString nullStr;
QVERIFY( nullStr.isNull() );
QVERIFY( nullStr.isEmpty() );
QString empty("");
QVERIFY( !empty.isNull() );
QVERIFY( empty.isEmpty() );

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oops... Ignore me, just getting confused! Anyway, qt docs still say QString::null is evil ;)

In the attribute form, the collapsed state of RelationReference
was not loaded correctly and was interfering with the collapsed
state of the relation editor.

Plus, the state was saved globally for a reference. Meaning that
if a reference was used (through other references) from other
layers, it was sharing the same state.
@m-kuhn
Copy link
Member

m-kuhn commented Nov 2, 2016

@nyalldawson I think this should be ready to merge?

@nyalldawson nyalldawson merged commit 4f5337f into qgis:master Nov 2, 2016
@pvalsecc pvalsecc deleted the fix_collapsing branch November 3, 2016 09:05
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.

3 participants