-
-
Notifications
You must be signed in to change notification settings - Fork 4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Gui: support dragging of toolbar inside status/menu bar #13973
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since you already have custom There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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; | ||
} | ||
|
||
void ToolBarManager::populateUndockMenu(QMenu *menu, ToolBarArea *area) | ||
{ | ||
menu->setTitle(tr("Undock toolbars")); | ||
auto tooltip = QObject::tr("Undock from status bar"); | ||
auto addMenuUndockItem = [&](QToolBar *toolbar, int, ToolBarArea *area) { | ||
auto *action = toolbar->toggleViewAction(); | ||
auto undockAction = new QAction(menu); | ||
undockAction->setText(action->text()); | ||
undockAction->setToolTip(tooltip); | ||
menu->addAction(undockAction); | ||
QObject::connect(undockAction, &QAction::triggered, [area, toolbar]() { | ||
if (toolbar->parentWidget() == getMainWindow()) { | ||
return; | ||
} | ||
auto pos = toolbar->mapToGlobal(QPoint(0, 0)); | ||
QSignalBlocker blocker(toolbar); | ||
area->removeWidget(toolbar); | ||
getMainWindow()->addToolBar(toolbar); | ||
toolbar->setWindowFlags(Qt::Tool | ||
| Qt::FramelessWindowHint | ||
| Qt::X11BypassWindowManagerHint); | ||
toolbar->move(pos.x(), pos.y()-toolbar->height()-10); | ||
toolbar->adjustSize(); | ||
toolbar->setVisible(true); | ||
Q_EMIT toolbar->topLevelChanged(true); | ||
}); | ||
}; | ||
if (area) { | ||
area->foreachToolBar(addMenuUndockItem); | ||
} | ||
else { | ||
statusBarArea->foreachToolBar(addMenuUndockItem); | ||
menuBarLeftArea->foreachToolBar(addMenuUndockItem); | ||
menuBarRightArea->foreachToolBar(addMenuUndockItem); | ||
} | ||
} | ||
|
||
bool ToolBarManager::showContextMenu(QObject *source) | ||
{ | ||
QMenu menu; | ||
QMenu menuUndock; | ||
QLayout* layout = nullptr; | ||
ToolBarArea* area = nullptr; | ||
if (getMainWindow()->statusBar() == source) { | ||
|
@@ -957,12 +1068,7 @@ | |
} | ||
|
||
area->foreachToolBar(addMenuVisibleItem); | ||
populateUndockMenu(&menuUndock, area); | ||
|
||
if (!menuUndock.actions().empty()) { | ||
menu.addSeparator(); | ||
menu.addMenu(&menuUndock); | ||
} | ||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
if (sep) { | ||
if (movable) { | ||
Comment on lines
+1231
to
+1232
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.