QGIS » QGIS Application

QgsCollapsibleGroupBox is by default auto-saving it's folded state.

The feature is nice, but this has the following problems :

- It leads to a strange behaviour when there are several instances of the same QgsCollapsibleGroupBox : while folding/unfolding group boxes, each instance has it own folded state, but upon reload, each instance will have the last expanded state (see screenshort).

This only happens when the group boxes have the same object name and the same parent window name. It is bad form to name two group boxes with the same object name, but I don't think you are implying that's the case. Please provide a functional use case example of this issue (you only show a screen shot of the result) so the class can be fixed, if needed.

An additional class feature that might fix this (assuming I understand the issue) is an accessible property to set the QSettings key string, as an option to having the key built off of the parent window's object name (default). This way, the setting string could be generated base upon the current composer map item, instead of the window name, if say, the group box doesn't change, but its contents do. This assumes the group box is destroyed between map item selections.

- It is actually quite unexpected for a QWidget to auto-store settings. It seems to me that it's not implemented in the right place. All other widgets (checkboxes...) need ad-hoc implementation to save their state.

There is a big difference between auto-saving the collapsed state of this custom widget and the state of an actual form control, e.g. checkbox. Form controls usually directly relate to setting options, and as such should not generally be auto-saved (though many are with named slots); however, the collapsed state of this group box class (not a form control widget) is purely cosmetic and completely independent of any functional application setting.

There is nothing 'wrong' with auto-saving the collapsed state of this custom widget on its destruction. There is no 'expected' behavior for a custom widget. They work as needed. It was designed this way so developers do not have to write get/set code for every use just to save a cosmetic attribute of the widget. Its auto-saving of the collapsed state is a feature IMO.

- It overrides the ability to set the initial folded state from within QtDesigner (which you can do by adding the property as dynamic properties)

Actually, I think the feature was developped to be used in global options. But since those CollapsibleBoxes are very practical and good looking, it would be better to consider them as normal QWidgets, to be used everywhere.

In the provided patch, the saveCollapsedState is set to false by defaut.
Additionnaly, collapsed, saveCollapsedState and saveCheckedStateproperties are declared so they can be editted in QtDesigner.
All existing QgsCollapsibleGroupBox in the UI files are set to collapsed=false, saveCollapsedState=true and saveCheckedStateproperties=false.

So this should lead to no change of behaviour in existing code.

The main drawback is that DEVs will have to remember they can add those properties as "dynamic properties" in QtCreator.

The Q_PROPERTY solution is a decent workaround for setting properties in Designer. So why not leave the initial class property values as is (e.g. mSaveCollapsedState = true;) and have the dynamic property for not auto-saving be the one added? Really no different than setting it in code, and nothing needs changed in the existing .ui files.

Another issue with this approach: if the class ever get refactored, all of the .ui files' dynamic properties would need to be as well. In my test, QtCreator did not locate any properties in .ui files during a refactor search/replace command, or in a general search across the project (default CMake project). Errors would be thrown during compilation, though.

If you want Designer integration, creating a custom widget for Designer is the best approach.

IMO, the class and functionality should stay as is, but any work to make it better and any work on creating a Designer plugin for it would certainly be welcome.

Thanks for you answers.

I like the two classes solution too ! I'd suggest calling the complete one QgsPersistentCollapsibleGroupBox (a bit long, but at least it's very clear of what the class does)

An additional class feature that might fix this (assuming I understand the issue) is an accessible property to set the QSettings key string, as an option to having the key built off of the parent window's object name (default).

Yes that is a good idea too !

Olivier : can you provide a patch without the changes to default state (including mods to ui files) and a test case (with code) for the bug you mention? I'm not sure I understand why this happens.

About the test case, I'm quite new to QGis developpement, so I still have to learn about testing and won't be able to provide a test case soon. But here's a simple file explaining the bug.

I think I should be able to provide a patch with this in the next few days if we agree on this solution.

Solution would be to set the settings key to a unique value, possibly <frame id>/widget/parent.

Actually, you'd also need to store the settings in the project file rather than in the global settings (since two different projects can have objects with same IDs). But for global options, I think the goal was exactly to save the settings as being global to all projects. So it's not that simple.

In fact, this illustrates well my frustration about this class : it has some too specific features that makes it unsuitable to use as a normal widget.
For an inattentive developer, it could even lead to severe bugs : the use of persistence of non-cosmetic nature would induce modifications across all projects ! (and this seems even encouraged by the saveCheckedState property !)

IMO, if we want to have QgsCollapsibleGroupBox used more generally across QGis and plugins, it would be better to provide a simple and robust class. And if this means change a bit of the existing code, it's better to do it now than later. (right now, the class is used only in few places of QGis, and I doubt that many plugin developers already use it).

Besides, it seems to me that every developer (even for small plugins) has to deal with persistence. He has to implement it for every other aspects of his work. Providing auto-persistence for one specific widget will only make the code more obscure by doing two very similar tasks in two very different ways. Take for example QgsProjectProperties and compare how it's QTabWidget's state and its QgsCollapsibleGroupBox's states are stored.

But anyways, the problem is not that important for now... One simply has to remember to disable the feature in case the implementation does not suit his needs.

I join another patch which does only add the Q_PROPERTY(), comments and the properties in the .ui files.

Here is a patch with 2 classes, including Q_PROPERTY stuff. I haven't updated nor tested sip though.

Sorry I'll be away for a few days, won't be able to look at this until next week. So Larry - go ahead as you deem best, thanks.