From e3d4bcef3ddfd73038b9573c0e80da20c99f6ea4 Mon Sep 17 00:00:00 2001 From: Pieter Hijma Date: Thu, 25 Apr 2024 21:09:46 +0200 Subject: [PATCH 1/2] Gui: Rearrange the property view context menu --- src/Gui/propertyeditor/PropertyEditor.cpp | 111 +++++++++++++--------- 1 file changed, 65 insertions(+), 46 deletions(-) diff --git a/src/Gui/propertyeditor/PropertyEditor.cpp b/src/Gui/propertyeditor/PropertyEditor.cpp index f9dc427a47..a4fb1105ac 100644 --- a/src/Gui/propertyeditor/PropertyEditor.cpp +++ b/src/Gui/propertyeditor/PropertyEditor.cpp @@ -651,18 +651,11 @@ enum MenuAction { void PropertyEditor::contextMenuEvent(QContextMenuEvent *) { QMenu menu; - QAction *autoExpand = menu.addAction(tr("Auto expand")); - autoExpand->setCheckable(true); - autoExpand->setChecked(autoexpand); - autoExpand->setData(QVariant(MA_AutoExpand)); - - QAction *showAll = menu.addAction(tr("Show all")); - showAll->setCheckable(true); - showAll->setChecked(PropertyView::showAll()); - showAll->setData(QVariant(MA_ShowAll)); + QAction *autoExpand = nullptr; auto contextIndex = currentIndex(); + // acquiring the selected properties std::unordered_set props; const auto indexes = selectedIndexes(); for(const auto& index : indexes) { @@ -678,49 +671,70 @@ void PropertyEditor::contextMenuEvent(QContextMenuEvent *) { } } - if(props.size() == 1) { - auto item = static_cast(contextIndex.internalPointer()); - auto prop = *props.begin(); - if(item->isBound() - && !prop->isDerivedFrom(App::PropertyExpressionEngine::getClassTypeId()) - && !prop->isReadOnly() - && !prop->testStatus(App::Property::Immutable) - && !(prop->getType() & App::Prop_ReadOnly)) + // add property + if(!props.empty()) { + menu.addAction(tr("Add property"))->setData(QVariant(MA_AddProp)); + if (std::all_of(props.begin(), props.end(), [](auto prop) { + return prop->testStatus(App::Property::PropDynamic) + && !boost::starts_with(prop->getName(),prop->getGroup()); + })) { - contextIndex = propertyModel->buddy(contextIndex); - setCurrentIndex(contextIndex); - menu.addSeparator(); - menu.addAction(tr("Expression..."))->setData(QVariant(MA_Expression)); + menu.addAction(tr("Rename property group"))->setData(QVariant(MA_EditPropGroup)); } } + // remove property + bool canRemove = !props.empty(); + unsigned long propType = 0; + unsigned long propStatus = 0xffffffff; + for(auto prop : props) { + propType |= prop->getType(); + propStatus &= prop->getStatus(); + if(!prop->testStatus(App::Property::PropDynamic) + || prop->testStatus(App::Property::LockDynamic)) + { + canRemove = false; + } + } + if(canRemove) + menu.addAction(tr("Remove property"))->setData(QVariant(MA_RemoveProp)); + + // add a separator between adding/removing properties and the rest + if (!props.empty() || canRemove) { + menu.addSeparator(); + } + + // show all + QAction *showAll = menu.addAction(tr("Show all")); + showAll->setCheckable(true); + showAll->setChecked(PropertyView::showAll()); + showAll->setData(QVariant(MA_ShowAll)); + if(PropertyView::showAll()) { - if(!props.empty()) { - menu.addAction(tr("Add property"))->setData(QVariant(MA_AddProp)); - if (std::all_of(props.begin(), props.end(), [](auto prop) { - return prop->testStatus(App::Property::PropDynamic) - && !boost::starts_with(prop->getName(),prop->getGroup()); - })) + // auto expand + autoExpand = menu.addAction(tr("Auto expand")); + autoExpand->setCheckable(true); + autoExpand->setChecked(autoexpand); + autoExpand->setData(QVariant(MA_AutoExpand)); + + // expression + if(props.size() == 1) { + auto item = static_cast(contextIndex.internalPointer()); + auto prop = *props.begin(); + if(item->isBound() + && !prop->isDerivedFrom(App::PropertyExpressionEngine::getClassTypeId()) + && !prop->isReadOnly() + && !prop->testStatus(App::Property::Immutable) + && !(prop->getType() & App::Prop_ReadOnly)) { - menu.addAction(tr("Rename property group"))->setData(QVariant(MA_EditPropGroup)); + contextIndex = propertyModel->buddy(contextIndex); + setCurrentIndex(contextIndex); + // menu.addSeparator(); + menu.addAction(tr("Expression..."))->setData(QVariant(MA_Expression)); } } - bool canRemove = !props.empty(); - unsigned long propType = 0; - unsigned long propStatus = 0xffffffff; - for(auto prop : props) { - propType |= prop->getType(); - propStatus &= prop->getStatus(); - if(!prop->testStatus(App::Property::PropDynamic) - || prop->testStatus(App::Property::LockDynamic)) - { - canRemove = false; - } - } - if(canRemove) - menu.addAction(tr("Remove property"))->setData(QVariant(MA_RemoveProp)); - + // the various flags if(!props.empty()) { menu.addSeparator(); @@ -759,9 +773,14 @@ void PropertyEditor::contextMenuEvent(QContextMenuEvent *) { switch(action->data().toInt()) { case MA_AutoExpand: - autoexpand = autoExpand->isChecked(); - if (autoexpand) - expandAll(); + if (autoExpand) { + // Variable autoExpand should not be null when we arrive here, but + // since we explicitly initialize the variable to nullptr, a check + // nonetheless. + autoexpand = autoExpand->isChecked(); + if (autoexpand) + expandAll(); + } return; case MA_ShowAll: PropertyView::setShowAll(action->isChecked()); From 94312fe8cf65a972a169c7bc163d9369635bd0b8 Mon Sep 17 00:00:00 2001 From: Pieter Hijma Date: Sun, 19 May 2024 22:53:05 +0200 Subject: [PATCH 2/2] Gui: Make one level + status submenu Based on a discussion in the PR, make the menu such that there is only one level except for the status flags. This makes showall unambiguous only meaning that it shows the hidden properties instead of showing all the hidden properties and all options of the menu. "Show all" has therefore been renamed to "Show hidden". --- src/Gui/propertyeditor/PropertyEditor.cpp | 135 +++++++++++----------- 1 file changed, 67 insertions(+), 68 deletions(-) diff --git a/src/Gui/propertyeditor/PropertyEditor.cpp b/src/Gui/propertyeditor/PropertyEditor.cpp index a4fb1105ac..a93b76424d 100644 --- a/src/Gui/propertyeditor/PropertyEditor.cpp +++ b/src/Gui/propertyeditor/PropertyEditor.cpp @@ -634,7 +634,7 @@ void PropertyEditor::removeProperty(const App::Property& prop) enum MenuAction { MA_AutoExpand, - MA_ShowAll, + MA_ShowHidden, MA_Expression, MA_RemoveProp, MA_AddProp, @@ -672,15 +672,13 @@ void PropertyEditor::contextMenuEvent(QContextMenuEvent *) { } // add property - if(!props.empty()) { - menu.addAction(tr("Add property"))->setData(QVariant(MA_AddProp)); - if (std::all_of(props.begin(), props.end(), [](auto prop) { - return prop->testStatus(App::Property::PropDynamic) - && !boost::starts_with(prop->getName(),prop->getGroup()); - })) - { - menu.addAction(tr("Rename property group"))->setData(QVariant(MA_EditPropGroup)); - } + menu.addAction(tr("Add property"))->setData(QVariant(MA_AddProp)); + if (!props.empty() && std::all_of(props.begin(), props.end(), [](auto prop) { + return prop->testStatus(App::Property::PropDynamic) + && !boost::starts_with(prop->getName(),prop->getGroup()); + })) + { + menu.addAction(tr("Rename property group"))->setData(QVariant(MA_EditPropGroup)); } // remove property @@ -700,71 +698,72 @@ void PropertyEditor::contextMenuEvent(QContextMenuEvent *) { menu.addAction(tr("Remove property"))->setData(QVariant(MA_RemoveProp)); // add a separator between adding/removing properties and the rest - if (!props.empty() || canRemove) { - menu.addSeparator(); - } + menu.addSeparator(); // show all - QAction *showAll = menu.addAction(tr("Show all")); - showAll->setCheckable(true); - showAll->setChecked(PropertyView::showAll()); - showAll->setData(QVariant(MA_ShowAll)); + QAction *showHidden = menu.addAction(tr("Show hidden")); + showHidden->setCheckable(true); + showHidden->setChecked(PropertyView::showAll()); + showHidden->setData(QVariant(MA_ShowHidden)); - if(PropertyView::showAll()) { - // auto expand - autoExpand = menu.addAction(tr("Auto expand")); - autoExpand->setCheckable(true); - autoExpand->setChecked(autoexpand); - autoExpand->setData(QVariant(MA_AutoExpand)); + // auto expand + autoExpand = menu.addAction(tr("Auto expand")); + autoExpand->setCheckable(true); + autoExpand->setChecked(autoexpand); + autoExpand->setData(QVariant(MA_AutoExpand)); - // expression - if(props.size() == 1) { - auto item = static_cast(contextIndex.internalPointer()); - auto prop = *props.begin(); - if(item->isBound() - && !prop->isDerivedFrom(App::PropertyExpressionEngine::getClassTypeId()) - && !prop->isReadOnly() - && !prop->testStatus(App::Property::Immutable) - && !(prop->getType() & App::Prop_ReadOnly)) - { - contextIndex = propertyModel->buddy(contextIndex); - setCurrentIndex(contextIndex); - // menu.addSeparator(); - menu.addAction(tr("Expression..."))->setData(QVariant(MA_Expression)); - } + // expression + if(props.size() == 1) { + auto item = static_cast(contextIndex.internalPointer()); + auto prop = *props.begin(); + if(item->isBound() + && !prop->isDerivedFrom(App::PropertyExpressionEngine::getClassTypeId()) + && !prop->isReadOnly() + && !prop->testStatus(App::Property::Immutable) + && !(prop->getType() & App::Prop_ReadOnly)) + { + contextIndex = propertyModel->buddy(contextIndex); + setCurrentIndex(contextIndex); + // menu.addSeparator(); + menu.addAction(tr("Expression..."))->setData(QVariant(MA_Expression)); } + } - // the various flags - if(!props.empty()) { - menu.addSeparator(); + // the various flags + if(!props.empty()) { + menu.addSeparator(); - QAction *action; - QString text; -#define _ACTION_SETUP(_name) do {\ - text = tr(#_name);\ - action = menu.addAction(text);\ - action->setData(QVariant(MA_##_name));\ - action->setCheckable(true);\ - if(propStatus & (1<setChecked(true);\ - }while(0) -#define ACTION_SETUP(_name) do {\ - _ACTION_SETUP(_name);\ - if(propType & App::Prop_##_name) {\ - action->setText(text + QString::fromLatin1(" *"));\ - action->setChecked(true);\ - }\ - }while(0) + // the subMenu is allocated on the heap but managed by menu. + auto subMenu = new QMenu(QString::fromLatin1("Status"), &menu); - ACTION_SETUP(Hidden); - ACTION_SETUP(Output); - ACTION_SETUP(NoRecompute); - ACTION_SETUP(ReadOnly); - ACTION_SETUP(Transient); - _ACTION_SETUP(Touched); - _ACTION_SETUP(EvalOnRestore); - _ACTION_SETUP(CopyOnChange); - } + QAction *action; + QString text; +#define _ACTION_SETUP(_name) do { \ + text = tr(#_name); \ + action = subMenu->addAction(text); \ + action->setData(QVariant(MA_##_name)); \ + action->setCheckable(true); \ + if(propStatus & (1<setChecked(true); \ + }while(0) +#define ACTION_SETUP(_name) do { \ + _ACTION_SETUP(_name); \ + if(propType & App::Prop_##_name) { \ + action->setText(text + QString::fromLatin1(" *")); \ + action->setChecked(true); \ + } \ + }while(0) + + ACTION_SETUP(Hidden); + ACTION_SETUP(Output); + ACTION_SETUP(NoRecompute); + ACTION_SETUP(ReadOnly); + ACTION_SETUP(Transient); + _ACTION_SETUP(Touched); + _ACTION_SETUP(EvalOnRestore); + _ACTION_SETUP(CopyOnChange); + + menu.addMenu(subMenu); } auto action = menu.exec(QCursor::pos()); @@ -782,7 +781,7 @@ void PropertyEditor::contextMenuEvent(QContextMenuEvent *) { expandAll(); } return; - case MA_ShowAll: + case MA_ShowHidden: PropertyView::setShowAll(action->isChecked()); return; #define ACTION_CHECK(_name) \