From 6e1504fafc947aa7ca53e75bafd60577352e05ef Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 9 Dec 2019 14:52:54 +0100 Subject: [PATCH 1/6] [license] Use just one button - replace the text plus toolbutton (which has an ambiguous arrow in it) by a single button with text saying what it will do. --- src/modules/license/LicenseWidget.cpp | 62 ++++++--------------------- src/modules/license/LicenseWidget.h | 6 +-- 2 files changed, 16 insertions(+), 52 deletions(-) diff --git a/src/modules/license/LicenseWidget.cpp b/src/modules/license/LicenseWidget.cpp index a4c1dfaaa..0d4190d57 100644 --- a/src/modules/license/LicenseWidget.cpp +++ b/src/modules/license/LicenseWidget.cpp @@ -27,13 +27,9 @@ #include #include #include -#include +#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 ) { @@ -56,9 +52,9 @@ LicenseWidget::LicenseWidget( LicenseEntry entry, QWidget* parent ) : QWidget( parent ) , m_entry( std::move( entry ) ) , m_label( new QLabel( this ) ) - , m_viewLicenseLabel( new QLabel( this ) ) - , m_expandLicenseButton( nullptr ) + , m_viewLicenseButton( new QPushButton( this ) ) , m_fullText( nullptr ) + , m_isExpanded( m_entry.expandByDefault() ) { QPalette pal( palette() ); pal.setColor( QPalette::Background, palette().window().color().lighter( 108 ) ); @@ -76,46 +72,28 @@ LicenseWidget::LicenseWidget( LicenseEntry entry, QWidget* parent ) m_label->setSizePolicy( QSizePolicy::Preferred, QSizePolicy::Minimum ); wiLayout->addWidget( m_label ); - m_viewLicenseLabel->setSizePolicy( QSizePolicy::Preferred, QSizePolicy::Preferred ); - m_viewLicenseLabel->setAlignment( Qt::AlignVCenter | Qt::AlignRight ); - wiLayout->addWidget( m_viewLicenseLabel ); + m_viewLicenseButton->setSizePolicy( QSizePolicy::Preferred, QSizePolicy::Minimum ); + wiLayout->addWidget( m_viewLicenseButton ); - m_expandLicenseButton = new QToolButton( this ); - wiLayout->addWidget( m_expandLicenseButton ); if ( m_entry.isLocal() ) { QVBoxLayout* vLayout = new QVBoxLayout; - - m_expandLicenseButton->setArrowType( ArrowLocalLicenseIsCollapsed ); - connect( m_expandLicenseButton, &QAbstractButton::clicked, this, &LicenseWidget::expandClicked ); - vLayout->addLayout( wiLayout ); m_fullText = new QLabel( this ); m_fullText->setText( loadLocalFile( m_entry.m_url ) ); - m_fullText->hide(); m_fullText->setStyleSheet( "border-top: 1px solid black; margin-top: 1em; padding-top: 1em;" ); m_fullText->setObjectName( "licenseItemFullText" ); + m_fullText->setHidden( !m_isExpanded ); + 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(); - } + connect( m_viewLicenseButton, &QAbstractButton::clicked, this, &LicenseWidget::expandClicked ); } else { - m_expandLicenseButton->setArrowType( ArrowOpenExternalLink ); - connect( m_expandLicenseButton, &QAbstractButton::clicked, this, &LicenseWidget::viewClicked ); - - // Normally setOpenExternalLinks( true ) would do, but we need the - // open code anyway for the toolbutton, let's share it. - connect( m_viewLicenseLabel, &QLabel::linkActivated, this, &LicenseWidget::viewClicked ); - setLayout( wiLayout ); // Only the horizontal layout needed + connect( m_viewLicenseButton, &QAbstractButton::clicked, this, &LicenseWidget::viewClicked ); } retranslateUi(); @@ -174,19 +152,11 @@ LicenseWidget::retranslateUi() void LicenseWidget::expandClicked() { - if ( m_expandLicenseButton->arrowType() == ArrowLocalLicenseIsExpanded ) - { - m_expandLicenseButton->setArrowType( ArrowLocalLicenseIsCollapsed ); - } - else - { - m_expandLicenseButton->setArrowType( ArrowLocalLicenseIsExpanded ); - } - + m_isExpanded = !m_isExpanded; // Show/hide based on the new arrow direction. if ( m_fullText ) { - m_fullText->setHidden( m_expandLicenseButton->arrowType() == ArrowLocalLicenseIsCollapsed ); + m_fullText->setHidden( !m_isExpanded ); } updateExpandToolTip(); @@ -198,17 +168,11 @@ LicenseWidget::updateExpandToolTip() { if ( m_entry.isLocal() ) { - const bool isNowCollapsed = m_expandLicenseButton->arrowType() == ArrowLocalLicenseIsCollapsed; - - m_expandLicenseButton->setToolTip( isNowCollapsed ? tr( "Shows the complete license text" ) - : tr( "Hide license text" ) ); - m_viewLicenseLabel->setText( isNowCollapsed ? tr( "Show license agreement" ) : tr( "Hide license agreement" ) ); + m_viewLicenseButton->setText( m_isExpanded ? tr( "Hide license text" ) : tr( "Show the license text" ) ); } else { - m_expandLicenseButton->setToolTip( tr( "Opens the license agreement in a browser window." ) ); - m_viewLicenseLabel->setText( - tr( "View license agreement" ).arg( m_entry.m_url.toString() ) ); + m_viewLicenseButton->setText( tr( "Open license agreement in browser." ) ); } } diff --git a/src/modules/license/LicenseWidget.h b/src/modules/license/LicenseWidget.h index e11d7a746..600516d6d 100644 --- a/src/modules/license/LicenseWidget.h +++ b/src/modules/license/LicenseWidget.h @@ -27,7 +27,7 @@ #include #include -class QToolButton; +class QPushButton; class LicenseWidget : public QWidget { @@ -44,8 +44,8 @@ private: LicenseEntry m_entry; QLabel* m_label; - QLabel* m_viewLicenseLabel; - QToolButton* m_expandLicenseButton; + QPushButton* m_viewLicenseButton; QLabel* m_fullText; + bool m_isExpanded; }; #endif From 7330afd96ad90d4cc0999aca7a220ed5aa9bb614 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 9 Dec 2019 16:49:43 +0100 Subject: [PATCH 2/6] [license] Massage display of buttons - try to keep them the same height - show the URL that will be opened --- src/modules/license/LicenseWidget.cpp | 48 +++++++++++++++++---------- src/modules/license/LicenseWidget.h | 3 +- 2 files changed, 33 insertions(+), 18 deletions(-) diff --git a/src/modules/license/LicenseWidget.cpp b/src/modules/license/LicenseWidget.cpp index 0d4190d57..0d6ef76d6 100644 --- a/src/modules/license/LicenseWidget.cpp +++ b/src/modules/license/LicenseWidget.cpp @@ -53,7 +53,7 @@ LicenseWidget::LicenseWidget( LicenseEntry entry, QWidget* parent ) , m_entry( std::move( entry ) ) , m_label( new QLabel( this ) ) , m_viewLicenseButton( new QPushButton( this ) ) - , m_fullText( nullptr ) + , m_licenceTextLabel( new QLabel( this ) ) , m_isExpanded( m_entry.expandByDefault() ) { QPalette pal( palette() ); @@ -69,33 +69,40 @@ LicenseWidget::LicenseWidget( LicenseEntry entry, QWidget* parent ) QHBoxLayout* wiLayout = new QHBoxLayout; m_label->setWordWrap( true ); - m_label->setSizePolicy( QSizePolicy::Preferred, QSizePolicy::Minimum ); + m_label->setSizePolicy( QSizePolicy::Minimum, QSizePolicy::Minimum ); wiLayout->addWidget( m_label ); - m_viewLicenseButton->setSizePolicy( QSizePolicy::Preferred, QSizePolicy::Minimum ); + wiLayout->addSpacing( 1 ); + m_viewLicenseButton->setSizePolicy( QSizePolicy::Minimum, QSizePolicy::Minimum ); wiLayout->addWidget( m_viewLicenseButton ); + QVBoxLayout* vLayout = new QVBoxLayout; + vLayout->addLayout( wiLayout ); + m_licenceTextLabel->setStyleSheet( "border-top: 1px solid black; margin-top: 0px; padding-top: 1em;" ); + m_licenceTextLabel->setObjectName( "licenseItemFullText" ); + if ( m_entry.isLocal() ) { - QVBoxLayout* vLayout = new QVBoxLayout; - vLayout->addLayout( wiLayout ); - m_fullText = new QLabel( this ); - m_fullText->setText( loadLocalFile( m_entry.m_url ) ); - m_fullText->setStyleSheet( "border-top: 1px solid black; margin-top: 1em; padding-top: 1em;" ); - m_fullText->setObjectName( "licenseItemFullText" ); - - m_fullText->setHidden( !m_isExpanded ); - - vLayout->addWidget( m_fullText ); - setLayout( vLayout ); + m_fullTextContents = loadLocalFile( m_entry.m_url ); + if ( m_isExpanded ) + { + m_licenceTextLabel->setText( m_fullTextContents ); + } + else + { + m_licenceTextLabel->setText( tr( "URL: %1" ).arg( m_entry.m_url.toDisplayString() ) ); + } connect( m_viewLicenseButton, &QAbstractButton::clicked, this, &LicenseWidget::expandClicked ); } else { - setLayout( wiLayout ); // Only the horizontal layout needed + m_licenceTextLabel->setText( tr( "URL: %1" ).arg( m_entry.m_url.toDisplayString() ) ); connect( m_viewLicenseButton, &QAbstractButton::clicked, this, &LicenseWidget::viewClicked ); } + vLayout->addWidget( m_licenceTextLabel ); + setLayout( vLayout ); + retranslateUi(); } @@ -154,9 +161,16 @@ LicenseWidget::expandClicked() { m_isExpanded = !m_isExpanded; // Show/hide based on the new arrow direction. - if ( m_fullText ) + if ( !m_fullTextContents.isEmpty() ) { - m_fullText->setHidden( !m_isExpanded ); + if ( m_isExpanded ) + { + m_licenceTextLabel->setText( m_fullTextContents ); + } + else + { + m_licenceTextLabel->setText( tr( "URL: %1" ).arg( m_entry.m_url.toDisplayString() ) ); + } } updateExpandToolTip(); diff --git a/src/modules/license/LicenseWidget.h b/src/modules/license/LicenseWidget.h index 600516d6d..ffbb9d3c3 100644 --- a/src/modules/license/LicenseWidget.h +++ b/src/modules/license/LicenseWidget.h @@ -45,7 +45,8 @@ private: LicenseEntry m_entry; QLabel* m_label; QPushButton* m_viewLicenseButton; - QLabel* m_fullText; + QLabel* m_licenceTextLabel; + QString m_fullTextContents; bool m_isExpanded; }; #endif From 8a912e6ca1ead0b47c70ed3d9a34356151a05aaa Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 9 Dec 2019 17:08:35 +0100 Subject: [PATCH 3/6] [license] Fix the height of each item - needs a qwidget to put the top-items (license name, button) in - fixes issue where the gap between the button and the hrule would change depending on what is expanded --- src/modules/license/LicenseWidget.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/modules/license/LicenseWidget.cpp b/src/modules/license/LicenseWidget.cpp index 0d6ef76d6..4fb338d46 100644 --- a/src/modules/license/LicenseWidget.cpp +++ b/src/modules/license/LicenseWidget.cpp @@ -66,18 +66,21 @@ LicenseWidget::LicenseWidget( LicenseEntry entry, QWidget* parent ) setSizePolicy( QSizePolicy::Expanding, QSizePolicy::Minimum ); setContentsMargins( 4, 4, 4, 4 ); + QVBoxLayout* vLayout = new QVBoxLayout; + QWidget* topPart = new QWidget( this ); + vLayout->addWidget( topPart ); + QHBoxLayout* wiLayout = new QHBoxLayout; + topPart->setLayout( wiLayout ); m_label->setWordWrap( true ); m_label->setSizePolicy( QSizePolicy::Minimum, QSizePolicy::Minimum ); wiLayout->addWidget( m_label ); - wiLayout->addSpacing( 1 ); - m_viewLicenseButton->setSizePolicy( QSizePolicy::Minimum, QSizePolicy::Minimum ); + wiLayout->addSpacing( 10 ); + m_viewLicenseButton->setSizePolicy( QSizePolicy::Minimum, QSizePolicy::Fixed ); wiLayout->addWidget( m_viewLicenseButton ); - QVBoxLayout* vLayout = new QVBoxLayout; - vLayout->addLayout( wiLayout ); m_licenceTextLabel->setStyleSheet( "border-top: 1px solid black; margin-top: 0px; padding-top: 1em;" ); m_licenceTextLabel->setObjectName( "licenseItemFullText" ); From c22022056369cee5c91804684c55d069420ca154 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 9 Dec 2019 17:14:35 +0100 Subject: [PATCH 4/6] [license] Give stylesheets meaningful names --- src/modules/license/LicensePage.cpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/modules/license/LicensePage.cpp b/src/modules/license/LicensePage.cpp index fc90d843c..8f6f700ca 100644 --- a/src/modules/license/LicensePage.cpp +++ b/src/modules/license/LicensePage.cpp @@ -44,6 +44,12 @@ #include +static const char mustAccept[] = "#acceptFrame { border: 1px solid red;" + "background-color: #fff6f6;" + "border-radius: 4px;" + "padding: 2px; }"; +static const char okAccept[] = "#acceptFrame { padding: 3px }"; + const NamedEnumTable< LicenseEntry::Type >& LicenseEntry::typeNames() { @@ -107,10 +113,7 @@ LicensePage::LicensePage( QWidget* parent ) ui->additionalText->setWordWrap( true ); ui->acceptFrame->setFrameStyle( QFrame::NoFrame | QFrame::Plain ); - ui->acceptFrame->setStyleSheet( "#acceptFrame { border: 1px solid red;" - "background-color: #fff6f6;" - "border-radius: 4px;" - "padding: 2px; }" ); + ui->acceptFrame->setStyleSheet( mustAccept ); ui->acceptFrame->layout()->setMargin( CalamaresUtils::defaultFontHeight() / 2 ); updateGlobalStorage( false ); // Have not agreed yet @@ -195,14 +198,11 @@ LicensePage::checkAcceptance( bool checked ) m_isNextEnabled = checked || m_allLicensesOptional; if ( !m_isNextEnabled ) { - ui->acceptFrame->setStyleSheet( "#acceptFrame { border: 1px solid red;" - "background-color: #fff8f8;" - "border-radius: 4px;" - "padding: 2px; }" ); + ui->acceptFrame->setStyleSheet( mustAccept ); } else { - ui->acceptFrame->setStyleSheet( "#acceptFrame { padding: 3px }" ); + ui->acceptFrame->setStyleSheet( okAccept ); } emit nextStatusChanged( m_isNextEnabled ); } From 59ea0417fc693adb45c4a4d72dc46a467f834f64 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 9 Dec 2019 17:25:07 +0100 Subject: [PATCH 5/6] [license] Move a less-useful message to tooltip - the message about setup continuing can be a tooltip --- src/modules/license/LicensePage.cpp | 10 +++++----- src/modules/license/LicensePage.ui | 20 ++------------------ 2 files changed, 7 insertions(+), 23 deletions(-) diff --git a/src/modules/license/LicensePage.cpp b/src/modules/license/LicensePage.cpp index 8f6f700ca..526820920 100644 --- a/src/modules/license/LicensePage.cpp +++ b/src/modules/license/LicensePage.cpp @@ -110,8 +110,6 @@ LicensePage::LicensePage( QWidget* parent ) ui->mainText->setWordWrap( true ); ui->mainText->setSizePolicy( QSizePolicy::Expanding, QSizePolicy::Minimum ); - ui->additionalText->setWordWrap( true ); - ui->acceptFrame->setFrameStyle( QFrame::NoFrame | QFrame::Plain ); ui->acceptFrame->setStyleSheet( mustAccept ); ui->acceptFrame->layout()->setMargin( CalamaresUtils::defaultFontHeight() / 2 ); @@ -157,7 +155,8 @@ LicensePage::retranslate() 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." ) ); + QString mustAcceptText( tr( "If you do not agree with the terms, the setup procedure cannot continue." ) ); + ui->acceptCheckBox->setToolTip( mustAcceptText ); } else { @@ -166,8 +165,9 @@ LicensePage::retranslate() "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." ) ); + QString okAcceptText( tr( "If you do not agree with the terms, proprietary software will not " + "be installed, and open source alternatives will be used instead." ) ); + ui->acceptCheckBox->setToolTip( okAcceptText ); } ui->retranslateUi( this ); diff --git a/src/modules/license/LicensePage.ui b/src/modules/license/LicensePage.ui index a81fae2b8..b5936d5de 100644 --- a/src/modules/license/LicensePage.ui +++ b/src/modules/license/LicensePage.ui @@ -15,7 +15,7 @@ - + @@ -87,7 +87,7 @@ 0 0 765 - 81 + 89 @@ -107,22 +107,6 @@ - - - - - 0 - 0 - - - - - - - <additionalText> - - - From ac1c0d97d2027507517c6e49704253d3b419ca7e Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 9 Dec 2019 17:34:46 +0100 Subject: [PATCH 6/6] [license] Prevent single item from expanding - the last item would expand vertically to fill the scroll area; add a spacer that pushes against it --- src/modules/license/LicensePage.cpp | 1 + src/modules/license/LicenseWidget.cpp | 2 ++ 2 files changed, 3 insertions(+) diff --git a/src/modules/license/LicensePage.cpp b/src/modules/license/LicensePage.cpp index 526820920..237e8d0dc 100644 --- a/src/modules/license/LicensePage.cpp +++ b/src/modules/license/LicensePage.cpp @@ -137,6 +137,7 @@ LicensePage::setEntries( const QList< LicenseEntry >& entriesList ) m_entries.append( w ); m_allLicensesOptional &= !entry.isRequired(); } + ui->licenseEntriesLayout->addSpacerItem( new QSpacerItem( 10, 10, QSizePolicy::Minimum, QSizePolicy::Expanding ) ); ui->acceptCheckBox->setChecked( false ); checkAcceptance( false ); diff --git a/src/modules/license/LicenseWidget.cpp b/src/modules/license/LicenseWidget.cpp index 4fb338d46..b78972c00 100644 --- a/src/modules/license/LicenseWidget.cpp +++ b/src/modules/license/LicenseWidget.cpp @@ -68,6 +68,7 @@ LicenseWidget::LicenseWidget( LicenseEntry entry, QWidget* parent ) QVBoxLayout* vLayout = new QVBoxLayout; QWidget* topPart = new QWidget( this ); + topPart->setSizePolicy( QSizePolicy::Minimum, QSizePolicy::Minimum ); vLayout->addWidget( topPart ); QHBoxLayout* wiLayout = new QHBoxLayout; @@ -102,6 +103,7 @@ LicenseWidget::LicenseWidget( LicenseEntry entry, QWidget* parent ) m_licenceTextLabel->setText( tr( "URL: %1" ).arg( m_entry.m_url.toDisplayString() ) ); connect( m_viewLicenseButton, &QAbstractButton::clicked, this, &LicenseWidget::viewClicked ); } + m_licenceTextLabel->setSizePolicy( QSizePolicy::Minimum, QSizePolicy::Minimum ); vLayout->addWidget( m_licenceTextLabel ); setLayout( vLayout );