From 4731d79a4f75d16160c0f62b21b978b534a780a8 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 4 Oct 2021 12:48:16 +0200 Subject: [PATCH] [summary] Reduce code-duplication The summary page can rely on the Config object to create lists of relevant steps; this code was declared but not defined / implemented for Config (but also not called, so it was ok). This is basically shuffling bits around in preparation for using the model directly, rather than re-implementing the widget-creation code. While here, split off the page-resizing into a free function so that the code reads nicer. --- src/modules/summary/Config.cpp | 12 ++++-- src/modules/summary/Config.h | 3 +- src/modules/summary/SummaryPage.cpp | 60 +++++++++-------------------- src/modules/summary/SummaryPage.h | 2 - 4 files changed, 29 insertions(+), 48 deletions(-) diff --git a/src/modules/summary/Config.cpp b/src/modules/summary/Config.cpp index f76b28959..98079cb6d 100644 --- a/src/modules/summary/Config.cpp +++ b/src/modules/summary/Config.cpp @@ -95,8 +95,8 @@ Config::retranslate() Q_EMIT messageChanged( m_message ); } -void -Config::collectSummaries( const Calamares::ViewStep* upToHere, Widgets withWidgets ) +Calamares::ViewStepList +Config::stepsForSummary( const Calamares::ViewStep* upToHere ) { Calamares::ViewStepList steps; for ( Calamares::ViewStep* step : Calamares::ViewManager::instance()->viewSteps() ) @@ -122,8 +122,14 @@ Config::collectSummaries( const Calamares::ViewStep* upToHere, Widgets withWidge steps.append( step ); } + return steps; +} - m_summary->setSummaryList( steps, withWidgets == Widgets::Enabled ); + +void +Config::collectSummaries( const Calamares::ViewStep* upToHere, Widgets withWidgets ) +{ + m_summary->setSummaryList( stepsForSummary( upToHere ), withWidgets == Widgets::Enabled ); } void diff --git a/src/modules/summary/Config.h b/src/modules/summary/Config.h index ac87c7db7..cf819b503 100644 --- a/src/modules/summary/Config.h +++ b/src/modules/summary/Config.h @@ -79,6 +79,8 @@ public: 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, Widgets withWidgets ); ///@brief Clear the model of steps (to avoid dangling widgets) @@ -90,7 +92,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 82670f603..f3f05c3cc 100644 --- a/src/modules/summary/SummaryPage.cpp +++ b/src/modules/summary/SummaryPage.cpp @@ -79,6 +79,22 @@ createBodyLabel( const QString& text, const QPalette& bodyPalette ) return label; } +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 @@ -99,7 +115,7 @@ SummaryPage::buildWidgets( Config* config, SummaryViewStep* viewstep ) bodyPalette.setColor( WindowBackground, palette().window().color().lighter( 108 ) ); bool first = true; - const Calamares::ViewStepList steps = stepsForSummary( Calamares::ViewManager::instance()->viewSteps(), viewstep ); + const Calamares::ViewStepList steps = Config::stepsForSummary( viewstep ); for ( Calamares::ViewStep* step : steps ) { @@ -138,47 +154,7 @@ SummaryPage::buildWidgets( Config* config, SummaryViewStep* viewstep ) 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 viewstep->ensureSize( widgetSize ); // Only expand height - } -} - -Calamares::ViewStepList -SummaryPage::stepsForSummary( const Calamares::ViewStepList& allSteps, SummaryViewStep* viewstep ) 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 ( viewstep == step ) - { - break; - } - - steps.append( step ); - } - - return steps; + ensureSize( this, m_scrollArea, viewstep ); } void diff --git a/src/modules/summary/SummaryPage.h b/src/modules/summary/SummaryPage.h index 5e201dc3a..9976020f7 100644 --- a/src/modules/summary/SummaryPage.h +++ b/src/modules/summary/SummaryPage.h @@ -53,8 +53,6 @@ public: void cleanup(); private: - Calamares::ViewStepList stepsForSummary( const Calamares::ViewStepList& allSteps, SummaryViewStep* viewstep ) const; - QVBoxLayout* m_layout = nullptr; QWidget* m_contentWidget = nullptr; QScrollArea* m_scrollArea = nullptr;