Gui: support dragging of toolbar inside status/menu bar #13973
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,10 +29,12 @@ | |
| # include <QHBoxLayout> | ||
| # include <QMenuBar> | ||
| # include <QMouseEvent> | ||
| # include <QPainter> | ||
| # include <QPointer> | ||
| # include <QStatusBar> | ||
| # include <QToolBar> | ||
| # include <QToolButton> | ||
| # include <QStyleOption> | ||
| #endif | ||
|
|
||
| #include <boost/algorithm/string/predicate.hpp> | ||
|
|
@@ -188,6 +190,7 @@ | |
| { | ||
| if (_layout->indexOf(w) < 0) { | ||
| _layout->addWidget(w); | ||
| ToolBarManager::getInstance()->setToolBarMovable(qobject_cast<QToolBar*>(w)); | ||
| adjustParent(); | ||
| QString name = w->objectName(); | ||
| if (!name.isEmpty()) { | ||
|
|
@@ -207,6 +210,7 @@ | |
| _layout->removeWidget(w); | ||
| } | ||
| _layout->insertWidget(idx, w); | ||
| ToolBarManager::getInstance()->setToolBarMovable(qobject_cast<QToolBar*>(w)); | ||
| adjustParent(); | ||
| saveState(); | ||
| } | ||
|
|
@@ -221,6 +225,12 @@ | |
| void removeWidget(QWidget *w) | ||
| { | ||
| _layout->removeWidget(w); | ||
| if (auto tb = qobject_cast<QToolBar*>(w)) { | ||
| if (auto grip = tb->findChild<ToolBarGrip*>()) { | ||
| tb->removeAction(grip->action()); | ||
| grip->deleteLater(); | ||
| } | ||
| } | ||
| QString name = w->objectName(); | ||
| if (!name.isEmpty()) { | ||
| Base::ConnectionBlocker block(_conn); | ||
|
|
@@ -294,10 +304,137 @@ | |
| boost::signals2::scoped_connection &_conn; | ||
| }; | ||
|
|
||
| class ToolBar: public QToolBar | ||
| { | ||
| public: | ||
| void initStyleOption(QStyleOptionToolBar *option) const | ||
| { | ||
| QToolBar::initStyleOption(option); | ||
| } | ||
| }; | ||
|
|
||
| } // namespace Gui | ||
|
|
||
| // ----------------------------------------------------------- | ||
|
|
||
| ToolBarGrip::ToolBarGrip(QToolBar * parent) | ||
| :QWidget(parent) | ||
| { | ||
| QStyle *style = parent->style(); | ||
| QStyleOptionToolBar opt; | ||
| static_cast<ToolBar*>(parent)->initStyleOption(&opt); | ||
| opt.features = QStyleOptionToolBar::Movable; | ||
| int width = style->subElementRect(QStyle::SE_ToolBarHandle, &opt, parent).width(); | ||
| this->setFixedWidth(width+4); | ||
|
Comment on lines
+327
to
+328
There was a problem hiding this comment. I suppose that we simply don't support vertical toolbars - It would be nice to state it in the comment though |
||
|
|
||
| setCursor(Qt::OpenHandCursor); | ||
| setMouseTracking(true); | ||
| auto actions = parent->actions(); | ||
| _action = parent->insertWidget(actions.isEmpty()?nullptr:actions[0], this); | ||
| setVisible(true); | ||
| } | ||
|
|
||
| QAction *ToolBarGrip::action() const | ||
| { | ||
| return _action; | ||
| } | ||
|
|
||
| void ToolBarGrip::paintEvent(QPaintEvent*) | ||
| { | ||
| QPainter painter(this); | ||
| if (auto toolbar = qobject_cast<QToolBar*>(parentWidget())) { | ||
|
There was a problem hiding this comment. Why not |
||
| QStyle *style = toolbar->style(); | ||
| QStyleOptionToolBar opt; | ||
| static_cast<ToolBar*>(toolbar)->initStyleOption(&opt); | ||
| opt.features = QStyleOptionToolBar::Movable; | ||
| // opt.rect = style->subElementRect(QStyle::SE_ToolBarHandle, &opt, toolbar); | ||
|
There was a problem hiding this comment. Please remove this commented code to not confuse newcomers. |
||
| opt.rect = this->rect(); | ||
| style->drawPrimitive(QStyle::PE_IndicatorToolBarHandle, &opt, &painter, toolbar); | ||
| } | ||
| else { | ||
| painter.setPen(Qt::transparent); | ||
| painter.setOpacity(0.5); | ||
| painter.setBrush(QBrush(Qt::black, Qt::Dense6Pattern)); | ||
| QRect rect(this->rect()); | ||
| painter.drawRect(rect); | ||
|
Comment on lines
+355
to
+359
There was a problem hiding this comment. Why is drawing of this widget different when it is not in the custom toolbar area? |
||
| } | ||
| } | ||
|
|
||
| void ToolBarGrip::mouseMoveEvent(QMouseEvent *me) | ||
| { | ||
| auto toolbar = qobject_cast<QToolBar*>(parentWidget()); | ||
| if (!toolbar) { | ||
| return; | ||
| } | ||
| auto area = ToolBarManager::getInstance()->getToolBarArea(toolbar); | ||
| if (!area) | ||
| return; | ||
|
|
||
| QPoint pos = me->globalPos(); | ||
| QRect rect(toolbar->mapToGlobal(QPoint(0,0)), toolbar->size()); | ||
| if (rect.contains(pos)) | ||
| return; | ||
|
|
||
| // If mouse is out of the toolbar, remove the toolbar from area and float the | ||
| // toolbar | ||
| { | ||
| QSignalBlocker blocker(toolbar); | ||
| area->removeWidget(toolbar); | ||
| getMainWindow()->addToolBar(toolbar); | ||
| toolbar->setWindowFlags(Qt::Tool | ||
| | Qt::FramelessWindowHint | ||
| | Qt::X11BypassWindowManagerHint); | ||
| toolbar->adjustSize(); | ||
| pos = toolbar->mapFromGlobal(pos); | ||
| toolbar->move(pos.x()-10, pos.y()-10); | ||
| toolbar->setVisible(true); | ||
| } | ||
| toolbar->topLevelChanged(true); | ||
|
Comment on lines
+380
to
+392
There was a problem hiding this comment. Since you already have custom There was a problem hiding this comment. While you don't have to mark signals with explicit |
||
|
|
||
| // After removing from area, this grip will be deleted. In order to | ||
| // continue toolbar dragging (because the mouse button is still pressed), | ||
| // we fake mouse events and send to toolbar. For some reason, | ||
| // send/postEvent() does not work, only timer works. | ||
| QPointer tb(toolbar); | ||
| QTimer::singleShot(0, [tb] { | ||
| auto modifiers = QApplication::queryKeyboardModifiers(); | ||
| auto buttons = QApplication::mouseButtons(); | ||
| if (buttons != Qt::LeftButton | ||
| || QWidget::mouseGrabber() | ||
| || modifiers != Qt::NoModifier | ||
| || !tb) { | ||
| return; | ||
| } | ||
| QPoint pos(10, 10); | ||
| QPoint globalPos(tb->mapToGlobal(pos)); | ||
| QMouseEvent mouseEvent( | ||
| QEvent::MouseButtonPress, | ||
| pos, globalPos, Qt::LeftButton, buttons, modifiers); | ||
| QApplication::sendEvent(tb, &mouseEvent); | ||
|
|
||
| // Mose follow the mouse press event with mouse move with some offset | ||
| // in order to activate toolbar dragging. | ||
| QPoint offset(30, 30); | ||
| QMouseEvent mouseMoveEvent( | ||
| QEvent::MouseMove, | ||
| pos+offset, globalPos+offset, | ||
| Qt::LeftButton, buttons, modifiers); | ||
| QApplication::sendEvent(tb, &mouseMoveEvent); | ||
| }); | ||
| } | ||
|
|
||
| void ToolBarGrip::mousePressEvent(QMouseEvent *) | ||
| { | ||
| setCursor(Qt::ClosedHandCursor); | ||
| } | ||
|
|
||
| void ToolBarGrip::mouseReleaseEvent(QMouseEvent *) | ||
| { | ||
| setCursor(Qt::OpenHandCursor); | ||
| } | ||
|
|
||
| // ----------------------------------------------------------- | ||
|
|
||
| ToolBarManager* ToolBarManager::_instance = nullptr; // NOLINT | ||
|
|
||
| ToolBarManager* ToolBarManager::getInstance() | ||
|
|
@@ -756,6 +893,18 @@ | |
| menuBarLeftArea->restoreState(mbLeftToolBars); | ||
| } | ||
|
|
||
| ToolBarArea *ToolBarManager::getToolBarArea(QToolBar *toolbar) const | ||
|
There was a problem hiding this comment. In #13721 I already have such method, but it does more like Qt does it so via enum. This method also covers standard Qt::ToolBarArea's: https://github.com/FreeCAD/FreeCAD/pull/13721/files#diff-dafe6a032a9dc3c2149c60fc7319fbdcda9b32e255a21185f62849cc8ba41902R438. Can you rework your code to be more inline with it so we won't have conflicts? |
||
| { | ||
| auto parent = toolbar->parentWidget(); | ||
| if (parent == statusBarArea) | ||
| return statusBarArea; | ||
| if (parent == menuBarLeftArea) | ||
| return menuBarLeftArea; | ||
| if (parent == menuBarRightArea) | ||
| return menuBarRightArea; | ||
| return nullptr; | ||
| } | ||
|
|
||
| bool ToolBarManager::addToolBarToArea(QObject *source, QMouseEvent *ev) | ||
| { | ||
| auto statusBar = getMainWindow()->statusBar(); | ||
|
|
@@ -887,47 +1036,9 @@ | |
| return false; | ||
| } | ||
|
|
||
| bool ToolBarManager::showContextMenu(QObject *source) | ||
| { | ||
| QMenu menu; | ||
| QLayout* layout = nullptr; | ||
| ToolBarArea* area = nullptr; | ||
| if (getMainWindow()->statusBar() == source) { | ||
|
|
@@ -957,12 +1068,7 @@ | |
| } | ||
|
|
||
| area->foreachToolBar(addMenuVisibleItem); | ||
|
|
||
| menu.exec(QCursor::pos()); | ||
| return true; | ||
| } | ||
|
|
@@ -1097,9 +1203,47 @@ | |
| { | ||
| for (auto& tb : toolBars()) { | ||
| tb->setMovable(moveable); | ||
| setToolBarMovable(tb); | ||
| } | ||
| } | ||
|
|
||
| void ToolBarManager::setToolBarMovable(QToolBar *toolbar) const | ||
|
There was a problem hiding this comment. Since you already added custom I wonder if it would not be better to make this function as handler for At least it should have more descriptive name because for the whole review process I was sure that it calls |
||
| { | ||
| if (!toolbar) | ||
| return; | ||
|
Comment on lines
+1212
to
+1213
There was a problem hiding this comment. I know that this classes do not have automatic code formatting enabled - but it would be nice to adhere to the formatting requested by the project. |
||
|
|
||
| bool movable = toolbar->isMovable(); | ||
|
|
||
| if (auto area = getToolBarArea(toolbar)) { | ||
| if (auto grip = toolbar->findChild<ToolBarGrip*>()) { | ||
| if (!movable) { | ||
| toolbar->removeAction(grip->action()); | ||
| grip->deleteLater(); | ||
| } | ||
| } | ||
| else { | ||
| new ToolBarGrip(toolbar); | ||
|
There was a problem hiding this comment. Creating new object without assigning it to anything is a very strange construction. Probably it is fine as it has properly defined parent so Qt will take care of it but nonetheless it is quite odd - please at least put comment over it that it is intentional and how it is supposed to work. |
||
| } | ||
| area->adjustParent(); | ||
| } | ||
| QString name = QStringLiteral("_fc_toolbar_sep_"); | ||
| auto sep = toolbar->findChild<QAction*>(name); | ||
|
There was a problem hiding this comment.
|
||
| if (sep) { | ||
| if (movable) { | ||
|
Comment on lines
+1231
to
+1232
There was a problem hiding this comment. why not |
||
| toolbar->removeAction(sep); | ||
| sep->deleteLater(); | ||
| } | ||
| } | ||
| else if (!movable) { | ||
| auto actions = toolbar->actions(); | ||
| if (actions.size()) { | ||
| sep = toolbar->insertSeparator(actions[0]); | ||
| sep->setObjectName(name); | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
||
| QToolBar* ToolBarManager::findToolBar(const QList<QToolBar*>& toolbars, const QString& item) const | ||
| { | ||
| for (QToolBar* it : toolbars) { | ||
|
|
@@ -1229,4 +1373,4 @@ | |
| } | ||
| } | ||
|
|
||
| #include "moc_ToolBarManager.cpp" | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While I do understand benefit of having our own ToolBar widget (it would come handy for a lot of reasons) - I don't understand why you create this class now while its only method is a simple call to parent implementation. I'd suggest removing this method and adding other useful helpers like
float(). Override the moveable behavior etc.