From 2a0716bf43d765c5d4b675a690b941e2350f9723 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Thu, 28 Nov 2019 11:24:56 +0100 Subject: [PATCH 1/8] [license] Move the 'please review' text to the top. --- src/modules/license/LicensePage.cpp | 8 ++------ src/modules/license/LicensePage.ui | 20 ++++++++++++++++++-- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/src/modules/license/LicensePage.cpp b/src/modules/license/LicensePage.cpp index c5ba38133..fcbe62b3d 100644 --- a/src/modules/license/LicensePage.cpp +++ b/src/modules/license/LicensePage.cpp @@ -157,9 +157,7 @@ LicensePage::retranslate() ui->mainText->setText( tr( "

License Agreement

" "This setup procedure will install proprietary " "software that is subject to licensing terms." ) ); - ui->additionalText->setText( tr( "Please review the End User License " - "Agreements (EULAs) above.
" - "If you do not agree with the terms, the setup procedure cannot continue." ) ); + ui->additionalText->setText( tr( "If you do not agree with the terms, the setup procedure cannot continue." ) ); } else { @@ -168,9 +166,7 @@ LicensePage::retranslate() "software that is subject to licensing terms " "in order to provide additional features and enhance the user " "experience." ) ); - ui->additionalText->setText( tr( "Please review the End User License " - "Agreements (EULAs) above.
" - "If you do not agree with the terms, proprietary software will not " + ui->additionalText->setText( tr( "If you do not agree with the terms, proprietary software will not " "be installed, and open source alternatives will be used instead." ) ); } ui->retranslateUi( this ); diff --git a/src/modules/license/LicensePage.ui b/src/modules/license/LicensePage.ui index 767b392a0..c5857a716 100644 --- a/src/modules/license/LicensePage.ui +++ b/src/modules/license/LicensePage.ui @@ -15,7 +15,7 @@ - + @@ -32,6 +32,16 @@ + + + + + + + Please review the End User License Agreements (EULAs). + + + @@ -47,6 +57,12 @@ + + + 0 + 0 + + QFrame::NoFrame @@ -65,7 +81,7 @@ 0 0 765 - 94 + 86 From 5ed8ec9990f202e3d51c45a532eeefeb8196cf73 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Thu, 28 Nov 2019 11:28:02 +0100 Subject: [PATCH 2/8] [license] Reduce translation overhead. --- src/modules/license/LicensePage.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/modules/license/LicensePage.cpp b/src/modules/license/LicensePage.cpp index fcbe62b3d..ae7d5faa2 100644 --- a/src/modules/license/LicensePage.cpp +++ b/src/modules/license/LicensePage.cpp @@ -123,7 +123,7 @@ LicensePage::LicensePage( QWidget* parent ) connect( ui->acceptCheckBox, &QCheckBox::toggled, this, &LicensePage::checkAcceptance ); - CALAMARES_RETRANSLATE( ui->acceptCheckBox->setText( tr( "I accept the terms and conditions above." ) ); ) + CALAMARES_RETRANSLATE_SLOT( &LicensePage::retranslate ) } void @@ -145,13 +145,12 @@ LicensePage::setEntries( const QList< LicenseEntry >& entriesList ) m_entries.append( w ); } ui->licenseEntriesLayout->addStretch(); - - CALAMARES_RETRANSLATE_SLOT( &LicensePage::retranslate ) } void LicensePage::retranslate() { + ui->acceptCheckBox->setText( tr( "I accept the terms and conditions above." ) ); if ( !m_allLicensesOptional ) { ui->mainText->setText( tr( "

License Agreement

" From d8020e3574ddc7ff5f265b5e8d9e76512eddd3a0 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Thu, 28 Nov 2019 12:03:22 +0100 Subject: [PATCH 3/8] [license] Tidy up setting-of-entries - we loop over all the entries anyway, so calculate allLicensesOptional along the way (debatable whether std::none_of is clearer) - always un-check the accept-box when resetting entries. --- src/modules/license/LicensePage.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/modules/license/LicensePage.cpp b/src/modules/license/LicensePage.cpp index ae7d5faa2..1cb42ea8b 100644 --- a/src/modules/license/LicensePage.cpp +++ b/src/modules/license/LicensePage.cpp @@ -130,21 +130,21 @@ void LicensePage::setEntries( const QList< LicenseEntry >& entriesList ) { CalamaresUtils::clearLayout( ui->licenseEntriesLayout ); + + m_allLicensesOptional = true; + m_entries.clear(); m_entries.reserve( entriesList.count() ); - - auto isRequired = []( const LicenseEntry& e ) { return e.m_required; }; - m_allLicensesOptional = std::none_of( entriesList.cbegin(), entriesList.cend(), isRequired ); - - checkAcceptance( false ); - for ( const LicenseEntry& entry : entriesList ) { LicenseWidget* w = new LicenseWidget( entry ); ui->licenseEntriesLayout->addWidget( w ); m_entries.append( w ); + m_allLicensesOptional &= !entry.isRequired(); } - ui->licenseEntriesLayout->addStretch(); + + ui->acceptCheckBox->setChecked(false); + checkAcceptance( false ); } void From d322d783eaaa5009a5e47d0bbd8f5239c66c3d63 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Thu, 28 Nov 2019 13:17:15 +0100 Subject: [PATCH 4/8] [license] Chase deprecations in Qt --- src/modules/license/LicenseWidget.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/license/LicenseWidget.cpp b/src/modules/license/LicenseWidget.cpp index 25509be95..91e5646dc 100644 --- a/src/modules/license/LicenseWidget.cpp +++ b/src/modules/license/LicenseWidget.cpp @@ -57,7 +57,7 @@ LicenseWidget::LicenseWidget( LicenseEntry entry, QWidget* parent ) , m_fullText( nullptr ) { QPalette pal( palette() ); - pal.setColor( QPalette::Background, palette().background().color().lighter( 108 ) ); + pal.setColor( QPalette::Background, palette().window().color().lighter( 108 ) ); setObjectName( "licenseItem" ); From c870fca787d6ca858132821cccca66c5c4dcdeb5 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Thu, 28 Nov 2019 13:23:11 +0100 Subject: [PATCH 5/8] [license] Use more meaningful names for arrows - The arrows Up, Down, Right are used on toolbuttons, but in the context of this module, those are directions with meaning; give them better names. - Because of #1268, the meaning of up- and down- may be swapped; I'm not sure of which look makes the most sense. This is prep- work for easily swapping the looks by using the meaningful names instead. SEE #1268 --- src/modules/license/LicenseWidget.cpp | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/modules/license/LicenseWidget.cpp b/src/modules/license/LicenseWidget.cpp index 91e5646dc..609e850a9 100644 --- a/src/modules/license/LicenseWidget.cpp +++ b/src/modules/license/LicenseWidget.cpp @@ -30,6 +30,10 @@ #include #include +static constexpr const auto ArrowOpenExternalLink = Qt::RightArrow; +static constexpr const auto ArrowLocalLicenseIsCollapsed = Qt::UpArrow; +static constexpr const auto ArrowLocalLicenseIsExpanded = Qt::DownArrow; + static QString loadLocalFile( const QUrl& u ) { @@ -82,7 +86,7 @@ LicenseWidget::LicenseWidget( LicenseEntry entry, QWidget* parent ) { QVBoxLayout* vLayout = new QVBoxLayout; - m_expandLicenseButton->setArrowType( Qt::UpArrow ); + m_expandLicenseButton->setArrowType( ArrowLocalLicenseIsCollapsed ); connect( m_expandLicenseButton, &QAbstractButton::clicked, this, &LicenseWidget::expandClicked ); vLayout->addLayout( wiLayout ); @@ -97,7 +101,7 @@ LicenseWidget::LicenseWidget( LicenseEntry entry, QWidget* parent ) } else { - m_expandLicenseButton->setArrowType( Qt::RightArrow ); + m_expandLicenseButton->setArrowType( ArrowOpenExternalLink ); connect( m_expandLicenseButton, &QAbstractButton::clicked, this, &LicenseWidget::viewClicked ); // Normally setOpenExternalLinks( true ) would do, but we need the @@ -163,19 +167,19 @@ LicenseWidget::retranslateUi() void LicenseWidget::expandClicked() { - if ( m_expandLicenseButton->arrowType() == Qt::DownArrow ) + if ( m_expandLicenseButton->arrowType() == ArrowLocalLicenseIsExpanded ) { - m_expandLicenseButton->setArrowType( Qt::UpArrow ); + m_expandLicenseButton->setArrowType( ArrowLocalLicenseIsCollapsed ); } else { - m_expandLicenseButton->setArrowType( Qt::DownArrow ); + m_expandLicenseButton->setArrowType( ArrowLocalLicenseIsExpanded ); } // Show/hide based on the new arrow direction. if ( m_fullText ) { - m_fullText->setHidden( m_expandLicenseButton->arrowType() == Qt::UpArrow ); + m_fullText->setHidden( m_expandLicenseButton->arrowType() == ArrowLocalLicenseIsCollapsed ); } updateExpandToolTip(); @@ -187,7 +191,7 @@ LicenseWidget::updateExpandToolTip() { if ( m_entry.isLocal() ) { - const bool isNowCollapsed = m_expandLicenseButton->arrowType() == Qt::UpArrow; + const bool isNowCollapsed = m_expandLicenseButton->arrowType() == ArrowLocalLicenseIsCollapsed; m_expandLicenseButton->setToolTip( isNowCollapsed ? tr( "Shows the complete license text" ) : tr( "Hide license text" ) ); From 1de6062233a7bc3fe7f2e7934333ec26c55bf6d6 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Thu, 28 Nov 2019 13:31:16 +0100 Subject: [PATCH 6/8] [license] Add should-be-expanded display option to license entries - In code, add the necessary bool - document meaning in the config file - actually expand the full text if the entry is local and set to expanding- by-default. This implementation is a bit lazy since it just pretends to click on the toggle button. - While here, reduce scope for UB by initializing POD members --- src/modules/license/LicensePage.cpp | 1 + src/modules/license/LicensePage.h | 6 ++++-- src/modules/license/LicenseWidget.cpp | 7 +++++++ src/modules/license/license.conf | 5 +++++ 4 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/modules/license/LicensePage.cpp b/src/modules/license/LicensePage.cpp index 1cb42ea8b..45205dcd4 100644 --- a/src/modules/license/LicensePage.cpp +++ b/src/modules/license/LicensePage.cpp @@ -72,6 +72,7 @@ LicenseEntry::LicenseEntry( const QVariantMap& conf ) m_url = QUrl( conf[ "url" ].toString() ); m_required = CalamaresUtils::getBool( conf, "required", false ); + m_expand = CalamaresUtils::getBool( conf, "expand", false ); bool ok = false; QString typeString = conf.value( "type", "software" ).toString(); diff --git a/src/modules/license/LicensePage.h b/src/modules/license/LicensePage.h index 65f26b543..dcd3ea7b4 100644 --- a/src/modules/license/LicensePage.h +++ b/src/modules/license/LicensePage.h @@ -56,13 +56,15 @@ struct LicenseEntry bool isValid() const { return !m_id.isEmpty(); } bool isRequired() const { return m_required; } bool isLocal() const; + bool expandByDefault() const { return m_expand; } QString m_id; QString m_prettyName; QString m_prettyVendor; - Type m_type; + Type m_type = Type::Software; QUrl m_url; - bool m_required; + bool m_required = false; + bool m_expand = false; }; class LicensePage : public QWidget diff --git a/src/modules/license/LicenseWidget.cpp b/src/modules/license/LicenseWidget.cpp index 609e850a9..a4c1dfaaa 100644 --- a/src/modules/license/LicenseWidget.cpp +++ b/src/modules/license/LicenseWidget.cpp @@ -98,6 +98,13 @@ LicenseWidget::LicenseWidget( LicenseEntry entry, QWidget* parent ) vLayout->addWidget( m_fullText ); setLayout( vLayout ); + + if ( m_entry.expandByDefault() ) + { + // Since we started in a collapsed state, toggle it to expand. + // This can only be done once the full text has been added. + expandClicked(); + } } else { diff --git a/src/modules/license/license.conf b/src/modules/license/license.conf index 9057f8a51..8a1672887 100644 --- a/src/modules/license/license.conf +++ b/src/modules/license/license.conf @@ -14,6 +14,10 @@ # to the URL is provided, which opens in the default web browser. A local # URL (i.e. file:///) assumes that the contents are HTML or plain text, and # displays the license in-line. YAML: string, mandatory. +# - expand A boolean value only relevant for **local** URLs. If true, +# the license text is displayed in "expanded" form by +# default, rather than requiring the user to first open it up. +# YAML: boolean, optional, default is false. entries: - id: nvidia name: Nvidia @@ -43,3 +47,4 @@ entries: type: software required: true url: file:../LICENSE + expand: true From 0b126b2c62d7a8f31a5d116895645fd05665f087 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 3 Dec 2019 12:49:11 +0100 Subject: [PATCH 7/8] [license] Massage the messages some more - split shared

message off - do some string-concatenation, but only of whole sentences - shave off some vertical space by dropping the mainsubtext item --- src/modules/license/LicensePage.cpp | 25 ++++++++++++++++--------- src/modules/license/LicensePage.ui | 10 ---------- 2 files changed, 16 insertions(+), 19 deletions(-) diff --git a/src/modules/license/LicensePage.cpp b/src/modules/license/LicensePage.cpp index 45205dcd4..915b396a7 100644 --- a/src/modules/license/LicensePage.cpp +++ b/src/modules/license/LicensePage.cpp @@ -144,7 +144,7 @@ LicensePage::setEntries( const QList< LicenseEntry >& entriesList ) m_allLicensesOptional &= !entry.isRequired(); } - ui->acceptCheckBox->setChecked(false); + ui->acceptCheckBox->setChecked( false ); checkAcceptance( false ); } @@ -152,20 +152,27 @@ void LicensePage::retranslate() { ui->acceptCheckBox->setText( tr( "I accept the terms and conditions above." ) ); + + QString header = tr( "

License Agreement

" ); + QString review = tr( "Please review the End User License Agreements (EULAs)." ); + const auto br = QStringLiteral( "
" ); + if ( !m_allLicensesOptional ) { - ui->mainText->setText( tr( "

License Agreement

" - "This setup procedure will install proprietary " - "software that is subject to licensing terms." ) ); + ui->mainText->setText( header + + tr( "This setup procedure will install proprietary " + "software that is subject to licensing terms." ) + + br + review ); ui->additionalText->setText( tr( "If you do not agree with the terms, the setup procedure cannot continue." ) ); } else { - ui->mainText->setText( tr( "

License Agreement

" - "This setup procedure can install proprietary " - "software that is subject to licensing terms " - "in order to provide additional features and enhance the user " - "experience." ) ); + ui->mainText->setText( header + + tr( "This setup procedure can install proprietary " + "software that is subject to licensing terms " + "in order to provide additional features and enhance the user " + "experience." ) + + br + review ); ui->additionalText->setText( tr( "If you do not agree with the terms, proprietary software will not " "be installed, and open source alternatives will be used instead." ) ); } diff --git a/src/modules/license/LicensePage.ui b/src/modules/license/LicensePage.ui index c5857a716..13c4d94b1 100644 --- a/src/modules/license/LicensePage.ui +++ b/src/modules/license/LicensePage.ui @@ -32,16 +32,6 @@ - - - - - - - Please review the End User License Agreements (EULAs). - - - From 9fa021e3c6b5d92bfdb7a1758573ee3637489ead Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 3 Dec 2019 13:00:12 +0100 Subject: [PATCH 8/8] [license] Reduce margins hugely - Move layouting code into the .ui file - Reduce margins hugely -- atop the title block, around the scroll area, etc -- so that more license is visible at once. --- src/modules/license/LicensePage.cpp | 26 ++++++++------------------ src/modules/license/LicensePage.ui | 18 +++++++++++++++++- 2 files changed, 25 insertions(+), 19 deletions(-) diff --git a/src/modules/license/LicensePage.cpp b/src/modules/license/LicensePage.cpp index 915b396a7..fc90d843c 100644 --- a/src/modules/license/LicensePage.cpp +++ b/src/modules/license/LicensePage.cpp @@ -98,21 +98,14 @@ LicensePage::LicensePage( QWidget* parent ) { ui->setupUi( this ); - ui->verticalLayout->insertSpacing( 1, CalamaresUtils::defaultFontHeight() ); + // ui->verticalLayout->insertSpacing( 1, CalamaresUtils::defaultFontHeight() ); + CalamaresUtils::unmarginLayout( ui->verticalLayout ); - ui->mainText->setAlignment( Qt::AlignCenter ); ui->mainText->setWordWrap( true ); ui->mainText->setSizePolicy( QSizePolicy::Expanding, QSizePolicy::Minimum ); ui->additionalText->setWordWrap( true ); - ui->verticalLayout->insertSpacing( 4, CalamaresUtils::defaultFontHeight() / 2 ); - - ui->verticalLayout->setContentsMargins( CalamaresUtils::defaultFontHeight(), - CalamaresUtils::defaultFontHeight() * 3, - CalamaresUtils::defaultFontHeight(), - CalamaresUtils::defaultFontHeight() ); - ui->acceptFrame->setFrameStyle( QFrame::NoFrame | QFrame::Plain ); ui->acceptFrame->setStyleSheet( "#acceptFrame { border: 1px solid red;" "background-color: #fff6f6;" @@ -153,25 +146,22 @@ LicensePage::retranslate() { ui->acceptCheckBox->setText( tr( "I accept the terms and conditions above." ) ); - QString header = tr( "

License Agreement

" ); QString review = tr( "Please review the End User License Agreements (EULAs)." ); const auto br = QStringLiteral( "
" ); if ( !m_allLicensesOptional ) { - ui->mainText->setText( header - + tr( "This setup procedure will install proprietary " - "software that is subject to licensing terms." ) + ui->mainText->setText( tr( "This setup procedure will install proprietary " + "software that is subject to licensing terms." ) + br + review ); ui->additionalText->setText( tr( "If you do not agree with the terms, the setup procedure cannot continue." ) ); } else { - ui->mainText->setText( header - + tr( "This setup procedure can install proprietary " - "software that is subject to licensing terms " - "in order to provide additional features and enhance the user " - "experience." ) + ui->mainText->setText( tr( "This setup procedure can install proprietary " + "software that is subject to licensing terms " + "in order to provide additional features and enhance the user " + "experience." ) + br + review ); ui->additionalText->setText( tr( "If you do not agree with the terms, proprietary software will not " "be installed, and open source alternatives will be used instead." ) ); diff --git a/src/modules/license/LicensePage.ui b/src/modules/license/LicensePage.ui index 13c4d94b1..a81fae2b8 100644 --- a/src/modules/license/LicensePage.ui +++ b/src/modules/license/LicensePage.ui @@ -16,6 +16,16 @@ + + + + <h1>License Agreement</h1> + + + Qt::AlignCenter + + + @@ -30,6 +40,12 @@ <Calamares license text> + + Qt::AlignCenter + + + true + @@ -71,7 +87,7 @@ 0 0 765 - 86 + 81