diff --git a/CHANGES b/CHANGES index b668e2496..349e36179 100644 --- a/CHANGES +++ b/CHANGES @@ -18,6 +18,8 @@ This release contains contributions from (alphabetically by first name): ## Modules ## - The *partition* module now consistently uses the configured EFI partition size (defaults to 300MiB). + - Internal changes in the *summary* module to increase consistency + between *summary* and *summaryq*. # 3.2.44.2 (2021-09-27) # diff --git a/src/modules/summary/Config.cpp b/src/modules/summary/Config.cpp index 5110e09a6..9038f786f 100644 --- a/src/modules/summary/Config.cpp +++ b/src/modules/summary/Config.cpp @@ -26,7 +26,9 @@ SummaryModel::SummaryModel( QObject* parent ) QHash< int, QByteArray > SummaryModel::roleNames() const { - return { { Qt::DisplayRole, "title" }, { Qt::UserRole, "message" } }; + // Not including WidgetRole here because that wouldn't make sense + // in a QML context which is where the roleNames are important. + return { { TitleRole, "title" }, { MessageRole, "message" } }; } QVariant @@ -36,8 +38,18 @@ SummaryModel::data( const QModelIndex& index, int role ) const { return QVariant(); } - const auto item = m_summary.at( index.row() ); - return role == Qt::DisplayRole ? item.title : item.message; + auto& item = m_summary.at( index.row() ); + switch ( role ) + { + case TitleRole: + return item.title; + case MessageRole: + return item.message; + case WidgetRole: + return item.widget ? QVariant::fromValue( item.widget ) : QVariant(); + default: + return QVariant(); + } } int @@ -84,19 +96,19 @@ Config::retranslate() if ( Calamares::Settings::instance()->isSetupMode() ) { m_message = tr( "This is an overview of what will happen once you start " - "the setup procedure." ); + "the setup procedure." ); } else { m_message = tr( "This is an overview of what will happen once you start " - "the install procedure." ); + "the install procedure." ); } Q_EMIT titleChanged( m_title ); Q_EMIT messageChanged( m_message ); } -void -Config::collectSummaries( const Calamares::ViewStep* upToHere ) +Calamares::ViewStepList +Config::stepsForSummary( const Calamares::ViewStep* upToHere ) { Calamares::ViewStepList steps; for ( Calamares::ViewStep* step : Calamares::ViewManager::instance()->viewSteps() ) @@ -122,6 +134,18 @@ Config::collectSummaries( const Calamares::ViewStep* upToHere ) steps.append( step ); } - - m_summary->setSummaryList( steps ); + return steps; +} + + +void +Config::collectSummaries( const Calamares::ViewStep* upToHere, Widgets withWidgets ) +{ + m_summary->setSummaryList( stepsForSummary( upToHere ), withWidgets == Widgets::Enabled ); +} + +void +Config::clearSummaries() +{ + m_summary->setSummaryList( {}, false ); } diff --git a/src/modules/summary/Config.h b/src/modules/summary/Config.h index 15604d933..f0732f448 100644 --- a/src/modules/summary/Config.h +++ b/src/modules/summary/Config.h @@ -38,6 +38,13 @@ class SummaryModel : public QAbstractListModel friend class Config; public: + enum Roles : int + { + TitleRole = Qt::DisplayRole, // Name of the step + MessageRole = Qt::UserRole, // String saying what it will do + WidgetRole, // Pointer to widget + }; + explicit SummaryModel( QObject* parent = nullptr ); int rowCount( const QModelIndex& = QModelIndex() ) const override; QVariant data( const QModelIndex& index, int role ) const override; @@ -72,8 +79,19 @@ class Config : public QObject public: explicit Config( QObject* parent = nullptr ); + ///@brief Include widgets in the model? + enum class Widgets + { + Disabled, + Enabled + }; + + static Calamares::ViewStepList stepsForSummary( const Calamares::ViewStep* upToHere ); + ///@brief Called later, to load the model once all viewsteps are there - void collectSummaries( const Calamares::ViewStep* upToHere ); + void collectSummaries( const Calamares::ViewStep* upToHere, Widgets withWidgets ); + ///@brief Clear the model of steps (to avoid dangling widgets) + void clearSummaries(); QAbstractListModel* summaryModel() const { return m_summary; } @@ -81,7 +99,6 @@ public: QString message() const { return m_message; } private: - Calamares::ViewStepList stepsForSummary( const Calamares::ViewStepList& allSteps ) const; void retranslate(); SummaryModel* m_summary; diff --git a/src/modules/summary/SummaryPage.cpp b/src/modules/summary/SummaryPage.cpp index e0c063e52..c0df7afd7 100644 --- a/src/modules/summary/SummaryPage.cpp +++ b/src/modules/summary/SummaryPage.cpp @@ -27,18 +27,15 @@ #include #include -SummaryPage::SummaryPage( Config* config, const SummaryViewStep* thisViewStep, QWidget* parent ) +SummaryPage::SummaryPage( Config* config, QWidget* parent ) : QWidget() - , m_thisViewStep( thisViewStep ) , m_contentWidget( nullptr ) , m_scrollArea( new QScrollArea( this ) ) { Q_UNUSED( parent ) - this->setObjectName( "summaryStep" ); - Q_ASSERT( m_thisViewStep ); QVBoxLayout* layout = new QVBoxLayout( this ); layout->setContentsMargins( 0, 0, 0, 0 ); @@ -82,10 +79,52 @@ createBodyLabel( const QString& text, const QPalette& bodyPalette ) return label; } +static QWidget* +createStepWidget( const QString& description, QWidget* innerWidget, const QPalette& palette ) +{ + QWidget* w = new QWidget(); + QHBoxLayout* itemBodyLayout = new QHBoxLayout; + w->setLayout( itemBodyLayout ); + + // Indent the inner box by a bit + itemBodyLayout->addSpacing( CalamaresUtils::defaultFontHeight() * 2 ); + QVBoxLayout* itemBodyCoreLayout = new QVBoxLayout; + itemBodyLayout->addLayout( itemBodyCoreLayout ); + CalamaresUtils::unmarginLayout( itemBodyLayout ); + + itemBodyCoreLayout->addSpacing( CalamaresUtils::defaultFontHeight() / 2 ); + if ( !description.isEmpty() ) + { + itemBodyCoreLayout->addWidget( createBodyLabel( description, palette ) ); + } + if ( innerWidget ) + { + itemBodyCoreLayout->addWidget( innerWidget ); + } + + return w; +} + +static void +ensureSize( QWidget* parent, QScrollArea* container, Calamares::ViewStep* viewstep ) +{ + auto summarySize = container->widget()->sizeHint(); + if ( summarySize.height() > container->size().height() ) + { + auto enlarge = 2 + summarySize.height() - container->size().height(); + auto widgetSize = parent->size(); + widgetSize.setHeight( widgetSize.height() + enlarge ); + + cDebug() << "Summary widget is larger than viewport, enlarge by" << enlarge << "to" << widgetSize; + + emit viewstep->ensureSize( widgetSize ); // Only expand height + } +} + // Adds a widget for those ViewSteps that want a summary; // see SummaryPage documentation and also ViewStep docs. void -SummaryPage::onActivate() +SummaryPage::buildWidgets( Config* config, SummaryViewStep* viewstep ) { const int SECTION_SPACING = 12; @@ -101,91 +140,37 @@ SummaryPage::onActivate() QPalette bodyPalette( palette() ); bodyPalette.setColor( WindowBackground, palette().window().color().lighter( 108 ) ); - bool first = true; - const Calamares::ViewStepList steps = stepsForSummary( Calamares::ViewManager::instance()->viewSteps() ); + const auto* model = config->summaryModel(); + const auto rowCount = model->rowCount(); - for ( Calamares::ViewStep* step : steps ) + for ( int row = 0; row < rowCount; row++ ) { - QString text = step->prettyStatus(); - QWidget* widget = step->createSummaryWidget(); + const auto rowIndex = model->index( row ); + QString title = model->data( rowIndex, SummaryModel::TitleRole ).toString(); + QString text = model->data( rowIndex, SummaryModel::MessageRole ).toString(); + QWidget* widget = model->data( rowIndex, SummaryModel::WidgetRole ).value< QWidget* >(); if ( text.isEmpty() && !widget ) { continue; } - if ( !first ) + if ( row > 0 ) { m_layout->addSpacing( SECTION_SPACING ); } - first = false; - m_layout->addWidget( createTitleLabel( step->prettyName(), titleFont ) ); - QHBoxLayout* itemBodyLayout = new QHBoxLayout; - m_layout->addSpacing( CalamaresUtils::defaultFontHeight() / 2 ); - m_layout->addLayout( itemBodyLayout ); - itemBodyLayout->addSpacing( CalamaresUtils::defaultFontHeight() * 2 ); - QVBoxLayout* itemBodyCoreLayout = new QVBoxLayout; - itemBodyLayout->addLayout( itemBodyCoreLayout ); - CalamaresUtils::unmarginLayout( itemBodyLayout ); - if ( !text.isEmpty() ) - { - itemBodyCoreLayout->addWidget( createBodyLabel( text, bodyPalette ) ); - } - if ( widget ) - { - itemBodyCoreLayout->addWidget( widget ); - } - itemBodyLayout->addSpacing( CalamaresUtils::defaultFontHeight() * 2 ); + m_layout->addWidget( createTitleLabel( title, titleFont ) ); + m_layout->addWidget( createStepWidget( text, widget, bodyPalette ) ); } m_layout->addStretch(); m_scrollArea->setWidget( m_contentWidget ); - - auto summarySize = m_contentWidget->sizeHint(); - if ( summarySize.height() > m_scrollArea->size().height() ) - { - auto enlarge = 2 + summarySize.height() - m_scrollArea->size().height(); - auto widgetSize = this->size(); - widgetSize.setHeight( widgetSize.height() + enlarge ); - - cDebug() << "Summary widget is larger than viewport, enlarge by" << enlarge << "to" << widgetSize; - - emit m_thisViewStep->ensureSize( widgetSize ); // Only expand height - } -} - -Calamares::ViewStepList -SummaryPage::stepsForSummary( const Calamares::ViewStepList& allSteps ) const -{ - Calamares::ViewStepList steps; - for ( Calamares::ViewStep* step : allSteps ) - { - // We start from the beginning of the complete steps list. If we encounter any - // ExecutionViewStep, it means there was an execution phase in the past, and any - // jobs from before that phase were already executed, so we can safely clear the - // list of steps to summarize and start collecting from scratch. - if ( qobject_cast< Calamares::ExecutionViewStep* >( step ) ) - { - steps.clear(); - continue; - } - - // If we reach the parent step of this page, we're done collecting the list of - // steps to summarize. - if ( m_thisViewStep == step ) - { - break; - } - - steps.append( step ); - } - - return steps; + ensureSize( this, m_scrollArea, viewstep ); } void -SummaryPage::onLeave() +SummaryPage::cleanup() { delete m_contentWidget; m_contentWidget = nullptr; diff --git a/src/modules/summary/SummaryPage.h b/src/modules/summary/SummaryPage.h index 7d98cc711..9976020f7 100644 --- a/src/modules/summary/SummaryPage.h +++ b/src/modules/summary/SummaryPage.h @@ -45,22 +45,17 @@ class SummaryPage : public QWidget { Q_OBJECT public: - explicit SummaryPage( Config* config, const SummaryViewStep* thisViewStep, QWidget* parent = nullptr ); + explicit SummaryPage( Config* config, QWidget* parent = nullptr ); /// @brief Create contents showing all of the summary - void onActivate(); + void buildWidgets( Config* config, SummaryViewStep* viewstep ); /// @brief Clean up the widgets - void onLeave(); + void cleanup(); private: - Calamares::ViewStepList stepsForSummary( const Calamares::ViewStepList& allSteps ) const; - - const SummaryViewStep* m_thisViewStep; - QVBoxLayout* m_layout = nullptr; QWidget* m_contentWidget = nullptr; - - QScrollArea* m_scrollArea; + QScrollArea* m_scrollArea = nullptr; }; #endif // SUMMARYPAGE_H diff --git a/src/modules/summary/SummaryViewStep.cpp b/src/modules/summary/SummaryViewStep.cpp index d4e439ae3..9d63d0d37 100644 --- a/src/modules/summary/SummaryViewStep.cpp +++ b/src/modules/summary/SummaryViewStep.cpp @@ -16,7 +16,7 @@ CALAMARES_PLUGIN_FACTORY_DEFINITION( SummaryViewStepFactory, registerPlugin< Sum SummaryViewStep::SummaryViewStep( QObject* parent ) : Calamares::ViewStep( parent ) , m_config( new Config( this ) ) - , m_widget( new SummaryPage( m_config, this ) ) + , m_widget( new SummaryPage( m_config ) ) { emit nextStatusChanged( true ); } @@ -84,13 +84,14 @@ SummaryViewStep::jobs() const void SummaryViewStep::onActivate() { - m_config->collectSummaries( this ); - m_widget->onActivate(); + m_config->collectSummaries( this, Config::Widgets::Enabled ); + m_widget->buildWidgets( m_config, this ); } void SummaryViewStep::onLeave() { - m_widget->onLeave(); + m_config->clearSummaries(); + m_widget->cleanup(); } diff --git a/src/modules/summaryq/SummaryQmlViewStep.cpp b/src/modules/summaryq/SummaryQmlViewStep.cpp index 23e18a861..a5acdfddd 100644 --- a/src/modules/summaryq/SummaryQmlViewStep.cpp +++ b/src/modules/summaryq/SummaryQmlViewStep.cpp @@ -69,5 +69,5 @@ SummaryQmlViewStep::onActivate() { // Collect the steps before this one: those need to have their // summary (text or widget) displayed. - m_config->collectSummaries( this ); + m_config->collectSummaries( this, Config::Widgets::Disabled ); }