Template file dialog: kill old widgets on re-entering TemplateOptionsPage
ClosedPublic

Authored by kossebau on Feb 2 2017, 5:36 PM.

Details

Summary

Simple temporary fix for the current logic as triggered when going "Back"
from the options page to the previous and then entering again, which will
result in another call of the TemplateOptionsPage::load() method.
This should see some redesign possibly, but for this at least avoids
old layout & widget instances creating troubles.

Diff Detail

Repository
R33 KDevPlatform
Branch
fixTemplateOptionsPageOnReenter
Lint
No Linters Available
Unit
No Unit Test Coverage
kossebau updated this revision to Diff 10860.Feb 2 2017, 5:36 PM
kossebau retitled this revision from to Template file dialog: kill old widgets on re-entering TemplateOptionsPage.
kossebau updated this object.
kossebau added a reviewer: KDevelop.
Restricted Application added a subscriber: kdevelop-devel. · View Herald TranscriptFeb 2 2017, 5:36 PM
mwolff requested changes to this revision.Feb 2 2017, 10:17 PM
mwolff added a reviewer: mwolff.
mwolff added a subscriber: mwolff.

this is really dirty. can you please change the code to only add stuff to a "known" widget and then delete that widget and its children here?

This revision now requires changes to proceed.Feb 2 2017, 10:17 PM
kossebau updated this revision to Diff 10977.Feb 6 2017, 5:14 PM
kossebau edited edge metadata.

keep explicit list of option groupboxes around for deletion on reload

In D4413#82710, @mwolff wrote:

this is really dirty. can you please change the code to only add stuff to a "known" widget and then delete that widget and its children here?

Hm, would that be less dirty? Any solution is fragile as they all require that any created QWidget does not get the page itself set as parent widget, otherwise it would be missed out in any cleanup.
I initially did a separate list of any created widgets, though explicit and not as proposed here, implicit by adding another dummy QWidget,. But then I realized that all the information is there in the existing data structure and just needs to be queried when needed. Which is what the loop did here.

delete m_containerWidget;
m_containerWidget = new QWidget(this);
layout()->addWidget(m_containerWidget);

would be what you prefer here?
On debugging UI structures I personall dislike any unneeded extra QWidget, Thus proposing the explicit collection of the groupbox widgets then, as compromise. Though I still think that initially proposed loop is good and just looks dirty because it had so many explanations ;)

mwolff accepted this revision.Feb 15 2017, 10:14 PM
mwolff edited edge metadata.

this is much better

This revision is now accepted and ready to land.Feb 15 2017, 10:14 PM
This revision was automatically updated to reflect the committed changes.