From 43cd415d9adf3ea99216e2c6d709a41ac747fc9d Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Wed, 29 Jul 2020 13:42:56 +0200 Subject: [PATCH 1/5] [partition] Switch to 'modern' Error/ok icons --- src/modules/partition/gui/EncryptWidget.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/modules/partition/gui/EncryptWidget.cpp b/src/modules/partition/gui/EncryptWidget.cpp index 42a073db7..8fb9a7b30 100644 --- a/src/modules/partition/gui/EncryptWidget.cpp +++ b/src/modules/partition/gui/EncryptWidget.cpp @@ -141,15 +141,16 @@ EncryptWidget::onPassphraseEdited() m_ui->m_iconLabel->setToolTip( QString() ); if ( p1.isEmpty() && p2.isEmpty() ) { - m_ui->m_iconLabel->clear(); + applyPixmap( m_ui->m_iconLabel, CalamaresUtils::StatusWarning ); + m_ui->m_iconLabel->setToolTip( tr( "Please enter the same passphrase in both boxes." ) ); } else if ( p1 == p2 ) { - applyPixmap( m_ui->m_iconLabel, CalamaresUtils::Yes ); + applyPixmap( m_ui->m_iconLabel, CalamaresUtils::StatusOk ); } else { - applyPixmap( m_ui->m_iconLabel, CalamaresUtils::No ); + applyPixmap( m_ui->m_iconLabel, CalamaresUtils::StatusError ); m_ui->m_iconLabel->setToolTip( tr( "Please enter the same passphrase in both boxes." ) ); } From f1c4caba483c5ddadadfd85b72bdebb2f1857a5d Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Wed, 29 Jul 2020 13:59:06 +0200 Subject: [PATCH 2/5] [partition] Refactor checking next-enabled - move the calculations to an own method (so it can use early-return and log things to explain why next is disabled) --- src/modules/partition/gui/ChoicePage.cpp | 66 +++++++++++++++++------- src/modules/partition/gui/ChoicePage.h | 1 + 2 files changed, 48 insertions(+), 19 deletions(-) diff --git a/src/modules/partition/gui/ChoicePage.cpp b/src/modules/partition/gui/ChoicePage.cpp index 9e48e69ac..9f654b1bf 100644 --- a/src/modules/partition/gui/ChoicePage.cpp +++ b/src/modules/partition/gui/ChoicePage.cpp @@ -1442,46 +1442,74 @@ ChoicePage::currentChoice() const return m_choice; } - -void -ChoicePage::updateNextEnabled() +bool +ChoicePage::calculateNextEnabled() const { bool enabled = false; - auto sm_p = m_beforePartitionBarsView ? m_beforePartitionBarsView->selectionModel() : nullptr; switch ( m_choice ) { case NoChoice: - enabled = false; - break; + cDebug() << "No partitioning choice"; + return false; case Replace: case Alongside: - enabled = sm_p && sm_p->currentIndex().isValid(); + if ( !( sm_p && sm_p->currentIndex().isValid() ) ) + { + cDebug() << "No partition selected"; + return false; + } break; case Erase: case Manual: enabled = true; } - if ( m_isEfi && - ( m_choice == Alongside || - m_choice == Replace ) ) + if (!enabled) + { + cDebug() << "No valid choice made"; + return false; + } + + + if ( m_isEfi && ( m_choice == Alongside || m_choice == Replace ) ) { if ( m_core->efiSystemPartitions().count() == 0 ) - enabled = false; + { + cDebug() << "No EFI partition for alongside or replace"; + return false; + } } - if ( m_choice != Manual && - m_encryptWidget->isVisible() && - m_encryptWidget->state() == EncryptWidget::Encryption::Unconfirmed ) - enabled = false; + if ( m_choice != Manual && m_encryptWidget->isVisible() ) + { + switch ( m_encryptWidget->state() ) + { + case EncryptWidget::Encryption::Unconfirmed: + cDebug() << "No passphrase provided"; + return false; + case EncryptWidget::Encryption::Disabled: + case EncryptWidget::Encryption::Confirmed: + // Checkbox not checked, **or** passphrases match + break; + } + } - if ( enabled == m_nextEnabled ) - return; + return true; +} - m_nextEnabled = enabled; - emit nextStatusChanged( enabled ); + +void +ChoicePage::updateNextEnabled() +{ + bool enabled = calculateNextEnabled(); + + if ( enabled != m_nextEnabled ) + { + m_nextEnabled = enabled; + emit nextStatusChanged( enabled ); + } } void diff --git a/src/modules/partition/gui/ChoicePage.h b/src/modules/partition/gui/ChoicePage.h index 1ff8f0d40..a28892011 100644 --- a/src/modules/partition/gui/ChoicePage.h +++ b/src/modules/partition/gui/ChoicePage.h @@ -126,6 +126,7 @@ private slots: void onEraseSwapChoiceChanged(); private: + bool calculateNextEnabled() const; void updateNextEnabled(); void setupChoices(); QComboBox* createBootloaderComboBox( QWidget* parentButton ); From 0eb1f002db2d61ea2414cc80736c2e53de818b13 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Wed, 29 Jul 2020 14:13:43 +0200 Subject: [PATCH 3/5] [partition] defuse is-next-enabled Both the KPMCore and the ChoicePage -- asynchronously -- were connected to the nextStatusChanged() signal. So if the core said next was true, that could end up communicated to the ViewManager, enabling the *next* button in the UI. Changing to the *erase* page generally triggers a KPMCore reload, which later emits a `hasRootMountPointChanged()` signal, once the layout is applied and the disk gets a root mount point. So we'd get a `true` from KPMCore, which -- because it was connected directly to the signal to the VM -- would override any other considerations. Hook up both signals to an intermediate slot that just recalculates whether the next button should be enabled, based on the state both of the Choice page and whatever else. --- src/modules/partition/gui/PartitionViewStep.cpp | 9 +++++++-- src/modules/partition/gui/PartitionViewStep.h | 3 +++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/modules/partition/gui/PartitionViewStep.cpp b/src/modules/partition/gui/PartitionViewStep.cpp index b0142a82a..900d10e57 100644 --- a/src/modules/partition/gui/PartitionViewStep.cpp +++ b/src/modules/partition/gui/PartitionViewStep.cpp @@ -107,8 +107,8 @@ PartitionViewStep::continueLoading() m_waitingWidget->deleteLater(); m_waitingWidget = nullptr; - connect( m_core, &PartitionCoreModule::hasRootMountPointChanged, this, &PartitionViewStep::nextStatusChanged ); - connect( m_choicePage, &ChoicePage::nextStatusChanged, this, &PartitionViewStep::nextStatusChanged ); + connect( m_core, &PartitionCoreModule::hasRootMountPointChanged, this, &PartitionViewStep::nextPossiblyChanged ); + connect( m_choicePage, &ChoicePage::nextStatusChanged, this, &PartitionViewStep::nextPossiblyChanged ); } @@ -348,6 +348,11 @@ PartitionViewStep::isNextEnabled() const return false; } +void +PartitionViewStep::nextPossiblyChanged(bool) +{ + emit nextStatusChanged( isNextEnabled() ); +} bool PartitionViewStep::isBackEnabled() const diff --git a/src/modules/partition/gui/PartitionViewStep.h b/src/modules/partition/gui/PartitionViewStep.h index 63d11c816..47479a135 100644 --- a/src/modules/partition/gui/PartitionViewStep.h +++ b/src/modules/partition/gui/PartitionViewStep.h @@ -78,6 +78,9 @@ private: void initPartitionCoreModule(); void continueLoading(); + /// "slot" for changes to next-status from the KPMCore and ChoicePage + void nextPossiblyChanged( bool ); + PartitionCoreModule* m_core; QStackedWidget* m_widget; ChoicePage* m_choicePage; From ef4c2666e1d964f2c42fd39ea365029289c44770 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Wed, 29 Jul 2020 14:46:07 +0200 Subject: [PATCH 4/5] [partition] Update icons on all state changes The encryption widget (passphrase for disk encryption) should show ok / warning / error whenever the state changes; this avoids it showing up first with **no** icon (it should show a warning when both passphrases are empty). --- src/modules/partition/gui/EncryptWidget.cpp | 57 +++++++++++---------- 1 file changed, 30 insertions(+), 27 deletions(-) diff --git a/src/modules/partition/gui/EncryptWidget.cpp b/src/modules/partition/gui/EncryptWidget.cpp index 8fb9a7b30..5e44c15fd 100644 --- a/src/modules/partition/gui/EncryptWidget.cpp +++ b/src/modules/partition/gui/EncryptWidget.cpp @@ -91,9 +91,39 @@ EncryptWidget::retranslate() } +///@brief Give @p label the @p pixmap from the standard-pixmaps +static void +applyPixmap( QLabel* label, CalamaresUtils::ImageType pixmap ) +{ + label->setFixedWidth( label->height() ); + label->setPixmap( CalamaresUtils::defaultPixmap( pixmap, CalamaresUtils::Original, label->size() ) ); +} + void EncryptWidget::updateState() { + if ( m_ui->m_passphraseLineEdit->isVisible() ) + { + QString p1 = m_ui->m_passphraseLineEdit->text(); + QString p2 = m_ui->m_confirmLineEdit->text(); + + if ( p1.isEmpty() && p2.isEmpty() ) + { + applyPixmap( m_ui->m_iconLabel, CalamaresUtils::StatusWarning ); + m_ui->m_iconLabel->setToolTip( tr( "Please enter the same passphrase in both boxes." ) ); + } + else if ( p1 == p2 ) + { + applyPixmap( m_ui->m_iconLabel, CalamaresUtils::StatusOk ); + m_ui->m_iconLabel->setToolTip( QString() ); + } + else + { + applyPixmap( m_ui->m_iconLabel, CalamaresUtils::StatusError ); + m_ui->m_iconLabel->setToolTip( tr( "Please enter the same passphrase in both boxes." ) ); + } + } + Encryption newState; if ( m_ui->m_encryptCheckBox->isChecked() ) { @@ -119,14 +149,6 @@ EncryptWidget::updateState() } } -///@brief Give @p label the @p pixmap from the standard-pixmaps -static void -applyPixmap( QLabel* label, CalamaresUtils::ImageType pixmap ) -{ - label->setFixedWidth( label->height() ); - label->setPixmap( CalamaresUtils::defaultPixmap( pixmap, CalamaresUtils::Original, label->size() ) ); -} - void EncryptWidget::onPassphraseEdited() { @@ -135,25 +157,6 @@ EncryptWidget::onPassphraseEdited() m_ui->m_iconLabel->show(); } - QString p1 = m_ui->m_passphraseLineEdit->text(); - QString p2 = m_ui->m_confirmLineEdit->text(); - - m_ui->m_iconLabel->setToolTip( QString() ); - if ( p1.isEmpty() && p2.isEmpty() ) - { - applyPixmap( m_ui->m_iconLabel, CalamaresUtils::StatusWarning ); - m_ui->m_iconLabel->setToolTip( tr( "Please enter the same passphrase in both boxes." ) ); - } - else if ( p1 == p2 ) - { - applyPixmap( m_ui->m_iconLabel, CalamaresUtils::StatusOk ); - } - else - { - applyPixmap( m_ui->m_iconLabel, CalamaresUtils::StatusError ); - m_ui->m_iconLabel->setToolTip( tr( "Please enter the same passphrase in both boxes." ) ); - } - updateState(); } From 33fd5a1fad30b4d44c236335d8732923fbd2d12b Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Wed, 29 Jul 2020 17:56:30 +0200 Subject: [PATCH 5/5] [partition] Report a valid choice if a partition is selected --- src/modules/partition/gui/ChoicePage.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/modules/partition/gui/ChoicePage.cpp b/src/modules/partition/gui/ChoicePage.cpp index 9f654b1bf..1d0c96623 100644 --- a/src/modules/partition/gui/ChoicePage.cpp +++ b/src/modules/partition/gui/ChoicePage.cpp @@ -1460,6 +1460,7 @@ ChoicePage::calculateNextEnabled() const cDebug() << "No partition selected"; return false; } + enabled = true; break; case Erase: case Manual: