Skip to content
Permalink
Browse files

Update to QgsMessageBar and duplicateLayers()

- Move project macros message into project open method (as app property, partially showed behind later messages)
- Support for multi-line messages that also wrap at with canvas window width
- Update text stylesheets to handle all text styling
- Show count of remaining queued messages
- Add popup menu to Close button with action to optionally close all messages at once
- Add slot to clear all widgets, called on project close
- Update QgisApp::duplicateLayers() to use QgsMessageBar instead of popup dialogs
  • Loading branch information
dakcarto committed Nov 6, 2012
1 parent b754f07 commit 4910276930a3c3ca684c684c653cef571a6dc2c3
Showing with 111 additions and 34 deletions.
  1. +39 −25 src/app/qgisapp.cpp
  2. +53 −8 src/gui/qgsmessagebar.cpp
  3. +19 −1 src/gui/qgsmessagebar.h
@@ -515,20 +515,6 @@ QgisApp::QgisApp( QSplashScreen *splash, bool restorePlugins, QWidget * parent,
legendLayerSelectionChanged();
activateDeactivateLayerRelatedActions( NULL );

// create the notification widget for macros
mMacrosWarn = QgsMessageBar::createMessage( tr( "Security warning:" ),
tr( "macros have been disabled." ),
QgsApplication::getThemeIcon( "/mIconWarn.png" ),
mInfoBar );

QToolButton *btnEnableMacros = new QToolButton( mMacrosWarn );
btnEnableMacros->setText( tr( "Enable" ) );
btnEnableMacros->setStyleSheet( "background-color: rgba(255, 255, 255, 0); color: black; text-decoration: underline;" );
btnEnableMacros->setCursor( Qt::PointingHandCursor );
connect( btnEnableMacros, SIGNAL( clicked() ), mInfoBar, SLOT( popWidget() ) );
connect( btnEnableMacros, SIGNAL( clicked() ), this, SLOT( enableProjectMacros() ) );
mMacrosWarn->layout()->addWidget( btnEnableMacros );

addDockWidget( Qt::LeftDockWidgetArea, mUndoWidget );
mUndoWidget->hide();

@@ -3368,8 +3354,24 @@ bool QgisApp::addProject( QString projectFile )
}
else if ( enableMacros == 1 ) // ask
{
// create the notification widget for macros

QWidget *macroMsg = QgsMessageBar::createMessage( tr( "Security warning:" ),
tr( "project macros have been disabled." ),
QgsApplication::getThemeIcon( "/mIconWarn.png" ),
mInfoBar );

QToolButton *btnEnableMacros = new QToolButton( macroMsg );
btnEnableMacros->setText( tr( "Enable macros" ) );
btnEnableMacros->setStyleSheet( "background-color: rgba(255, 255, 255, 0); color: black; text-decoration: underline;" );
btnEnableMacros->setCursor( Qt::PointingHandCursor );
btnEnableMacros->setSizePolicy( QSizePolicy::Maximum, QSizePolicy::Preferred );
connect( btnEnableMacros, SIGNAL( clicked() ), mInfoBar, SLOT( popWidget() ) );
connect( btnEnableMacros, SIGNAL( clicked() ), this, SLOT( enableProjectMacros() ) );
macroMsg->layout()->addWidget( btnEnableMacros );

// display the macros notification widget
mInfoBar->pushWidget( mMacrosWarn, 1 );
mInfoBar->pushWidget( macroMsg, 1 );
}
}
}
@@ -5386,6 +5388,7 @@ void QgisApp::duplicateLayers( QList<QgsMapLayer *> lyrList )
mMapCanvas->freeze();
QgsMapLayer *dupLayer;
QString layerDupName, unSppType;
QList<QWidget *> msgBars;

foreach ( QgsMapLayer * selectedLyr, selectedLyrs )
{
@@ -5437,19 +5440,23 @@ void QgisApp::duplicateLayers( QList<QgsMapLayer *> lyrList )

if ( unSppType.isEmpty() && dupLayer && !dupLayer->isValid() )
{
QMessageBox::information( this,
tr( "Invalid Layer" ),
tr( "%1\n\nDuplication resulted in invalid layer." ).arg( selectedLyr->name() ) );
msgBars.append( QgsMessageBar::createMessage(
tr( "Duplicate layer: " ),
tr( "%1 (duplication resulted in invalid layer)" ).arg( selectedLyr->name() ) ,
QgsApplication::getThemeIcon( "/mIconWarn.png" ),
mInfoBar ) );
continue;
}

if ( !unSppType.isEmpty() || !dupLayer )
{
QMessageBox::information( this,
tr( "Unsupported Layer" ),
tr( "%1\n%2\n\nDuplication of layer type is unsupported." )
.arg( selectedLyr->name() )
.arg( !unSppType.isEmpty() ? QString( " (" ) + unSppType + QString( ")" ) : "" ) );
msgBars.append( QgsMessageBar::createMessage(
tr( "Duplicate layer: " ),
tr( "%1 (%2type unsupported)" )
.arg( selectedLyr->name() )
.arg( !unSppType.isEmpty() ? QString( "'" ) + unSppType + "' " : "" ),
QgsApplication::getThemeIcon( "/mIconWarn.png" ),
mInfoBar ) );
continue;
}

@@ -5478,6 +5485,13 @@ void QgisApp::duplicateLayers( QList<QgsMapLayer *> lyrList )
qApp->processEvents();

mMapCanvas->freeze( false );

// display errors in message bar after duplication of layers
foreach ( QWidget * msgBar, msgBars )
{
mInfoBar->pushWidget( msgBar, 1 );
}

}

void QgisApp::setLayerCRS()
@@ -6096,8 +6110,8 @@ void QgisApp::closeProject()
QgsPythonRunner::run( "qgis.utils.unloadProjectMacros();" );
}

// remove the warning message from the bar if present
mInfoBar->popWidget( mMacrosWarn );
// remove any message widgets from the message bar
mInfoBar->clearWidgets();

mTrustedMacros = false;

@@ -24,6 +24,7 @@
#include <QLabel>
#include <QToolButton>
#include <QGridLayout>
#include <QMenu>


QgsMessageBar::QgsMessageBar( QWidget *parent )
@@ -40,16 +41,34 @@ QgsMessageBar::QgsMessageBar( QWidget *parent )
mLayout->setContentsMargins( 9, 1, 9, 1 );
setLayout( mLayout );

mLayout->addItem( new QSpacerItem( 20, 20, QSizePolicy::Expanding ), 0, 1, 1, 1 );
mItemCount = new QLabel( this );
mItemCount->setObjectName( "mItemCount" );
mItemCount->setToolTip( tr( "Remaining messages" ) );
mItemCount->setSizePolicy( QSizePolicy::Maximum, QSizePolicy::Preferred );
mLayout->addWidget( mItemCount, 0, 1, 1, 1 );

mCloseMenu = new QMenu( this );
mCloseMenu->setObjectName( "mCloseMenu" );
mActionCloseAll = new QAction( tr( "Close all" ), this );
mCloseMenu->addAction( mActionCloseAll );
connect( mActionCloseAll, SIGNAL( triggered() ), this, SLOT( clearWidgets() ) );

mCloseBtn = new QToolButton( this );
mCloseBtn->setToolTip( tr( "Close" ) );
mCloseBtn->setStyleSheet( "QToolButton {background-color: rgba(255, 255, 255, 0);}" );
mCloseBtn->setMinimumWidth( 36 );
mCloseBtn->setStyleSheet(
"QToolButton { background-color: rgba(255, 255, 255, 0); } "
"QToolButton::menu-indicator { subcontrol-position: right bottom; subcontrol-origin: padding; bottom: 6px; }" );
mCloseBtn->setCursor( Qt::PointingHandCursor );
mCloseBtn->setIcon( QgsApplication::getThemeIcon( "/mIconClose.png" ) );
mCloseBtn->setSizePolicy( QSizePolicy::Maximum, QSizePolicy::Preferred );
mCloseBtn->setMenu( mCloseMenu );
connect( mCloseBtn, SIGNAL( clicked() ), this, SLOT( popWidget() ) );
mLayout->addWidget( mCloseBtn, 0, 2, 1, 1 );

connect( this, SIGNAL( widgetAdded( QWidget* ) ), this, SLOT( updateItemCount() ) );
connect( this, SIGNAL( widgetRemoved( QWidget* ) ), this, SLOT( updateItemCount() ) );

// start hidden
setVisible( false );
}
@@ -127,6 +146,20 @@ bool QgsMessageBar::popWidget()
return true;
}

bool QgsMessageBar::clearWidgets()
{
if ( !mCurrentItem && mList.empty() )
return true;

while ( mList.count() > 0 )
{
popWidget();
}
popWidget();

return !mCurrentItem && mList.empty();
}

void QgsMessageBar::pushItem( QgsMessageBarItem *item )
{
Q_ASSERT( item );
@@ -150,23 +183,29 @@ void QgsMessageBar::pushItem( QgsMessageBarItem *item )

setStyleSheet( item->styleSheet() );
show();

emit widgetAdded( item->widget() );
}

void QgsMessageBar::pushWidget( QWidget *widget, int level )
{
QString stylesheet;
if ( level >= 2 )
{
stylesheet = "QgsMessageBar { background-color: #d65253; border: 1px solid #9b3d3d; } QLabel { color: white; }";
stylesheet = "QgsMessageBar { background-color: #d65253; border: 1px solid #9b3d3d; } "
"QLabel#mMsgTitle, QLabel#mMsgText, QLabel#mItemCount { color: white; } ";
}
else if ( level == 1 )
{
stylesheet = "QgsMessageBar { background-color: #ffc800; border: 1px solid #e0aa00; } QLabel { color: black; }";
stylesheet = "QgsMessageBar { background-color: #ffc800; border: 1px solid #e0aa00; } "
"QLabel#mMsgTitle, QLabel#mMsgText, QLabel#mItemCount { color: black; } ";
}
else if ( level <= 0 )
{
stylesheet = "QgsMessageBar { background-color: #e7f5fe; border: 1px solid #b9cfe4; } QLabel { color: #2554a1; }";
stylesheet = "QgsMessageBar { background-color: #e7f5fe; border: 1px solid #b9cfe4; } "
"QLabel#mMsgTitle, QLabel#mMsgText, QLabel#mItemCount { color: #2554a1; } ";
}
stylesheet += "QLabel#mMsgTitle { font-weight: bold; } QLabel#mItemCount { font-style: italic; }";
pushWidget( widget, stylesheet );
}

@@ -198,14 +237,20 @@ QWidget* QgsMessageBar::createMessage( const QString &title, const QString &text
if ( !title.isEmpty() )
{
QLabel *lblTitle = new QLabel( title, widget );
QFont font = lblTitle->font();
font.setBold( true );
lblTitle->setFont( font );
lblTitle->setObjectName( "mMsgTitle" );
layout->addWidget( lblTitle );
}

QLabel *lblText = new QLabel( text, widget );
lblText->setObjectName( "mMsgText" );
lblText->setSizePolicy( QSizePolicy::MinimumExpanding, QSizePolicy::Preferred );
lblText->setWordWrap( true );
layout->addWidget( lblText );

return widget;
}

void QgsMessageBar::updateItemCount()
{
mItemCount->setText( mList.count() > 0 ? QString::number( mList.count() ) + QString( " " ) + tr( "more" ) : QString( "" ) );
}
@@ -27,7 +27,10 @@

class QWidget;
class QGridLayout;
class QMenu;
class QToolButton;
class QLabel;
class QAction;

/** \ingroup gui
* A bar for displaying non-blocking messages to the user.
@@ -65,7 +68,10 @@ class GUI_EXPORT QgsMessageBar: public QFrame
static QWidget* createMessage( const QString &title, const QString &text, const QIcon &icon, QWidget *parent = 0 );

signals:
//! emitted when a widget was removed from the bar
//! emitted when a message widget is added to the bar
void widgetAdded( QWidget *widget );

//! emitted when a message widget was removed from the bar
void widgetRemoved( QWidget *widget );

public slots:
@@ -75,6 +81,11 @@ class GUI_EXPORT QgsMessageBar: public QFrame
*/
bool popWidget();

/*! remove all items from the bar's widget list
* @return true if all items were removed, false otherwise
*/
bool clearWidgets();

private:
class QgsMessageBarItem
{
@@ -99,8 +110,15 @@ class GUI_EXPORT QgsMessageBar: public QFrame

QgsMessageBarItem *mCurrentItem;
QList<QgsMessageBarItem *> mList;
QMenu *mCloseMenu;
QToolButton *mCloseBtn;
QGridLayout *mLayout;
QLabel *mItemCount;
QAction *mActionCloseAll;

private slots:
//! updates count of items in widget list
void updateItemCount();
};

#endif

3 comments on commit 4910276

@brushtyler

This comment has been minimized.

Copy link
Contributor

@brushtyler brushtyler replied Nov 7, 2012

Hi Larry,
next time you need a review please open a pull request.

I'm writing my opinions inline.

  • Move project macros message into project open method
  • Add slot to clear all widgets, called on project close

Those changes looks good to me, though I guess that the message bar could be used not only for messages related to projects, so clear the bar when the project is closed couldn't be the right thing to do.

IMO we should add a parameter to define messages context: whether related to the project or to the application instance or maybe something else (to be defined).
Before your change the problem wasn't there (the project macro message was accessible from the project close method), but I agree that your change goes to the right direction.

  • Support for multi-line messages that also wrap at with canvas window width
  • Show count of remaining queued messages
  • Add popup menu to Close button with action to optionally close all messages at once

I really like those changes! I'm building everything, I'll tell you.
I would also add two buttons to switch to the prev/next message in the list.

  • Update text stylesheets to handle all text styling

I would keep the QLabel { color: ... } part so labels within any pushed widget (even custom ones) have that text color: the message level changes the background color and this could make dark/bright texts rather invisible on a dark/bright background.
If the developer want to override the color of a particular label he can still specify the new one using QLabel#objectName as CSS selector.

  • Update QgisApp::duplicateLayers() to use QgsMessageBar instead of popup dialogs

I don't like to add a message to the bar for each layer that fails, ok it's not a messagebox, but anyway the user has to close them.
BTW this is not related to the review of the code, it's just my opinion ;)

@dakcarto

This comment has been minimized.

Copy link
Member Author

@dakcarto dakcarto replied Nov 7, 2012

Hi Larry,
next time you need a review please open a pull request.

I understanding your reasoning here, but if the core commit is non-disruptive, moves the code forward in a beneficial manner, fixes an issue and is heavily tested, I don't see the difference (since we have github's commenting system) other than having to wait for all discussion to end before I could continue with using the commit in another dev branch (which I am currently doing).

If I am unsure about the implementation, think the code may disrupt or break something else I do not fully understand, or the changes would affect or is part of code someone else is currently working on, I think a pull request or sharing a dev branch off of my forked repo would then be in order.

I'm writing my opinions inline.

  • Move project macros message into project open method
  • Add slot to clear all widgets, called on project close

Those changes looks good to me, though I guess that the message bar could be used not only for messages related to projects, so clear the bar when the project is closed couldn't be the right thing to do.

What would a message be that would need to persist between project openings? Wouldn't all pre-project opening messages (like ones that might occur during app launch) be handled on the first empty project, or in the log? Also, couldn't any app-level messages just be generated again (if needed) on project open? Not sure I'm following the use case here for not clearing the bar on project close.

IMO we should add a parameter to define messages context: whether related to the project or to the application instance or maybe something else (to be defined).
Before your change the problem wasn't there (the project macro message was accessible from the project close method), but I agree that your change goes to the right direction.

Yes. That sounds good. Kind of like how syslog messages are keyed off of the 'facility' context, except the message bar is more of a temporal notification system, than a log. Not sure the best way to show that context in the bar. Maybe a label just before the message icon that reads: App, Project, or whatever? Or maybe an icon?

  • Support for multi-line messages that also wrap at with canvas window width
  • Show count of remaining queued messages
  • Add popup menu to Close button with action to optionally close all messages at once

I really like those changes! I'm building everything, I'll tell you.
I would also add two buttons to switch to the prev/next message in the list.

I tried a version of prev/next, but found the implementation of CurrentItem property (which is separate from mList) problematic for keeping track of current message number (e.g. for 'message 3 of 5' type counter ). Was going to try again after this commit.

  • Update text stylesheets to handle all text styling

I would keep the QLabel { color: ... } part so labels within any pushed widget (even custom ones) have that text color: the message level changes the background color and this could make dark/bright texts rather invisible on a dark/bright background.
If the developer want to override the color of a particular label he can still specify the new one using QLabel#objectName as CSS selector.

Makes sense. I'll make and test the change.

  • Update QgisApp::duplicateLayers() to use QgsMessageBar instead of popup dialogs

I don't like to add a message to the bar for each layer that fails, ok it's not a messagebox, but anyway the user has to close them.
BTW this is not related to the review of the code, it's just my opinion ;)

I could look at batching the message lines into grouped messages, e.g. grouping lists with a max line count of 5, since the message lines should never really need to wrap. Thanks.

@brushtyler

This comment has been minimized.

Copy link
Contributor

@brushtyler brushtyler replied Nov 7, 2012

if the core commit is non-disruptive, moves the code forward in a beneficial manner, fixes an issue and is heavily tested, I don't see the difference

ok, I agree.

Not sure I'm following the use case here for not clearing the bar on project close.
...
Not sure the best way to show that context in the bar. Maybe a label just before the message icon that reads: App, Project, or whatever? Or maybe an icon?

I've no use cases right now, it was just an idea. But I guess that defining a context for messages would be a good thing.
Anyway it's something can be added in the future.

I could look at batching the message lines into grouped messages, e.g. grouping lists with a max line count of 5

BTW after implementing the duplicateLayer also for the other providers it'll fail very few times.

Please sign in to comment.
You can’t perform that action at this time.