Don't call virtual method from c'tor

This patch fixes the following issues:

1. The c'tor of NonInternalLibraryDetailsController
called slotLibraryTypeChanged(), which called in turn
virtual updateWindowsOptionsEnablement(). This method
is reimplemented in subclasses, so we can't easily
make this method final.

The solution is to reorganize the code so that we don't
call anymore updateWindowsOptionsEnablement() from the
NonInternalLibraryDetailsController c'tor, but instead
we rely on the fact that it will be called indirectly through
updateGui, called for each leaf subclass from its c'tor.

2. Don't call multiple times updateGui when constructing
the subclass of NonInternalLibraryDetailsController or
when changing library type (from combo box). Before, each
time that happened, the updateGui was called three times.

3. Provide an ugly hack to nicely resize the wizard when
showing more widgets (if needed). This also eliminates flickering
between transitioning from Type to Details page.
It makes the layout much better, as it respects the reasonable
minimum size (when resizing to smaller size by the user).
The request to unhack addressed here: QTBUG-88666.

4. Mark all virtual methods with final for all
subclasses of LibraryDetailsController, where appropriate.

Change-Id: Ief7e5e9e1efdbfca5a06eaeeb1528018c29c39ca
Reviewed-by: Christian Kandeler <christian.kandeler@qt.io>
This commit is contained in:
Jarek Kobus
2020-11-19 17:52:20 +01:00
parent c5c3214cfb
commit 3b93ade234
2 changed files with 75 additions and 25 deletions

View File

@@ -60,12 +60,15 @@ LibraryDetailsController::LibraryDetailsController(
m_proFile(proFile), m_proFile(proFile),
m_libraryDetailsWidget(libraryDetails) m_libraryDetailsWidget(libraryDetails)
{ {
fillLibraryPlatformTypes(m_libraryDetailsWidget->libraryTypeComboBox);
setPlatformsVisible(true); setPlatformsVisible(true);
setLinkageGroupVisible(true); setLinkageGroupVisible(true);
setMacLibraryGroupVisible(true); setMacLibraryGroupVisible(true);
setPackageLineEditVisible(false); setPackageLineEditVisible(false);
setMacLibraryRadiosVisible(!Utils::HostOsInfo::isMacHost()); const bool isMacOs = libraryPlatformType() == Utils::OsTypeMac;
setLinkageRadiosVisible(Utils::HostOsInfo::isWindowsHost()); const bool isWindows = libraryPlatformType() == Utils::OsTypeWindows;
setMacLibraryRadiosVisible(!isMacOs);
setLinkageRadiosVisible(isWindows);
connect(m_libraryDetailsWidget->includePathChooser, &Utils::PathChooser::rawPathChanged, connect(m_libraryDetailsWidget->includePathChooser, &Utils::PathChooser::rawPathChanged,
this, &LibraryDetailsController::slotIncludePathChanged); this, &LibraryDetailsController::slotIncludePathChanged);
@@ -83,8 +86,6 @@ LibraryDetailsController::LibraryDetailsController(
this, &LibraryDetailsController::slotPlatformChanged); this, &LibraryDetailsController::slotPlatformChanged);
connect(m_libraryDetailsWidget->winCheckBox, &QAbstractButton::clicked, connect(m_libraryDetailsWidget->winCheckBox, &QAbstractButton::clicked,
this, &LibraryDetailsController::slotPlatformChanged); this, &LibraryDetailsController::slotPlatformChanged);
fillLibraryPlatformTypes(m_libraryDetailsWidget->libraryTypeComboBox);
} }
Ui::LibraryDetailsWidget *LibraryDetailsController::libraryDetailsWidget() const Ui::LibraryDetailsWidget *LibraryDetailsController::libraryDetailsWidget() const
@@ -170,6 +171,31 @@ void LibraryDetailsController::updateGui()
libraryDetailsWidget()->includePathChooser->setPath(suggestedIncludePath()); libraryDetailsWidget()->includePathChooser->setPath(suggestedIncludePath());
setIgnoreGuiSignals(false); setIgnoreGuiSignals(false);
// UGLY HACK BEGIN
//
// We need to invoke QWizardPrivate::updateLayout() method to properly
// recalculate the new minimum size for the whole wizard.
// This is done internally by QWizard e.g. when a new wizard page is being shown.
// Unfortunately, QWizard doesn't expose this method currently.
// Since the current implementation of QWizard::setTitleFormat() sets the
// format and calls QWizardPrivate::updateLayout() unconditionally
// we use it as a hacky solution to the above issue.
// For reference please see: QTBUG-88666
if (!m_wizard) {
QWidget *widget = libraryDetailsWidget()->detailsLayout->parentWidget();
while (widget) {
QWizard *wizard = qobject_cast<QWizard *>(widget);
if (wizard) {
m_wizard = wizard;
break;
}
widget = widget->parentWidget();
}
}
QTC_ASSERT(m_wizard, return);
m_wizard->setTitleFormat(m_wizard->titleFormat());
// UGLY HACK END
} }
QString LibraryDetailsController::proFile() const QString LibraryDetailsController::proFile() const
@@ -614,7 +640,7 @@ NonInternalLibraryDetailsController::NonInternalLibraryDetailsController(
this, &NonInternalLibraryDetailsController::slotLinkageTypeChanged); this, &NonInternalLibraryDetailsController::slotLinkageTypeChanged);
connect(libraryDetailsWidget()->libraryTypeComboBox, &QComboBox::currentTextChanged, connect(libraryDetailsWidget()->libraryTypeComboBox, &QComboBox::currentTextChanged,
this, &NonInternalLibraryDetailsController::slotLibraryTypeChanged); this, &NonInternalLibraryDetailsController::slotLibraryTypeChanged);
slotLibraryTypeChanged(); handleLibraryTypeChange();
} }
AddLibraryWizard::LinkageType NonInternalLibraryDetailsController::suggestedLinkageType() const AddLibraryWizard::LinkageType NonInternalLibraryDetailsController::suggestedLinkageType() const
@@ -676,18 +702,22 @@ void NonInternalLibraryDetailsController::updateWindowsOptionsEnablement()
libraryDetailsWidget()->winGroupBox->setEnabled(ena); libraryDetailsWidget()->winGroupBox->setEnabled(ena);
} }
void NonInternalLibraryDetailsController::slotLinkageTypeChanged() void NonInternalLibraryDetailsController::handleLinkageTypeChange()
{ {
if (guiSignalsIgnored())
return;
if (isMacLibraryRadiosVisible() if (isMacLibraryRadiosVisible()
&& libraryDetailsWidget()->staticRadio->isChecked()) { && libraryDetailsWidget()->staticRadio->isChecked()) {
setIgnoreGuiSignals(true); setIgnoreGuiSignals(true);
libraryDetailsWidget()->libraryRadio->setChecked(true); libraryDetailsWidget()->libraryRadio->setChecked(true);
setIgnoreGuiSignals(false); setIgnoreGuiSignals(false);
} }
}
void NonInternalLibraryDetailsController::slotLinkageTypeChanged()
{
if (guiSignalsIgnored())
return;
handleLinkageTypeChange();
updateGui(); updateGui();
} }
@@ -699,7 +729,7 @@ void NonInternalLibraryDetailsController::slotRemoveSuffixChanged(bool ena)
} }
} }
void NonInternalLibraryDetailsController::slotLibraryTypeChanged() void NonInternalLibraryDetailsController::handleLibraryTypeChange()
{ {
libraryDetailsWidget()->libraryPathChooser->setPromptDialogFilter(libraryPlatformFilter()); libraryDetailsWidget()->libraryPathChooser->setPromptDialogFilter(libraryPlatformFilter());
const bool isMacOs = libraryPlatformType() == Utils::OsTypeMac; const bool isMacOs = libraryPlatformType() == Utils::OsTypeMac;
@@ -709,14 +739,18 @@ void NonInternalLibraryDetailsController::slotLibraryTypeChanged()
setMacLibraryRadiosVisible(!isMacOs); setMacLibraryRadiosVisible(!isMacOs);
setLinkageRadiosVisible(isWindows); setLinkageRadiosVisible(isWindows);
setRemoveSuffixVisible(isWindows); setRemoveSuffixVisible(isWindows);
handleLibraryPathChange();
updateWindowsOptionsEnablement(); handleLinkageTypeChange();
slotLibraryPathChanged();
slotLinkageTypeChanged();
libraryDetailsWidget()->detailsLayout->parentWidget()->window()->adjustSize();
} }
void NonInternalLibraryDetailsController::slotLibraryPathChanged() void NonInternalLibraryDetailsController::slotLibraryTypeChanged()
{
handleLibraryTypeChange();
updateGui();
emit completeChanged();
}
void NonInternalLibraryDetailsController::handleLibraryPathChange()
{ {
if (libraryPlatformType() == Utils::OsTypeWindows) { if (libraryPlatformType() == Utils::OsTypeWindows) {
bool subfoldersEnabled = true; bool subfoldersEnabled = true;
@@ -741,9 +775,12 @@ void NonInternalLibraryDetailsController::slotLibraryPathChanged()
libraryDetailsWidget()->addSuffixCheckBox->setChecked(true); libraryDetailsWidget()->addSuffixCheckBox->setChecked(true);
} }
} }
}
void NonInternalLibraryDetailsController::slotLibraryPathChanged()
{
handleLibraryPathChange();
updateGui(); updateGui();
emit completeChanged(); emit completeChanged();
} }

View File

@@ -110,6 +110,7 @@ private:
bool m_windowsGroupVisible = true; bool m_windowsGroupVisible = true;
Ui::LibraryDetailsWidget *m_libraryDetailsWidget; Ui::LibraryDetailsWidget *m_libraryDetailsWidget;
QWizard *m_wizard = nullptr;
}; };
class NonInternalLibraryDetailsController : public LibraryDetailsController class NonInternalLibraryDetailsController : public LibraryDetailsController
@@ -122,11 +123,15 @@ public:
bool isComplete() const override; bool isComplete() const override;
QString snippet() const override; QString snippet() const override;
protected: protected:
AddLibraryWizard::LinkageType suggestedLinkageType() const override; AddLibraryWizard::LinkageType suggestedLinkageType() const override final;
AddLibraryWizard::MacLibraryType suggestedMacLibraryType() const override; AddLibraryWizard::MacLibraryType suggestedMacLibraryType() const override final;
QString suggestedIncludePath() const override; QString suggestedIncludePath() const override final;
void updateWindowsOptionsEnablement() override; void updateWindowsOptionsEnablement() override;
private: private:
void handleLinkageTypeChange();
void handleLibraryTypeChange();
void handleLibraryPathChange();
void slotLinkageTypeChanged(); void slotLinkageTypeChanged();
void slotRemoveSuffixChanged(bool ena); void slotRemoveSuffixChanged(bool ena);
void slotLibraryTypeChanged(); void slotLibraryTypeChanged();
@@ -142,6 +147,10 @@ public:
QObject *parent = nullptr); QObject *parent = nullptr);
bool isComplete() const override; bool isComplete() const override;
QString snippet() const override; QString snippet() const override;
protected:
void updateWindowsOptionsEnablement() override final {
NonInternalLibraryDetailsController::updateWindowsOptionsEnablement();
}
private: private:
bool isLinkPackageGenerated() const; bool isLinkPackageGenerated() const;
}; };
@@ -153,6 +162,10 @@ public:
explicit SystemLibraryDetailsController(Ui::LibraryDetailsWidget *libraryDetails, explicit SystemLibraryDetailsController(Ui::LibraryDetailsWidget *libraryDetails,
const QString &proFile, const QString &proFile,
QObject *parent = nullptr); QObject *parent = nullptr);
protected:
void updateWindowsOptionsEnablement() override final {
NonInternalLibraryDetailsController::updateWindowsOptionsEnablement();
}
}; };
class ExternalLibraryDetailsController : public NonInternalLibraryDetailsController class ExternalLibraryDetailsController : public NonInternalLibraryDetailsController
@@ -163,7 +176,7 @@ public:
const QString &proFile, const QString &proFile,
QObject *parent = nullptr); QObject *parent = nullptr);
protected: protected:
void updateWindowsOptionsEnablement() override; void updateWindowsOptionsEnablement() override final;
}; };
class InternalLibraryDetailsController : public LibraryDetailsController class InternalLibraryDetailsController : public LibraryDetailsController
@@ -176,10 +189,10 @@ public:
bool isComplete() const override; bool isComplete() const override;
QString snippet() const override; QString snippet() const override;
protected: protected:
AddLibraryWizard::LinkageType suggestedLinkageType() const override; AddLibraryWizard::LinkageType suggestedLinkageType() const override final;
AddLibraryWizard::MacLibraryType suggestedMacLibraryType() const override; AddLibraryWizard::MacLibraryType suggestedMacLibraryType() const override final;
QString suggestedIncludePath() const override; QString suggestedIncludePath() const override final;
void updateWindowsOptionsEnablement() override; void updateWindowsOptionsEnablement() override final;
private: private:
void slotCurrentLibraryChanged(); void slotCurrentLibraryChanged();
void updateProFile(); void updateProFile();