From 41ac21bdcde04dd62b82c4f1aaa1ad4974b46b90 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 27 Jan 2020 19:35:41 +0100 Subject: [PATCH 01/16] [welcome] Refactor results-details dialog - Factor out the "details" dialog into a separate class with a translation slot. This resolves the crash reported in #1307. --- .../welcome/checker/ResultsListWidget.cpp | 160 +++++++++++------- 1 file changed, 100 insertions(+), 60 deletions(-) diff --git a/src/modules/welcome/checker/ResultsListWidget.cpp b/src/modules/welcome/checker/ResultsListWidget.cpp index b72b91452..73f1c3a36 100644 --- a/src/modules/welcome/checker/ResultsListWidget.cpp +++ b/src/modules/welcome/checker/ResultsListWidget.cpp @@ -24,6 +24,7 @@ #include "Branding.h" #include "Settings.h" #include "utils/CalamaresUtilsGui.h" +#include "utils/Logger.h" #include "utils/Retranslator.h" #include "widgets/FixedAspectRatioLabel.h" @@ -70,7 +71,9 @@ ResultsListWidget::init( const Calamares::RequirementsList& checkEntries ) allChecked = false; if ( entry.mandatory ) + { requirementsSatisfied = false; + } ciw->setAutoFillBackground( true ); QPalette pal( ciw->palette() ); @@ -94,50 +97,45 @@ ResultsListWidget::init( const Calamares::RequirementsList& checkEntries ) if ( !requirementsSatisfied ) { - CALAMARES_RETRANSLATE( - QString message = Calamares::Settings::instance()->isSetupMode() - ? tr( "This computer does not satisfy the minimum " - "requirements for setting up %1.
" - "Setup cannot continue. " - "Details..." ) - : tr( "This computer does not satisfy the minimum " - "requirements for installing %1.
" - "Installation cannot continue. " - "Details..." ); - textLabel->setText( message.arg( *Calamares::Branding::ShortVersionedName ) ); - ) + CALAMARES_RETRANSLATE( QString message = Calamares::Settings::instance()->isSetupMode() + ? tr( "This computer does not satisfy the minimum " + "requirements for setting up %1.
" + "Setup cannot continue. " + "Details..." ) + : tr( "This computer does not satisfy the minimum " + "requirements for installing %1.
" + "Installation cannot continue. " + "Details..." ); + textLabel->setText( message.arg( *Calamares::Branding::ShortVersionedName ) ); ) textLabel->setOpenExternalLinks( false ); - connect( textLabel, &QLabel::linkActivated, - this, [ this, checkEntries ]( const QString& link ) - { + connect( textLabel, &QLabel::linkActivated, this, [this, checkEntries]( const QString& link ) { if ( link == "#details" ) + { showDetailsDialog( checkEntries ); + } } ); } else { - CALAMARES_RETRANSLATE( - QString message = Calamares::Settings::instance()->isSetupMode() - ? tr( "This computer does not satisfy some of the " - "recommended requirements for setting up %1.
" - "Setup can continue, but some features " - "might be disabled." ) - : tr( "This computer does not satisfy some of the " - "recommended requirements for installing %1.
" - "Installation can continue, but some features " - "might be disabled." ); - textLabel->setText( message.arg( *Calamares::Branding::ShortVersionedName ) ); - ) + CALAMARES_RETRANSLATE( QString message = Calamares::Settings::instance()->isSetupMode() + ? tr( "This computer does not satisfy some of the " + "recommended requirements for setting up %1.
" + "Setup can continue, but some features " + "might be disabled." ) + : tr( "This computer does not satisfy some of the " + "recommended requirements for installing %1.
" + "Installation can continue, but some features " + "might be disabled." ); + textLabel->setText( message.arg( *Calamares::Branding::ShortVersionedName ) ); ) } } if ( allChecked && requirementsSatisfied ) { - if ( !Calamares::Branding::instance()-> - imagePath( Calamares::Branding::ProductWelcome ).isEmpty() ) + if ( !Calamares::Branding::instance()->imagePath( Calamares::Branding::ProductWelcome ).isEmpty() ) { - QPixmap theImage = QPixmap( Calamares::Branding::instance()-> - imagePath( Calamares::Branding::ProductWelcome ) ); + QPixmap theImage + = QPixmap( Calamares::Branding::instance()->imagePath( Calamares::Branding::ProductWelcome ) ); if ( !theImage.isNull() ) { QLabel* imageLabel; @@ -159,41 +157,48 @@ ResultsListWidget::init( const Calamares::RequirementsList& checkEntries ) imageLabel->setSizePolicy( QSizePolicy::Expanding, QSizePolicy::Expanding ); } } - CALAMARES_RETRANSLATE( - textLabel->setText( tr( "This program will ask you some questions and " - "set up %2 on your computer." ) - .arg( *Calamares::Branding::ProductName ) ); - textLabel->setAlignment( Qt::AlignCenter ); - ) + CALAMARES_RETRANSLATE( textLabel->setText( tr( "This program will ask you some questions and " + "set up %2 on your computer." ) + .arg( *Calamares::Branding::ProductName ) ); + textLabel->setAlignment( Qt::AlignCenter ); ) } else + { m_mainLayout->addStretch(); + } } +class ResultsListDialog : public QDialog +{ +public: + ResultsListDialog( QWidget* parent, const Calamares::RequirementsList& checkEntries ); + virtual ~ResultsListDialog(); -void -ResultsListWidget::showDetailsDialog( const Calamares::RequirementsList& checkEntries ) +private: + QLabel* m_title; + QList< ResultWidget* > m_resultWidgets; + const Calamares::RequirementsList& m_entries; + + void retranslate(); +}; + +ResultsListDialog::ResultsListDialog( QWidget* parent, const Calamares::RequirementsList& checkEntries ) + : QDialog( parent ) + , m_entries( checkEntries ) { - QDialog* detailsDialog = new QDialog( this ); - QBoxLayout* mainLayout = new QVBoxLayout; - detailsDialog->setLayout( mainLayout ); + auto* mainLayout = new QVBoxLayout; + auto* entriesLayout = new QVBoxLayout; - QLabel* textLabel = new QLabel; - mainLayout->addWidget( textLabel ); - CALAMARES_RETRANSLATE( - textLabel->setText( tr( "For best results, please ensure that this computer:" ) ); - ) - QBoxLayout* entriesLayout = new QVBoxLayout; - CalamaresUtils::unmarginLayout( entriesLayout ); - mainLayout->addLayout( entriesLayout ); + m_title = new QLabel( this ); for ( const auto& entry : checkEntries ) { if ( !entry.hasDetails() ) + { continue; + } ResultWidget* ciw = new ResultWidget( entry.satisfied, entry.mandatory ); - CALAMARES_RETRANSLATE( ciw->setText( entry.enumerationText() ); ) entriesLayout->addWidget( ciw ); ciw->setSizePolicy( QSizePolicy::Expanding, QSizePolicy::Preferred ); @@ -204,18 +209,53 @@ ResultsListWidget::showDetailsDialog( const Calamares::RequirementsList& checkEn bgColor.setHsv( bgHue, 64, bgColor.value() ); pal.setColor( QPalette::Window, bgColor ); ciw->setPalette( pal ); + + m_resultWidgets.append( ciw ); } - QDialogButtonBox* buttonBox = new QDialogButtonBox( QDialogButtonBox::Close, - Qt::Horizontal, - this ); + QDialogButtonBox* buttonBox = new QDialogButtonBox( QDialogButtonBox::Close, Qt::Horizontal, this ); + + mainLayout->addWidget( m_title ); + mainLayout->addLayout( entriesLayout ); mainLayout->addWidget( buttonBox ); - detailsDialog->setModal( true ); - detailsDialog->setWindowTitle( tr( "System requirements" ) ); + setLayout( mainLayout ); + + CALAMARES_RETRANSLATE_SLOT( &ResultsListDialog::retranslate ) - connect( buttonBox, &QDialogButtonBox::clicked, - detailsDialog, &QDialog::close ); - detailsDialog->exec(); - detailsDialog->deleteLater(); + connect( buttonBox, &QDialogButtonBox::clicked, this, &QDialog::close ); + retranslate(); // Do it now to fill in the texts +} + +ResultsListDialog::~ResultsListDialog() +{ + cDebug() << "~ResultsListDialog @" << (void*)this; +} + +void +ResultsListDialog::retranslate() +{ + cDebug() << "ResultsListDialog::retranslate @" << (void*)this; + m_title->setText( tr( "For best results, please ensure that this computer:" ) ); + setWindowTitle( tr( "System requirements" ) ); + + int i = 0; + for ( const auto& entry : m_entries ) + { + if ( !entry.hasDetails() ) + { + continue; + } + m_resultWidgets[ i ]->setText( entry.enumerationText() ); + i++; + } +} + + +void +ResultsListWidget::showDetailsDialog( const Calamares::RequirementsList& checkEntries ) +{ + auto* dialog = new ResultsListDialog( this, checkEntries ); + dialog->exec(); + dialog->deleteLater(); } From ed1b3b576fc58b84f6233f7c8b2b407305ee1bcc Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 28 Jan 2020 12:21:22 +0100 Subject: [PATCH 02/16] [welcome] Refactor ResultsListDialog - drop useless debugging - add documentation - move to its own spot in the file (not mixed in with ResultsListWidget) --- .../welcome/checker/ResultsListWidget.cpp | 173 +++++++++--------- 1 file changed, 91 insertions(+), 82 deletions(-) diff --git a/src/modules/welcome/checker/ResultsListWidget.cpp b/src/modules/welcome/checker/ResultsListWidget.cpp index 73f1c3a36..956ea991b 100644 --- a/src/modules/welcome/checker/ResultsListWidget.cpp +++ b/src/modules/welcome/checker/ResultsListWidget.cpp @@ -34,6 +34,97 @@ #include #include +/** @brief A "details" dialog for the results-list + * + * This displays the same RequirementsList as ResultsListWidget, + * but the *details* part rather than the show description. + * + * This is an internal-to-the-widget class. + */ +class ResultsListDialog : public QDialog +{ +public: + /** @brief Create a dialog for the given @p checkEntries list of requirements. + * + * The list must continue to exist for the lifetime of the dialog, + * or UB happens. + */ + ResultsListDialog( QWidget* parent, const Calamares::RequirementsList& checkEntries ); + virtual ~ResultsListDialog(); + +private: + QLabel* m_title; + QList< ResultWidget* > m_resultWidgets; ///< One widget for each entry with details available + const Calamares::RequirementsList& m_entries; + + void retranslate(); +}; + +ResultsListDialog::ResultsListDialog( QWidget* parent, const Calamares::RequirementsList& checkEntries ) + : QDialog( parent ) + , m_entries( checkEntries ) +{ + auto* mainLayout = new QVBoxLayout; + auto* entriesLayout = new QVBoxLayout; + + m_title = new QLabel( this ); + + for ( const auto& entry : checkEntries ) + { + if ( !entry.hasDetails() ) + { + continue; + } + + ResultWidget* ciw = new ResultWidget( entry.satisfied, entry.mandatory ); + entriesLayout->addWidget( ciw ); + ciw->setSizePolicy( QSizePolicy::Expanding, QSizePolicy::Preferred ); + + ciw->setAutoFillBackground( true ); + QPalette pal( ciw->palette() ); + QColor bgColor = pal.window().color(); + int bgHue = ( entry.satisfied ) ? bgColor.hue() : ( entry.mandatory ) ? 0 : 60; + bgColor.setHsv( bgHue, 64, bgColor.value() ); + pal.setColor( QPalette::Window, bgColor ); + ciw->setPalette( pal ); + + m_resultWidgets.append( ciw ); + } + + QDialogButtonBox* buttonBox = new QDialogButtonBox( QDialogButtonBox::Close, Qt::Horizontal, this ); + + mainLayout->addWidget( m_title ); + mainLayout->addLayout( entriesLayout ); + mainLayout->addWidget( buttonBox ); + + setLayout( mainLayout ); + + CALAMARES_RETRANSLATE_SLOT( &ResultsListDialog::retranslate ) + + connect( buttonBox, &QDialogButtonBox::clicked, this, &QDialog::close ); + retranslate(); // Do it now to fill in the texts +} + +ResultsListDialog::~ResultsListDialog() {} + +void +ResultsListDialog::retranslate() +{ + m_title->setText( tr( "For best results, please ensure that this computer:" ) ); + setWindowTitle( tr( "System requirements" ) ); + + int i = 0; + for ( const auto& entry : m_entries ) + { + if ( !entry.hasDetails() ) + { + continue; + } + m_resultWidgets[ i ]->setText( entry.enumerationText() ); + i++; + } +} + ResultsListWidget::ResultsListWidget( QWidget* parent ) : QWidget( parent ) @@ -168,88 +259,6 @@ ResultsListWidget::init( const Calamares::RequirementsList& checkEntries ) } } -class ResultsListDialog : public QDialog -{ -public: - ResultsListDialog( QWidget* parent, const Calamares::RequirementsList& checkEntries ); - virtual ~ResultsListDialog(); - -private: - QLabel* m_title; - QList< ResultWidget* > m_resultWidgets; - const Calamares::RequirementsList& m_entries; - - void retranslate(); -}; - -ResultsListDialog::ResultsListDialog( QWidget* parent, const Calamares::RequirementsList& checkEntries ) - : QDialog( parent ) - , m_entries( checkEntries ) -{ - auto* mainLayout = new QVBoxLayout; - auto* entriesLayout = new QVBoxLayout; - - m_title = new QLabel( this ); - - for ( const auto& entry : checkEntries ) - { - if ( !entry.hasDetails() ) - { - continue; - } - - ResultWidget* ciw = new ResultWidget( entry.satisfied, entry.mandatory ); - entriesLayout->addWidget( ciw ); - ciw->setSizePolicy( QSizePolicy::Expanding, QSizePolicy::Preferred ); - - ciw->setAutoFillBackground( true ); - QPalette pal( ciw->palette() ); - QColor bgColor = pal.window().color(); - int bgHue = ( entry.satisfied ) ? bgColor.hue() : ( entry.mandatory ) ? 0 : 60; - bgColor.setHsv( bgHue, 64, bgColor.value() ); - pal.setColor( QPalette::Window, bgColor ); - ciw->setPalette( pal ); - - m_resultWidgets.append( ciw ); - } - - QDialogButtonBox* buttonBox = new QDialogButtonBox( QDialogButtonBox::Close, Qt::Horizontal, this ); - - mainLayout->addWidget( m_title ); - mainLayout->addLayout( entriesLayout ); - mainLayout->addWidget( buttonBox ); - - setLayout( mainLayout ); - - CALAMARES_RETRANSLATE_SLOT( &ResultsListDialog::retranslate ) - - connect( buttonBox, &QDialogButtonBox::clicked, this, &QDialog::close ); - retranslate(); // Do it now to fill in the texts -} - -ResultsListDialog::~ResultsListDialog() -{ - cDebug() << "~ResultsListDialog @" << (void*)this; -} - -void -ResultsListDialog::retranslate() -{ - cDebug() << "ResultsListDialog::retranslate @" << (void*)this; - m_title->setText( tr( "For best results, please ensure that this computer:" ) ); - setWindowTitle( tr( "System requirements" ) ); - - int i = 0; - for ( const auto& entry : m_entries ) - { - if ( !entry.hasDetails() ) - { - continue; - } - m_resultWidgets[ i ]->setText( entry.enumerationText() ); - i++; - } -} void From 320dcac946d54157c3747ecbe01d11fac01e72ac Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 28 Jan 2020 12:23:53 +0100 Subject: [PATCH 03/16] [welcome] Drop unneeded member variable --- src/modules/welcome/checker/ResultsListWidget.cpp | 14 +++++++------- src/modules/welcome/checker/ResultsListWidget.h | 3 +-- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/modules/welcome/checker/ResultsListWidget.cpp b/src/modules/welcome/checker/ResultsListWidget.cpp index 956ea991b..4a026ea9f 100644 --- a/src/modules/welcome/checker/ResultsListWidget.cpp +++ b/src/modules/welcome/checker/ResultsListWidget.cpp @@ -35,17 +35,17 @@ #include /** @brief A "details" dialog for the results-list - * + * * This displays the same RequirementsList as ResultsListWidget, * but the *details* part rather than the show description. - * + * * This is an internal-to-the-widget class. */ class ResultsListDialog : public QDialog { public: /** @brief Create a dialog for the given @p checkEntries list of requirements. - * + * * The list must continue to exist for the lifetime of the dialog, * or UB happens. */ @@ -134,13 +134,14 @@ ResultsListWidget::ResultsListWidget( QWidget* parent ) m_mainLayout = new QVBoxLayout; setLayout( m_mainLayout ); + int paddingSize = qBound( 32, CalamaresUtils::defaultFontHeight() * 4, 128 ); + QHBoxLayout* spacerLayout = new QHBoxLayout; m_mainLayout->addLayout( spacerLayout ); - m_paddingSize = qBound( 32, CalamaresUtils::defaultFontHeight() * 4, 128 ); - spacerLayout->addSpacing( m_paddingSize ); + spacerLayout->addSpacing( paddingSize ); m_entriesLayout = new QVBoxLayout; spacerLayout->addLayout( m_entriesLayout ); - spacerLayout->addSpacing( m_paddingSize ); + spacerLayout->addSpacing( paddingSize ); CalamaresUtils::unmarginLayout( spacerLayout ); } @@ -260,7 +261,6 @@ ResultsListWidget::init( const Calamares::RequirementsList& checkEntries ) } - void ResultsListWidget::showDetailsDialog( const Calamares::RequirementsList& checkEntries ) { diff --git a/src/modules/welcome/checker/ResultsListWidget.h b/src/modules/welcome/checker/ResultsListWidget.h index 3be02b0d0..588e6c3bc 100644 --- a/src/modules/welcome/checker/ResultsListWidget.h +++ b/src/modules/welcome/checker/ResultsListWidget.h @@ -38,7 +38,6 @@ private: QBoxLayout* m_mainLayout; QBoxLayout* m_entriesLayout; - int m_paddingSize; }; -#endif // CHECKER_RESULTSLISTWIDGET_H +#endif // CHECKER_RESULTSLISTWIDGET_H From bede280f9143d19af160513080b66f4734f4e06c Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 28 Jan 2020 12:41:36 +0100 Subject: [PATCH 04/16] [welcome] Refactor ResultsListWidget - no point in having init() called immediately after the constructor, if it only makes sense to have one call to init() ever to create the widget. - while here, give it the same kind of structure as the dialog, holding on to a reference to the list. --- .../welcome/checker/CheckerContainer.cpp | 22 ++++++++++--------- .../welcome/checker/ResultsListWidget.cpp | 8 ++----- .../welcome/checker/ResultsListWidget.h | 5 ++--- 3 files changed, 16 insertions(+), 19 deletions(-) diff --git a/src/modules/welcome/checker/CheckerContainer.cpp b/src/modules/welcome/checker/CheckerContainer.cpp index 0524bddb0..414250582 100644 --- a/src/modules/welcome/checker/CheckerContainer.cpp +++ b/src/modules/welcome/checker/CheckerContainer.cpp @@ -40,10 +40,7 @@ CheckerContainer::CheckerContainer( QWidget* parent ) CalamaresUtils::unmarginLayout( mainLayout ); mainLayout->addWidget( m_waitingWidget ); - CALAMARES_RETRANSLATE( - if ( m_waitingWidget ) - m_waitingWidget->setText( tr( "Gathering system information..." ) ); - ) + CALAMARES_RETRANSLATE( if ( m_waitingWidget ) m_waitingWidget->setText( tr( "Gathering system information..." ) ); ) } CheckerContainer::~CheckerContainer() @@ -52,32 +49,37 @@ CheckerContainer::~CheckerContainer() delete m_checkerWidget; } -void CheckerContainer::requirementsComplete( bool ok ) +void +CheckerContainer::requirementsComplete( bool ok ) { layout()->removeWidget( m_waitingWidget ); m_waitingWidget->deleteLater(); m_waitingWidget = nullptr; // Don't delete in destructor - m_checkerWidget = new ResultsListWidget( this ); - m_checkerWidget->init( m_requirements ); + m_checkerWidget = new ResultsListWidget( this, m_requirements ); layout()->addWidget( m_checkerWidget ); m_verdict = ok; } -void CheckerContainer::requirementsChecked(const Calamares::RequirementsList& l) +void +CheckerContainer::requirementsChecked( const Calamares::RequirementsList& l ) { m_requirements.append( l ); } -void CheckerContainer::requirementsProgress(const QString& message) +void +CheckerContainer::requirementsProgress( const QString& message ) { if ( m_waitingWidget ) + { m_waitingWidget->setText( message ); + } } -bool CheckerContainer::verdict() const +bool +CheckerContainer::verdict() const { return m_verdict; } diff --git a/src/modules/welcome/checker/ResultsListWidget.cpp b/src/modules/welcome/checker/ResultsListWidget.cpp index 4a026ea9f..0c3079579 100644 --- a/src/modules/welcome/checker/ResultsListWidget.cpp +++ b/src/modules/welcome/checker/ResultsListWidget.cpp @@ -126,8 +126,9 @@ ResultsListDialog::retranslate() } -ResultsListWidget::ResultsListWidget( QWidget* parent ) +ResultsListWidget::ResultsListWidget( QWidget* parent, const Calamares::RequirementsList& checkEntries ) : QWidget( parent ) + , m_entries( checkEntries ) { setSizePolicy( QSizePolicy::Expanding, QSizePolicy::Expanding ); @@ -143,12 +144,7 @@ ResultsListWidget::ResultsListWidget( QWidget* parent ) spacerLayout->addLayout( m_entriesLayout ); spacerLayout->addSpacing( paddingSize ); CalamaresUtils::unmarginLayout( spacerLayout ); -} - -void -ResultsListWidget::init( const Calamares::RequirementsList& checkEntries ) -{ bool allChecked = true; bool requirementsSatisfied = true; diff --git a/src/modules/welcome/checker/ResultsListWidget.h b/src/modules/welcome/checker/ResultsListWidget.h index 588e6c3bc..ae5da67ea 100644 --- a/src/modules/welcome/checker/ResultsListWidget.h +++ b/src/modules/welcome/checker/ResultsListWidget.h @@ -29,15 +29,14 @@ class ResultsListWidget : public QWidget { Q_OBJECT public: - explicit ResultsListWidget( QWidget* parent = nullptr ); - - void init( const Calamares::RequirementsList& checkEntries ); + explicit ResultsListWidget( QWidget* parent, const Calamares::RequirementsList& checkEntries ); private: void showDetailsDialog( const Calamares::RequirementsList& checkEntries ); QBoxLayout* m_mainLayout; QBoxLayout* m_entriesLayout; + const Calamares::RequirementsList& m_entries; }; #endif // CHECKER_RESULTSLISTWIDGET_H From 5aae736ced6383a8651b2818bcd5a173b9799a3f Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 28 Jan 2020 12:53:19 +0100 Subject: [PATCH 05/16] [welcome] Create ResultWidget in separate method --- .../welcome/checker/ResultsListWidget.cpp | 53 +++++++++++-------- 1 file changed, 32 insertions(+), 21 deletions(-) diff --git a/src/modules/welcome/checker/ResultsListWidget.cpp b/src/modules/welcome/checker/ResultsListWidget.cpp index 0c3079579..e06f4e7a7 100644 --- a/src/modules/welcome/checker/ResultsListWidget.cpp +++ b/src/modules/welcome/checker/ResultsListWidget.cpp @@ -34,6 +34,35 @@ #include #include +static void +createResultWidgets( QLayout* layout, + QList< ResultWidget* >& resultWidgets, + const Calamares::RequirementsList& checkEntries, + std::function< bool( const Calamares::RequirementEntry& ) > predicate ) +{ + for ( const auto& entry : checkEntries ) + { + if ( !predicate( entry ) ) + { + continue; + } + + ResultWidget* ciw = new ResultWidget( entry.satisfied, entry.mandatory ); + layout->addWidget( ciw ); + ciw->setSizePolicy( QSizePolicy::Expanding, QSizePolicy::Preferred ); + + ciw->setAutoFillBackground( true ); + QPalette pal( ciw->palette() ); + QColor bgColor = pal.window().color(); + int bgHue = ( entry.satisfied ) ? bgColor.hue() : ( entry.mandatory ) ? 0 : 60; + bgColor.setHsv( bgHue, 64, bgColor.value() ); + pal.setColor( QPalette::Window, bgColor ); + ciw->setPalette( pal ); + + resultWidgets.append( ciw ); + } +} + /** @brief A "details" dialog for the results-list * * This displays the same RequirementsList as ResultsListWidget, @@ -69,27 +98,9 @@ ResultsListDialog::ResultsListDialog( QWidget* parent, const Calamares::Requirem m_title = new QLabel( this ); - for ( const auto& entry : checkEntries ) - { - if ( !entry.hasDetails() ) - { - continue; - } - - ResultWidget* ciw = new ResultWidget( entry.satisfied, entry.mandatory ); - entriesLayout->addWidget( ciw ); - ciw->setSizePolicy( QSizePolicy::Expanding, QSizePolicy::Preferred ); - - ciw->setAutoFillBackground( true ); - QPalette pal( ciw->palette() ); - QColor bgColor = pal.window().color(); - int bgHue = ( entry.satisfied ) ? bgColor.hue() : ( entry.mandatory ) ? 0 : 60; - bgColor.setHsv( bgHue, 64, bgColor.value() ); - pal.setColor( QPalette::Window, bgColor ); - ciw->setPalette( pal ); - - m_resultWidgets.append( ciw ); - } + createResultWidgets( entriesLayout, m_resultWidgets, checkEntries, []( const Calamares::RequirementEntry& e ) { + return e.hasDetails(); + } ); QDialogButtonBox* buttonBox = new QDialogButtonBox( QDialogButtonBox::Close, Qt::Horizontal, this ); From 5795801be5272b0636b3cc663ee22bc5d131a571 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 28 Jan 2020 13:00:21 +0100 Subject: [PATCH 06/16] [welcome] Remove unneeded member variables. - local to the constructor, only needed to be members because of the weird split between constructor and init() --- .../welcome/checker/ResultsListWidget.cpp | 21 ++++++++++--------- .../welcome/checker/ResultsListWidget.h | 2 -- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/src/modules/welcome/checker/ResultsListWidget.cpp b/src/modules/welcome/checker/ResultsListWidget.cpp index e06f4e7a7..612ba62ce 100644 --- a/src/modules/welcome/checker/ResultsListWidget.cpp +++ b/src/modules/welcome/checker/ResultsListWidget.cpp @@ -143,16 +143,17 @@ ResultsListWidget::ResultsListWidget( QWidget* parent, const Calamares::Requirem { setSizePolicy( QSizePolicy::Expanding, QSizePolicy::Expanding ); - m_mainLayout = new QVBoxLayout; - setLayout( m_mainLayout ); + QBoxLayout* mainLayout = new QVBoxLayout; + QBoxLayout* entriesLayout = new QVBoxLayout; + + setLayout( mainLayout ); int paddingSize = qBound( 32, CalamaresUtils::defaultFontHeight() * 4, 128 ); QHBoxLayout* spacerLayout = new QHBoxLayout; - m_mainLayout->addLayout( spacerLayout ); + mainLayout->addLayout( spacerLayout ); spacerLayout->addSpacing( paddingSize ); - m_entriesLayout = new QVBoxLayout; - spacerLayout->addLayout( m_entriesLayout ); + spacerLayout->addLayout( entriesLayout ); spacerLayout->addSpacing( paddingSize ); CalamaresUtils::unmarginLayout( spacerLayout ); @@ -165,7 +166,7 @@ ResultsListWidget::ResultsListWidget( QWidget* parent, const Calamares::Requirem { ResultWidget* ciw = new ResultWidget( entry.satisfied, entry.mandatory ); CALAMARES_RETRANSLATE( ciw->setText( entry.negatedText() ); ) - m_entriesLayout->addWidget( ciw ); + entriesLayout->addWidget( ciw ); ciw->setSizePolicy( QSizePolicy::Expanding, QSizePolicy::Preferred ); allChecked = false; @@ -187,12 +188,12 @@ ResultsListWidget::ResultsListWidget( QWidget* parent, const Calamares::Requirem QLabel* textLabel = new QLabel; textLabel->setWordWrap( true ); - m_entriesLayout->insertWidget( 0, textLabel ); + entriesLayout->insertWidget( 0, textLabel ); textLabel->setSizePolicy( QSizePolicy::Expanding, QSizePolicy::Preferred ); if ( !allChecked ) { - m_entriesLayout->insertSpacing( 1, CalamaresUtils::defaultFontHeight() / 2 ); + entriesLayout->insertSpacing( 1, CalamaresUtils::defaultFontHeight() / 2 ); if ( !requirementsSatisfied ) { @@ -251,7 +252,7 @@ ResultsListWidget::ResultsListWidget( QWidget* parent, const Calamares::Requirem } imageLabel->setContentsMargins( 4, CalamaresUtils::defaultFontHeight() * 3 / 4, 4, 4 ); - m_mainLayout->addWidget( imageLabel ); + mainLayout->addWidget( imageLabel ); imageLabel->setAlignment( Qt::AlignCenter ); imageLabel->setSizePolicy( QSizePolicy::Expanding, QSizePolicy::Expanding ); } @@ -263,7 +264,7 @@ ResultsListWidget::ResultsListWidget( QWidget* parent, const Calamares::Requirem } else { - m_mainLayout->addStretch(); + mainLayout->addStretch(); } } diff --git a/src/modules/welcome/checker/ResultsListWidget.h b/src/modules/welcome/checker/ResultsListWidget.h index ae5da67ea..8d0dcb155 100644 --- a/src/modules/welcome/checker/ResultsListWidget.h +++ b/src/modules/welcome/checker/ResultsListWidget.h @@ -34,8 +34,6 @@ public: private: void showDetailsDialog( const Calamares::RequirementsList& checkEntries ); - QBoxLayout* m_mainLayout; - QBoxLayout* m_entriesLayout; const Calamares::RequirementsList& m_entries; }; From f5c0e57f177a011715e08e5ca356e224cb5caf78 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 28 Jan 2020 13:02:39 +0100 Subject: [PATCH 07/16] [welcome] Improve variable naming - these two are about whether all the checkEntries are satisfied (in general, and the mandatory ones) so make the names reflect that. --- src/modules/welcome/checker/ResultsListWidget.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/modules/welcome/checker/ResultsListWidget.cpp b/src/modules/welcome/checker/ResultsListWidget.cpp index 612ba62ce..5cb58e6b5 100644 --- a/src/modules/welcome/checker/ResultsListWidget.cpp +++ b/src/modules/welcome/checker/ResultsListWidget.cpp @@ -157,8 +157,8 @@ ResultsListWidget::ResultsListWidget( QWidget* parent, const Calamares::Requirem spacerLayout->addSpacing( paddingSize ); CalamaresUtils::unmarginLayout( spacerLayout ); - bool allChecked = true; - bool requirementsSatisfied = true; + bool requirementsSatisfied = true; // Give a warning + bool mandatorySatisfied = true; // Give an error for ( const auto& entry : checkEntries ) { @@ -169,10 +169,10 @@ ResultsListWidget::ResultsListWidget( QWidget* parent, const Calamares::Requirem entriesLayout->addWidget( ciw ); ciw->setSizePolicy( QSizePolicy::Expanding, QSizePolicy::Preferred ); - allChecked = false; + requirementsSatisfied = false; if ( entry.mandatory ) { - requirementsSatisfied = false; + mandatorySatisfied = false; } ciw->setAutoFillBackground( true ); @@ -191,11 +191,11 @@ ResultsListWidget::ResultsListWidget( QWidget* parent, const Calamares::Requirem entriesLayout->insertWidget( 0, textLabel ); textLabel->setSizePolicy( QSizePolicy::Expanding, QSizePolicy::Preferred ); - if ( !allChecked ) + if ( !requirementsSatisfied ) { entriesLayout->insertSpacing( 1, CalamaresUtils::defaultFontHeight() / 2 ); - if ( !requirementsSatisfied ) + if ( !mandatorySatisfied ) { CALAMARES_RETRANSLATE( QString message = Calamares::Settings::instance()->isSetupMode() ? tr( "This computer does not satisfy the minimum " @@ -230,7 +230,7 @@ ResultsListWidget::ResultsListWidget( QWidget* parent, const Calamares::Requirem } } - if ( allChecked && requirementsSatisfied ) + if ( requirementsSatisfied && mandatorySatisfied ) { if ( !Calamares::Branding::instance()->imagePath( Calamares::Branding::ProductWelcome ).isEmpty() ) { From ecc7719abdd1708774d012f1f615131faddb3364 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 28 Jan 2020 13:07:57 +0100 Subject: [PATCH 08/16] [welcome] Hoist checking for requirements-satisfied - lift it out of the loop that creates the widgets - some lambda-wankery, but the compiler hammers this down to simple loops and you can read the resulting code as none_of [the list] isUnSatisfied none_of [the list] isMandatoryAndUnSatisfied --- src/modules/welcome/checker/ResultsListWidget.cpp | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/modules/welcome/checker/ResultsListWidget.cpp b/src/modules/welcome/checker/ResultsListWidget.cpp index 5cb58e6b5..3567ec2b0 100644 --- a/src/modules/welcome/checker/ResultsListWidget.cpp +++ b/src/modules/welcome/checker/ResultsListWidget.cpp @@ -157,8 +157,12 @@ ResultsListWidget::ResultsListWidget( QWidget* parent, const Calamares::Requirem spacerLayout->addSpacing( paddingSize ); CalamaresUtils::unmarginLayout( spacerLayout ); - bool requirementsSatisfied = true; // Give a warning - bool mandatorySatisfied = true; // Give an error + // Check that all are satisfied (gives warnings if not) and + // all *mandatory* entries are satisfied (gives errors if not). + auto isUnSatisfied = [](const Calamares::RequirementEntry& e){ return !e.satisfied; }; + auto isMandatoryAndUnSatisfied = [](const Calamares::RequirementEntry& e){ return e.mandatory && !e.satisfied; }; + const bool requirementsSatisfied = std::none_of(checkEntries.begin(), checkEntries.end(), isUnSatisfied); + const bool mandatorySatisfied = std::none_of(checkEntries.begin(), checkEntries.end(), isMandatoryAndUnSatisfied); for ( const auto& entry : checkEntries ) { @@ -168,13 +172,6 @@ ResultsListWidget::ResultsListWidget( QWidget* parent, const Calamares::Requirem CALAMARES_RETRANSLATE( ciw->setText( entry.negatedText() ); ) entriesLayout->addWidget( ciw ); ciw->setSizePolicy( QSizePolicy::Expanding, QSizePolicy::Preferred ); - - requirementsSatisfied = false; - if ( entry.mandatory ) - { - mandatorySatisfied = false; - } - ciw->setAutoFillBackground( true ); QPalette pal( ciw->palette() ); QColor bgColor = pal.window().color(); From b476e4b38662a8fc40e18d2471dd8c83e32ad437 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 28 Jan 2020 13:24:01 +0100 Subject: [PATCH 09/16] [welcome] Refactor link-clicking - remove intermediate lambda - rename dialog slot to one handling links in general (which now **only** does the dialog link) --- .../welcome/checker/ResultsListWidget.cpp | 26 +++++++++---------- .../welcome/checker/ResultsListWidget.h | 4 ++- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/modules/welcome/checker/ResultsListWidget.cpp b/src/modules/welcome/checker/ResultsListWidget.cpp index 3567ec2b0..7f68d22ff 100644 --- a/src/modules/welcome/checker/ResultsListWidget.cpp +++ b/src/modules/welcome/checker/ResultsListWidget.cpp @@ -159,10 +159,10 @@ ResultsListWidget::ResultsListWidget( QWidget* parent, const Calamares::Requirem // Check that all are satisfied (gives warnings if not) and // all *mandatory* entries are satisfied (gives errors if not). - auto isUnSatisfied = [](const Calamares::RequirementEntry& e){ return !e.satisfied; }; - auto isMandatoryAndUnSatisfied = [](const Calamares::RequirementEntry& e){ return e.mandatory && !e.satisfied; }; - const bool requirementsSatisfied = std::none_of(checkEntries.begin(), checkEntries.end(), isUnSatisfied); - const bool mandatorySatisfied = std::none_of(checkEntries.begin(), checkEntries.end(), isMandatoryAndUnSatisfied); + auto isUnSatisfied = []( const Calamares::RequirementEntry& e ) { return !e.satisfied; }; + auto isMandatoryAndUnSatisfied = []( const Calamares::RequirementEntry& e ) { return e.mandatory && !e.satisfied; }; + const bool requirementsSatisfied = std::none_of( checkEntries.begin(), checkEntries.end(), isUnSatisfied ); + const bool mandatorySatisfied = std::none_of( checkEntries.begin(), checkEntries.end(), isMandatoryAndUnSatisfied ); for ( const auto& entry : checkEntries ) { @@ -205,12 +205,7 @@ ResultsListWidget::ResultsListWidget( QWidget* parent, const Calamares::Requirem "Details..." ); textLabel->setText( message.arg( *Calamares::Branding::ShortVersionedName ) ); ) textLabel->setOpenExternalLinks( false ); - connect( textLabel, &QLabel::linkActivated, this, [this, checkEntries]( const QString& link ) { - if ( link == "#details" ) - { - showDetailsDialog( checkEntries ); - } - } ); + connect( textLabel, &QLabel::linkActivated, this, &ResultsListWidget::linkClicked ); } else { @@ -267,9 +262,12 @@ ResultsListWidget::ResultsListWidget( QWidget* parent, const Calamares::Requirem void -ResultsListWidget::showDetailsDialog( const Calamares::RequirementsList& checkEntries ) +ResultsListWidget::linkClicked( const QString& link ) { - auto* dialog = new ResultsListDialog( this, checkEntries ); - dialog->exec(); - dialog->deleteLater(); + if ( link == "#details" ) + { + auto* dialog = new ResultsListDialog( this, m_entries ); + dialog->exec(); + dialog->deleteLater(); + } } diff --git a/src/modules/welcome/checker/ResultsListWidget.h b/src/modules/welcome/checker/ResultsListWidget.h index 8d0dcb155..4685ded9b 100644 --- a/src/modules/welcome/checker/ResultsListWidget.h +++ b/src/modules/welcome/checker/ResultsListWidget.h @@ -32,7 +32,9 @@ public: explicit ResultsListWidget( QWidget* parent, const Calamares::RequirementsList& checkEntries ); private: - void showDetailsDialog( const Calamares::RequirementsList& checkEntries ); + /// @brief A link in the explanatory text has been clicked + void linkClicked( const QString& link ); + void retranslate(); const Calamares::RequirementsList& m_entries; }; From 38d58e5b1610e4864499d6001642921278627d55 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 28 Jan 2020 13:32:53 +0100 Subject: [PATCH 10/16] [welcome] Hoist explanatory-label code - Create the label once, and it's ok for it to respond to links even if there's none in the code. - Turn into a member variable in preparation for retranslation-refactor. --- .../welcome/checker/CheckerContainer.cpp | 2 ++ .../welcome/checker/ResultsListWidget.cpp | 28 +++++++++---------- .../welcome/checker/ResultsListWidget.h | 4 ++- 3 files changed, 19 insertions(+), 15 deletions(-) diff --git a/src/modules/welcome/checker/CheckerContainer.cpp b/src/modules/welcome/checker/CheckerContainer.cpp index 414250582..3dfacb8f6 100644 --- a/src/modules/welcome/checker/CheckerContainer.cpp +++ b/src/modules/welcome/checker/CheckerContainer.cpp @@ -29,6 +29,8 @@ #include "utils/Retranslator.h" #include "widgets/WaitingWidget.h" +#include + CheckerContainer::CheckerContainer( QWidget* parent ) : QWidget( parent ) , m_waitingWidget( new WaitingWidget( QString(), this ) ) diff --git a/src/modules/welcome/checker/ResultsListWidget.cpp b/src/modules/welcome/checker/ResultsListWidget.cpp index 7f68d22ff..d7fdf5437 100644 --- a/src/modules/welcome/checker/ResultsListWidget.cpp +++ b/src/modules/welcome/checker/ResultsListWidget.cpp @@ -29,10 +29,10 @@ #include "widgets/FixedAspectRatioLabel.h" #include -#include #include #include #include +#include static void createResultWidgets( QLayout* layout, @@ -157,6 +157,13 @@ ResultsListWidget::ResultsListWidget( QWidget* parent, const Calamares::Requirem spacerLayout->addSpacing( paddingSize ); CalamaresUtils::unmarginLayout( spacerLayout ); + m_explanation = new QLabel; + m_explanation->setWordWrap( true ); + m_explanation->setSizePolicy( QSizePolicy::Expanding, QSizePolicy::Preferred ); + m_explanation->setOpenExternalLinks( false ); + connect( m_explanation, &QLabel::linkActivated, this, &ResultsListWidget::linkClicked ); + entriesLayout->addWidget( m_explanation ); + // Check that all are satisfied (gives warnings if not) and // all *mandatory* entries are satisfied (gives errors if not). auto isUnSatisfied = []( const Calamares::RequirementEntry& e ) { return !e.satisfied; }; @@ -182,11 +189,6 @@ ResultsListWidget::ResultsListWidget( QWidget* parent, const Calamares::Requirem } } - QLabel* textLabel = new QLabel; - - textLabel->setWordWrap( true ); - entriesLayout->insertWidget( 0, textLabel ); - textLabel->setSizePolicy( QSizePolicy::Expanding, QSizePolicy::Preferred ); if ( !requirementsSatisfied ) { @@ -203,9 +205,7 @@ ResultsListWidget::ResultsListWidget( QWidget* parent, const Calamares::Requirem "requirements for installing %1.
" "Installation cannot continue. " "Details..." ); - textLabel->setText( message.arg( *Calamares::Branding::ShortVersionedName ) ); ) - textLabel->setOpenExternalLinks( false ); - connect( textLabel, &QLabel::linkActivated, this, &ResultsListWidget::linkClicked ); + m_explanation->setText( message.arg( *Calamares::Branding::ShortVersionedName ) ); ) } else { @@ -218,7 +218,7 @@ ResultsListWidget::ResultsListWidget( QWidget* parent, const Calamares::Requirem "recommended requirements for installing %1.
" "Installation can continue, but some features " "might be disabled." ); - textLabel->setText( message.arg( *Calamares::Branding::ShortVersionedName ) ); ) + m_explanation->setText( message.arg( *Calamares::Branding::ShortVersionedName ) ); ) } } @@ -249,10 +249,10 @@ ResultsListWidget::ResultsListWidget( QWidget* parent, const Calamares::Requirem imageLabel->setSizePolicy( QSizePolicy::Expanding, QSizePolicy::Expanding ); } } - CALAMARES_RETRANSLATE( textLabel->setText( tr( "This program will ask you some questions and " - "set up %2 on your computer." ) - .arg( *Calamares::Branding::ProductName ) ); - textLabel->setAlignment( Qt::AlignCenter ); ) + CALAMARES_RETRANSLATE( m_explanation->setText( tr( "This program will ask you some questions and " + "set up %2 on your computer." ) + .arg( *Calamares::Branding::ProductName ) ); + m_explanation->setAlignment( Qt::AlignCenter ); ) } else { diff --git a/src/modules/welcome/checker/ResultsListWidget.h b/src/modules/welcome/checker/ResultsListWidget.h index 4685ded9b..62f2d1a17 100644 --- a/src/modules/welcome/checker/ResultsListWidget.h +++ b/src/modules/welcome/checker/ResultsListWidget.h @@ -22,9 +22,10 @@ #include "modulesystem/Requirement.h" -#include #include +class QLabel; + class ResultsListWidget : public QWidget { Q_OBJECT @@ -36,6 +37,7 @@ private: void linkClicked( const QString& link ); void retranslate(); + QLabel* m_explanation = nullptr; ///< Explanatory text above the list, with link const Calamares::RequirementsList& m_entries; }; From 39534325e68e8ccb5e0e5963eb751baf6e122a2c Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 28 Jan 2020 13:39:27 +0100 Subject: [PATCH 11/16] [welcome] Re-use widget creation code - for the list, the code can be the same as for the dialog, only the predicate is different. - while here, implement retranslate() since there's no text on the list widgets otherwise. --- .../welcome/checker/ResultsListWidget.cpp | 41 ++++++++++--------- .../welcome/checker/ResultsListWidget.h | 3 ++ 2 files changed, 24 insertions(+), 20 deletions(-) diff --git a/src/modules/welcome/checker/ResultsListWidget.cpp b/src/modules/welcome/checker/ResultsListWidget.cpp index d7fdf5437..db28d079b 100644 --- a/src/modules/welcome/checker/ResultsListWidget.cpp +++ b/src/modules/welcome/checker/ResultsListWidget.cpp @@ -110,9 +110,9 @@ ResultsListDialog::ResultsListDialog( QWidget* parent, const Calamares::Requirem setLayout( mainLayout ); - CALAMARES_RETRANSLATE_SLOT( &ResultsListDialog::retranslate ) - connect( buttonBox, &QDialogButtonBox::clicked, this, &QDialog::close ); + + CALAMARES_RETRANSLATE_SLOT( &ResultsListDialog::retranslate ) retranslate(); // Do it now to fill in the texts } @@ -171,24 +171,7 @@ ResultsListWidget::ResultsListWidget( QWidget* parent, const Calamares::Requirem const bool requirementsSatisfied = std::none_of( checkEntries.begin(), checkEntries.end(), isUnSatisfied ); const bool mandatorySatisfied = std::none_of( checkEntries.begin(), checkEntries.end(), isMandatoryAndUnSatisfied ); - for ( const auto& entry : checkEntries ) - { - if ( !entry.satisfied ) - { - ResultWidget* ciw = new ResultWidget( entry.satisfied, entry.mandatory ); - CALAMARES_RETRANSLATE( ciw->setText( entry.negatedText() ); ) - entriesLayout->addWidget( ciw ); - ciw->setSizePolicy( QSizePolicy::Expanding, QSizePolicy::Preferred ); - ciw->setAutoFillBackground( true ); - QPalette pal( ciw->palette() ); - QColor bgColor = pal.window().color(); - int bgHue = ( entry.satisfied ) ? bgColor.hue() : ( entry.mandatory ) ? 0 : 60; - bgColor.setHsv( bgHue, 64, bgColor.value() ); - pal.setColor( QPalette::Window, bgColor ); - ciw->setPalette( pal ); - } - } - + createResultWidgets( entriesLayout, m_resultWidgets, checkEntries, isUnSatisfied ); if ( !requirementsSatisfied ) { @@ -258,6 +241,9 @@ ResultsListWidget::ResultsListWidget( QWidget* parent, const Calamares::Requirem { mainLayout->addStretch(); } + + CALAMARES_RETRANSLATE_SLOT( &ResultsListWidget::retranslate ) + retranslate(); } @@ -271,3 +257,18 @@ ResultsListWidget::linkClicked( const QString& link ) dialog->deleteLater(); } } + +void +ResultsListWidget::retranslate() +{ + int i = 0; + for ( const auto& entry : m_entries ) + { + if ( entry.satisfied ) + { + continue; + } + m_resultWidgets[ i ]->setText( entry.negatedText() ); + i++; + } +} diff --git a/src/modules/welcome/checker/ResultsListWidget.h b/src/modules/welcome/checker/ResultsListWidget.h index 62f2d1a17..357ad88c2 100644 --- a/src/modules/welcome/checker/ResultsListWidget.h +++ b/src/modules/welcome/checker/ResultsListWidget.h @@ -20,6 +20,8 @@ #ifndef CHECKER_RESULTSLISTWIDGET_H #define CHECKER_RESULTSLISTWIDGET_H +#include "ResultWidget.h" + #include "modulesystem/Requirement.h" #include @@ -39,6 +41,7 @@ private: QLabel* m_explanation = nullptr; ///< Explanatory text above the list, with link const Calamares::RequirementsList& m_entries; + QList< ResultWidget* > m_resultWidgets; ///< One widget for each unsatisfied entry }; #endif // CHECKER_RESULTSLISTWIDGET_H From 28d91979c3e0c31680eb03314d7b6b7d207f61bd Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 28 Jan 2020 13:50:23 +0100 Subject: [PATCH 12/16] [welcome] Make resultWidgets less error-prone - instead of counting and needing to keep track of the predicate applied while creating the widgets, push nullptrs to the widget list instead reflecting "this entry did not satisfy the predicate for widget creation". --- .../welcome/checker/ResultsListWidget.cpp | 25 +++++++++++++------ 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/src/modules/welcome/checker/ResultsListWidget.cpp b/src/modules/welcome/checker/ResultsListWidget.cpp index db28d079b..f8fe5d1a6 100644 --- a/src/modules/welcome/checker/ResultsListWidget.cpp +++ b/src/modules/welcome/checker/ResultsListWidget.cpp @@ -34,16 +34,29 @@ #include #include +/** @brief Add widgets to @p layout for the list @p checkEntries + * + * The @p resultWidgets is filled with pointers to the widgets; + * for each entry in @p checkEntries that satisfies @p predicate, + * a widget is created, otherwise a nullptr is added instead. + * + * Adds all the widgets to the given @p layout. + * + * Afterwards, @p resultWidgets has a length equal to @p checkEntries. + */ static void createResultWidgets( QLayout* layout, QList< ResultWidget* >& resultWidgets, const Calamares::RequirementsList& checkEntries, std::function< bool( const Calamares::RequirementEntry& ) > predicate ) { + resultWidgets.clear(); + resultWidgets.reserve( checkEntries.count() ); for ( const auto& entry : checkEntries ) { if ( !predicate( entry ) ) { + resultWidgets.append( nullptr ); continue; } @@ -127,11 +140,10 @@ ResultsListDialog::retranslate() int i = 0; for ( const auto& entry : m_entries ) { - if ( !entry.hasDetails() ) + if ( m_resultWidgets[ i ] ) { - continue; + m_resultWidgets[ i ]->setText( entry.enumerationText() ); } - m_resultWidgets[ i ]->setText( entry.enumerationText() ); i++; } } @@ -241,7 +253,7 @@ ResultsListWidget::ResultsListWidget( QWidget* parent, const Calamares::Requirem { mainLayout->addStretch(); } - + CALAMARES_RETRANSLATE_SLOT( &ResultsListWidget::retranslate ) retranslate(); } @@ -264,11 +276,10 @@ ResultsListWidget::retranslate() int i = 0; for ( const auto& entry : m_entries ) { - if ( entry.satisfied ) + if ( m_resultWidgets[ i ] ) { - continue; + m_resultWidgets[ i ]->setText( entry.negatedText() ); } - m_resultWidgets[ i ]->setText( entry.negatedText() ); i++; } } From 221a79b64c5dc1bcc9feedc01d7d4c756a403793 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 28 Jan 2020 13:58:08 +0100 Subject: [PATCH 13/16] [welcome] Simplify logic - \not mandatorySatisfied \implies \not requirementsSatisfied, so the ifs can be combined and simplified --- src/modules/welcome/checker/ResultsListWidget.cpp | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/modules/welcome/checker/ResultsListWidget.cpp b/src/modules/welcome/checker/ResultsListWidget.cpp index f8fe5d1a6..5a936b1a0 100644 --- a/src/modules/welcome/checker/ResultsListWidget.cpp +++ b/src/modules/welcome/checker/ResultsListWidget.cpp @@ -215,9 +215,9 @@ ResultsListWidget::ResultsListWidget( QWidget* parent, const Calamares::Requirem "might be disabled." ); m_explanation->setText( message.arg( *Calamares::Branding::ShortVersionedName ) ); ) } + mainLayout->addStretch(); } - - if ( requirementsSatisfied && mandatorySatisfied ) + else { if ( !Calamares::Branding::instance()->imagePath( Calamares::Branding::ProductWelcome ).isEmpty() ) { @@ -249,10 +249,6 @@ ResultsListWidget::ResultsListWidget( QWidget* parent, const Calamares::Requirem .arg( *Calamares::Branding::ProductName ) ); m_explanation->setAlignment( Qt::AlignCenter ); ) } - else - { - mainLayout->addStretch(); - } CALAMARES_RETRANSLATE_SLOT( &ResultsListWidget::retranslate ) retranslate(); From 1ac4786365663d5eafcccd14b87d7d4c9477db26 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 28 Jan 2020 14:15:57 +0100 Subject: [PATCH 14/16] [welcome] Move all the translation work to the slot - this needs to (re)check the satisfaction states to figure out the message, but that's useful if the state of the checks changes (e.g. in #1106) --- .../welcome/checker/ResultsListWidget.cpp | 80 +++++++++++-------- 1 file changed, 46 insertions(+), 34 deletions(-) diff --git a/src/modules/welcome/checker/ResultsListWidget.cpp b/src/modules/welcome/checker/ResultsListWidget.cpp index 5a936b1a0..83b54f1c5 100644 --- a/src/modules/welcome/checker/ResultsListWidget.cpp +++ b/src/modules/welcome/checker/ResultsListWidget.cpp @@ -35,13 +35,13 @@ #include /** @brief Add widgets to @p layout for the list @p checkEntries - * + * * The @p resultWidgets is filled with pointers to the widgets; * for each entry in @p checkEntries that satisfies @p predicate, * a widget is created, otherwise a nullptr is added instead. - * + * * Adds all the widgets to the given @p layout. - * + * * Afterwards, @p resultWidgets has a length equal to @p checkEntries. */ static void @@ -188,33 +188,6 @@ ResultsListWidget::ResultsListWidget( QWidget* parent, const Calamares::Requirem if ( !requirementsSatisfied ) { entriesLayout->insertSpacing( 1, CalamaresUtils::defaultFontHeight() / 2 ); - - if ( !mandatorySatisfied ) - { - CALAMARES_RETRANSLATE( QString message = Calamares::Settings::instance()->isSetupMode() - ? tr( "This computer does not satisfy the minimum " - "requirements for setting up %1.
" - "Setup cannot continue. " - "Details..." ) - : tr( "This computer does not satisfy the minimum " - "requirements for installing %1.
" - "Installation cannot continue. " - "Details..." ); - m_explanation->setText( message.arg( *Calamares::Branding::ShortVersionedName ) ); ) - } - else - { - CALAMARES_RETRANSLATE( QString message = Calamares::Settings::instance()->isSetupMode() - ? tr( "This computer does not satisfy some of the " - "recommended requirements for setting up %1.
" - "Setup can continue, but some features " - "might be disabled." ) - : tr( "This computer does not satisfy some of the " - "recommended requirements for installing %1.
" - "Installation can continue, but some features " - "might be disabled." ); - m_explanation->setText( message.arg( *Calamares::Branding::ShortVersionedName ) ); ) - } mainLayout->addStretch(); } else @@ -244,10 +217,7 @@ ResultsListWidget::ResultsListWidget( QWidget* parent, const Calamares::Requirem imageLabel->setSizePolicy( QSizePolicy::Expanding, QSizePolicy::Expanding ); } } - CALAMARES_RETRANSLATE( m_explanation->setText( tr( "This program will ask you some questions and " - "set up %2 on your computer." ) - .arg( *Calamares::Branding::ProductName ) ); - m_explanation->setAlignment( Qt::AlignCenter ); ) + m_explanation->setAlignment( Qt::AlignCenter ); } CALAMARES_RETRANSLATE_SLOT( &ResultsListWidget::retranslate ) @@ -278,4 +248,46 @@ ResultsListWidget::retranslate() } i++; } + + // Check that all are satisfied (gives warnings if not) and + // all *mandatory* entries are satisfied (gives errors if not). + auto isUnSatisfied = []( const Calamares::RequirementEntry& e ) { return !e.satisfied; }; + auto isMandatoryAndUnSatisfied = []( const Calamares::RequirementEntry& e ) { return e.mandatory && !e.satisfied; }; + const bool requirementsSatisfied = std::none_of( m_entries.begin(), m_entries.end(), isUnSatisfied ); + const bool mandatorySatisfied = std::none_of( m_entries.begin(), m_entries.end(), isMandatoryAndUnSatisfied ); + + if ( !requirementsSatisfied ) + { + QString message; + const bool setup = Calamares::Settings::instance()->isSetupMode(); + if ( !mandatorySatisfied ) + { + message = setup ? tr( "This computer does not satisfy the minimum " + "requirements for setting up %1.
" + "Setup cannot continue. " + "Details..." ) + : tr( "This computer does not satisfy the minimum " + "requirements for installing %1.
" + "Installation cannot continue. " + "Details..." ); + } + else + { + message = setup ? tr( "This computer does not satisfy some of the " + "recommended requirements for setting up %1.
" + "Setup can continue, but some features " + "might be disabled." ) + : tr( "This computer does not satisfy some of the " + "recommended requirements for installing %1.
" + "Installation can continue, but some features " + "might be disabled." ); + } + m_explanation->setText( message.arg( *Calamares::Branding::ShortVersionedName ) ); + } + else + { + m_explanation->setText( tr( "This program will ask you some questions and " + "set up %2 on your computer." ) + .arg( *Calamares::Branding::ProductName ) ); + } } From 9d69d0a89306e72383b406bdca6d3a436f9b8f1f Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 28 Jan 2020 14:21:02 +0100 Subject: [PATCH 15/16] [welcome] Remove unused variables --- src/modules/welcome/checker/ResultsListWidget.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/modules/welcome/checker/ResultsListWidget.cpp b/src/modules/welcome/checker/ResultsListWidget.cpp index 83b54f1c5..6e32ebd1f 100644 --- a/src/modules/welcome/checker/ResultsListWidget.cpp +++ b/src/modules/welcome/checker/ResultsListWidget.cpp @@ -179,9 +179,7 @@ ResultsListWidget::ResultsListWidget( QWidget* parent, const Calamares::Requirem // Check that all are satisfied (gives warnings if not) and // all *mandatory* entries are satisfied (gives errors if not). auto isUnSatisfied = []( const Calamares::RequirementEntry& e ) { return !e.satisfied; }; - auto isMandatoryAndUnSatisfied = []( const Calamares::RequirementEntry& e ) { return e.mandatory && !e.satisfied; }; const bool requirementsSatisfied = std::none_of( checkEntries.begin(), checkEntries.end(), isUnSatisfied ); - const bool mandatorySatisfied = std::none_of( checkEntries.begin(), checkEntries.end(), isMandatoryAndUnSatisfied ); createResultWidgets( entriesLayout, m_resultWidgets, checkEntries, isUnSatisfied ); From ad4ac1d25c6013cff9617aed9a178a0895ae271c Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 28 Jan 2020 14:23:38 +0100 Subject: [PATCH 16/16] [welcome] Update copyrights --- src/modules/welcome/checker/CheckerContainer.cpp | 2 +- src/modules/welcome/checker/ResultsListWidget.cpp | 2 +- src/modules/welcome/checker/ResultsListWidget.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/modules/welcome/checker/CheckerContainer.cpp b/src/modules/welcome/checker/CheckerContainer.cpp index 3dfacb8f6..248b0481c 100644 --- a/src/modules/welcome/checker/CheckerContainer.cpp +++ b/src/modules/welcome/checker/CheckerContainer.cpp @@ -1,7 +1,7 @@ /* === This file is part of Calamares - === * * Copyright 2014-2017, Teo Mrnjavac - * Copyright 2017, 2019, Adriaan de Groot + * Copyright 2017, 2019-2020, Adriaan de Groot * Copyright 2017, Gabriel Craciunescu * * Calamares is free software: you can redistribute it and/or modify diff --git a/src/modules/welcome/checker/ResultsListWidget.cpp b/src/modules/welcome/checker/ResultsListWidget.cpp index 6e32ebd1f..f5877707f 100644 --- a/src/modules/welcome/checker/ResultsListWidget.cpp +++ b/src/modules/welcome/checker/ResultsListWidget.cpp @@ -1,7 +1,7 @@ /* === This file is part of Calamares - === * * Copyright 2014-2015, Teo Mrnjavac - * Copyright 2017, 2019, Adriaan de Groot + * Copyright 2017, 2019-2020, Adriaan de Groot * * Calamares is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by diff --git a/src/modules/welcome/checker/ResultsListWidget.h b/src/modules/welcome/checker/ResultsListWidget.h index 357ad88c2..25eae8b3e 100644 --- a/src/modules/welcome/checker/ResultsListWidget.h +++ b/src/modules/welcome/checker/ResultsListWidget.h @@ -1,7 +1,7 @@ /* === This file is part of Calamares - === * * Copyright 2014-2015, Teo Mrnjavac - * Copyright 2019, Adriaan de Groot + * Copyright 2019-2020, Adriaan de Groot * * Calamares is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by