Skip to content

Commit

Permalink
Update to QgsMessageBar and duplicateLayers()
Browse files Browse the repository at this point in the history 
- 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
dakcarto committed Nov 6, 2012
1 parent b754f07 commit 4910276
Show file tree
Hide file tree
Showing 3 changed files with 111 additions and 34 deletions.
64 changes: 39 additions & 25 deletions src/app/qgisapp.cpp
View file
Expand Up @@ -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();

Expand Down Expand Up @@ -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 );
}
}
}
Expand Down Expand Up @@ -5386,6 +5388,7 @@ void QgisApp::duplicateLayers( QList<QgsMapLayer *> lyrList )
mMapCanvas->freeze();
QgsMapLayer *dupLayer;
QString layerDupName, unSppType;
QList<QWidget *> msgBars;

foreach ( QgsMapLayer * selectedLyr, selectedLyrs )
{
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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;

Expand Down
61 changes: 53 additions & 8 deletions src/gui/qgsmessagebar.cpp
View file
Expand Up @@ -24,6 +24,7 @@
#include <QLabel>
#include <QToolButton>
#include <QGridLayout>
#include <QMenu>


QgsMessageBar::QgsMessageBar( QWidget *parent )
Expand All @@ -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 );
}
Expand Down Expand Up @@ -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 );
Expand All @@ -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 );
}

Expand Down Expand Up @@ -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( "" ) );
}
20 changes: 19 additions & 1 deletion src/gui/qgsmessagebar.h
View file
Expand Up @@ -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.
Expand Down Expand Up @@ -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:
Expand All @@ -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
{
Expand All @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.