From 6fc0842a24168032a39eff1b0f114df6de530b12 Mon Sep 17 00:00:00 2001 From: John Tourtellott Date: Fri, 23 Dec 2022 15:21:31 -0500 Subject: [PATCH 1/9] Turn off QTableWidget's tab key navigation * Was breaking all tab ordering logic * Also start tabOrder.sbt for testing --- .../attribute_collection/tabOrder.sbt | 50 +++++++++++++++++++ smtk/extension/qt/qtGroupItem.cxx | 1 + 2 files changed, 51 insertions(+) create mode 100644 data/attribute/attribute_collection/tabOrder.sbt diff --git a/data/attribute/attribute_collection/tabOrder.sbt b/data/attribute/attribute_collection/tabOrder.sbt new file mode 100644 index 0000000000..65504c49b2 --- /dev/null +++ b/data/attribute/attribute_collection/tabOrder.sbt @@ -0,0 +1,50 @@ + + + + + + + 1 + 2 + + + 1 + + + 1 + 2 + 3 + + + 3 + + + 1 + 2 + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/smtk/extension/qt/qtGroupItem.cxx b/smtk/extension/qt/qtGroupItem.cxx index a334a70326..e3e2765c82 100644 --- a/smtk/extension/qt/qtGroupItem.cxx +++ b/smtk/extension/qt/qtGroupItem.cxx @@ -555,6 +555,7 @@ void qtGroupItem::addItemsToTable(int index) 0, QHeaderView::ResizeToContents); m_internals->ItemsTable->verticalHeader()->setSectionResizeMode(QHeaderView::ResizeToContents); frameLayout->addWidget(m_internals->ItemsTable); + m_internals->ItemsTable->setTabKeyNavigation(false); } m_internals->ItemsTable->blockSignals(true); -- GitLab From 08202675a4ef4cd380b50efb479f83da9ab25e16 Mon Sep 17 00:00:00 2001 From: John Tourtellott Date: Fri, 23 Dec 2022 17:29:01 -0500 Subject: [PATCH 2/9] Add qtItem methods to set last previous editor and get last editor widgets * Interim: only works in simple cases (current tabOrder.sbt) * Future: probably make qtItem methods pure virtual * Future: can probably combine into 1 method * Also sets minus button focus to click-only, but Qt still tabs to it --- .../attribute_collection/tabOrder.sbt | 23 ++++++---- smtk/extension/qt/qtDiscreteValueEditor.cxx | 5 +++ smtk/extension/qt/qtDiscreteValueEditor.h | 1 + smtk/extension/qt/qtGroupItem.cxx | 42 ++++++++++++++++++- smtk/extension/qt/qtGroupItem.h | 1 + smtk/extension/qt/qtInputsItem.cxx | 38 ++++++++++++++++- smtk/extension/qt/qtInputsItem.h | 3 ++ smtk/extension/qt/qtItem.h | 5 +++ 8 files changed, 108 insertions(+), 10 deletions(-) diff --git a/data/attribute/attribute_collection/tabOrder.sbt b/data/attribute/attribute_collection/tabOrder.sbt index 65504c49b2..9964f68e6a 100644 --- a/data/attribute/attribute_collection/tabOrder.sbt +++ b/data/attribute/attribute_collection/tabOrder.sbt @@ -2,11 +2,11 @@ - + 1 - 2 - + 3.14159 + 1 @@ -16,10 +16,17 @@ 3 - 3 + + + + + + 3.33,6.66 + + Fourth - 1 + String data 2 @@ -36,13 +43,13 @@ FilterByAdvanceLevel="false" FilterByCategory="false" UseScrollingContainer="false"> - + - + - + diff --git a/smtk/extension/qt/qtDiscreteValueEditor.cxx b/smtk/extension/qt/qtDiscreteValueEditor.cxx index a99c8195aa..018faece19 100644 --- a/smtk/extension/qt/qtDiscreteValueEditor.cxx +++ b/smtk/extension/qt/qtDiscreteValueEditor.cxx @@ -86,6 +86,11 @@ qtDiscreteValueEditor::~qtDiscreteValueEditor() delete this->Internals; } +QWidget* qtDiscreteValueEditor::widget() const +{ + return this->Internals->m_combo; +} + void qtDiscreteValueEditor::createWidget() { smtk::attribute::ResourcePtr attResource = this->Internals->m_inputItem->attributeResource(); diff --git a/smtk/extension/qt/qtDiscreteValueEditor.h b/smtk/extension/qt/qtDiscreteValueEditor.h index 4df0221099..f65783f7de 100644 --- a/smtk/extension/qt/qtDiscreteValueEditor.h +++ b/smtk/extension/qt/qtDiscreteValueEditor.h @@ -35,6 +35,7 @@ public: ~qtDiscreteValueEditor() override; bool useSelectionManger() const { return m_useSelectionManager; } void updateContents(); + QWidget* widget() const; // the internal editing widget (QComboBox) public Q_SLOTS: void onInputValueChanged(); diff --git a/smtk/extension/qt/qtGroupItem.cxx b/smtk/extension/qt/qtGroupItem.cxx index e3e2765c82..20635b219f 100644 --- a/smtk/extension/qt/qtGroupItem.cxx +++ b/smtk/extension/qt/qtGroupItem.cxx @@ -25,6 +25,7 @@ #include #include +#include #include #include #include @@ -54,6 +55,8 @@ public: QPointer m_titleLabel; QPointer m_alertLabel; std::map m_itemViewMap; + QWidget* m_previousEditor = nullptr; + qtItem* m_lastItem = nullptr; }; qtItem* qtGroupItem::createItemWidget(const qtAttributeItemInfo& info) @@ -104,6 +107,11 @@ void qtGroupItem::setLabelVisible(bool visible) m_internals->m_titleFrame->setVisible(visible); } +QWidget* qtGroupItem::lastEditor() const +{ + return m_internals->m_lastItem ? m_internals->m_lastItem->lastEditor() : nullptr; +} + void qtGroupItem::createWidget() { auto item = m_itemInfo.itemAs(); @@ -542,6 +550,23 @@ void qtGroupItem::addItemsToTable(int index) return; } + QWidget* previousEditor = nullptr; + if (index == 0) + { + previousEditor = m_internals->m_previousEditor; + } + else if (index == m_internals->ItemsTable->rowCount()) + { + previousEditor = m_internals->m_lastItem->lastEditor(); + } + else + { + qCritical() << "Internal Error: Unable to add items in middle of table" + << "index" << index << "rowCount" << m_internals->ItemsTable->rowCount() << __FILE__ + << "line" << __LINE__; + return; + } + QBoxLayout* frameLayout = qobject_cast(m_internals->ChildrensFrame->layout()); if (!m_internals->ItemsTable) { @@ -618,6 +643,10 @@ void qtGroupItem::addItemsToTable(int index) childItem->setLabelVisible(false); m_internals->ItemsTable->setCellWidget(index, added + 1, childItem->widget()); itemList.push_back(childItem); + + childItem->setPreviousEditor(previousEditor); + previousEditor = childItem->lastEditor(); + connect( childItem, SIGNAL(widgetSizeChanged()), @@ -631,8 +660,14 @@ void qtGroupItem::addItemsToTable(int index) this, SLOT(onChildItemModified()), static_cast(Qt::AutoConnection | Qt::UniqueConnection)); - } + } // if (childItem) + } // for (j) + + if (!itemList.isEmpty() && index == (m_internals->ItemsTable->rowCount() - 1)) + { + m_internals->m_lastItem = itemList.last(); } + // Check to see if the last column is not fixed width and set the table to stretch // the last column if that is the case if (!(itemList.isEmpty() || itemList.back()->isFixedWidth())) @@ -653,6 +688,11 @@ void qtGroupItem::addItemsToTable(int index) minusButton->setToolTip("Remove Row"); //QVariant vdata(static_cast(i)); //minusButton->setProperty("SubgroupIndex", vdata); + + // Set focus policy to click only. + // Qt "tab" logic still traverses the buttons; reason unknown. + minusButton->setFocusPolicy(Qt::ClickFocus); + connect(minusButton, SIGNAL(clicked()), this, SLOT(onRemoveSubGroup())); m_internals->ItemsTable->setCellWidget(index, 0, minusButton); diff --git a/smtk/extension/qt/qtGroupItem.h b/smtk/extension/qt/qtGroupItem.h index b2ebecc4b2..865c9b77b3 100644 --- a/smtk/extension/qt/qtGroupItem.h +++ b/smtk/extension/qt/qtGroupItem.h @@ -33,6 +33,7 @@ public: qtGroupItem(const qtAttributeItemInfo& info); ~qtGroupItem() override; void setLabelVisible(bool) override; + QWidget* lastEditor() const override; public Q_SLOTS: void updateItemData() override; diff --git a/smtk/extension/qt/qtInputsItem.cxx b/smtk/extension/qt/qtInputsItem.cxx index ead928f347..4565678a62 100644 --- a/smtk/extension/qt/qtInputsItem.cxx +++ b/smtk/extension/qt/qtInputsItem.cxx @@ -241,8 +241,32 @@ public: QPointer m_expressionResultLineEdit; QString m_lastExpression; int m_editPrecision; + + QList m_editors; }; +QWidget* qtInputsItem::lastEditor() const +{ + if (m_internals->m_editors.isEmpty()) + { + return nullptr; + } + return m_internals->m_editors.last(); +} + +void qtInputsItem::setPreviousEditor(QWidget* editor) +{ + QWidget* previousEd = editor; + for (QWidget* ed : m_internals->m_editors) + { + if (ed != nullptr) // needed? + { + QWidget::setTabOrder(previousEd, ed); + } + previousEd = ed; + } +} + qtItem* qtInputsItem::createItemWidget(const qtAttributeItemInfo& info) { // So we support this type of item? @@ -939,6 +963,7 @@ void qtInputsItem::updateUI() { return; } + m_internals->m_editors.clear(); m_widget = new QFrame(this->parentWidget()); m_widget->setObjectName(dataObj->name().c_str()); @@ -1164,6 +1189,11 @@ QWidget* qtInputsItem::createInputWidget(int elementIdx, QLayout* childLayout) { return nullptr; } + // m_internals->m_editors.reserve(elementIdx + 1); + while (m_internals->m_editors.count() <= elementIdx) + { + m_internals->m_editors << nullptr; + } if (item->isDiscrete()) { @@ -1172,9 +1202,13 @@ QWidget* qtInputsItem::createInputWidget(int elementIdx, QLayout* childLayout) QObject::connect(editor, SIGNAL(widgetSizeChanged()), this, SIGNAL(widgetSizeChanged())); // editor->setUseSelectionManager(m_useSelectionManager); m_internals->DiscreteEditors.append(editor); + m_internals->m_editors[elementIdx] = editor->widget(); return editor; } - return this->createEditBox(elementIdx, m_widget); + + QWidget* editorWidget = this->createEditBox(elementIdx, m_widget); + m_internals->m_editors[elementIdx] = editorWidget; + return editorWidget; } QFrame* qtInputsItem::createExpressionRefFrame() @@ -1684,6 +1718,7 @@ QWidget* qtInputsItem::createIntWidget( { editBox->setText(vitem->valueAsString(elementIdx).c_str()); } + return editBox; } @@ -1710,6 +1745,7 @@ QWidget* qtInputsItem::createIntWidget( spinbox->setValue(iitem->value(elementIdx)); } connect(spinbox, SIGNAL(valueChanged(int)), this, SLOT(intValueChanged(int))); + return spinbox; } return nullptr; diff --git a/smtk/extension/qt/qtInputsItem.h b/smtk/extension/qt/qtInputsItem.h index 4d9f329a1f..14564bdae0 100644 --- a/smtk/extension/qt/qtInputsItem.h +++ b/smtk/extension/qt/qtInputsItem.h @@ -51,6 +51,9 @@ public: bool isFixedWidth() const override; bool eventFilter(QObject* filterObj, QEvent* ev) override; + QWidget* lastEditor() const override; + void setPreviousEditor(QWidget*) override; + public Q_SLOTS: void setOutputOptional(int); void onExpressionReferenceChanged(); diff --git a/smtk/extension/qt/qtItem.h b/smtk/extension/qt/qtItem.h index 8f33d47a63..d684560462 100644 --- a/smtk/extension/qt/qtItem.h +++ b/smtk/extension/qt/qtItem.h @@ -96,6 +96,11 @@ public: /// cleanup such as stop observing SMTK "signals". virtual void markForDeletion(); + /** \brief Returns editor widget, used when setting tab order */ + virtual QWidget* lastEditor() const { return nullptr; } // future: make pure virtual? + /** \brief Sets previous widget for tabbing order */ + virtual void setPreviousEditor(QWidget* w) {} // future: make pure virtual? + public Q_SLOTS: // Controls whether the Selection Manager should be used for setting model // and mesh entity items - Note that this is just a hint and could be ignored -- GitLab From 66574fef6f5bb13061b46519dbb1053494e172f9 Mon Sep 17 00:00:00 2001 From: John Tourtellott Date: Thu, 29 Dec 2022 12:39:53 -0500 Subject: [PATCH 3/9] At tab ordering code to qtReferenceItemEditor (combo box) * Also rename method in qtDiscreteValueEditor from widget() to editingWidget() * Also disable selection inside QTableWidget --- .../attribute_collection/tabOrder.sbt | 42 ++++++++++++++++--- smtk/extension/qt/qtDiscreteValueEditor.cxx | 2 +- smtk/extension/qt/qtDiscreteValueEditor.h | 2 +- smtk/extension/qt/qtGroupItem.cxx | 5 ++- smtk/extension/qt/qtInputsItem.cxx | 2 +- smtk/extension/qt/qtReferenceItemEditor.cxx | 13 ++++++ smtk/extension/qt/qtReferenceItemEditor.h | 3 ++ 7 files changed, 60 insertions(+), 9 deletions(-) diff --git a/data/attribute/attribute_collection/tabOrder.sbt b/data/attribute/attribute_collection/tabOrder.sbt index 9964f68e6a..44e90c7d9c 100644 --- a/data/attribute/attribute_collection/tabOrder.sbt +++ b/data/attribute/attribute_collection/tabOrder.sbt @@ -2,11 +2,11 @@ - + 1 3.14159 - + 1 @@ -31,6 +31,31 @@ + + + 1 + + + 1 + + + + + + + + + + + + + string item + + + String data + + + @@ -43,13 +68,20 @@ FilterByAdvanceLevel="false" FilterByCategory="false" UseScrollingContainer="false"> - + + - + + + + + + + - + diff --git a/smtk/extension/qt/qtDiscreteValueEditor.cxx b/smtk/extension/qt/qtDiscreteValueEditor.cxx index 018faece19..b6b0d134c3 100644 --- a/smtk/extension/qt/qtDiscreteValueEditor.cxx +++ b/smtk/extension/qt/qtDiscreteValueEditor.cxx @@ -86,7 +86,7 @@ qtDiscreteValueEditor::~qtDiscreteValueEditor() delete this->Internals; } -QWidget* qtDiscreteValueEditor::widget() const +QWidget* qtDiscreteValueEditor::editingWidget() const { return this->Internals->m_combo; } diff --git a/smtk/extension/qt/qtDiscreteValueEditor.h b/smtk/extension/qt/qtDiscreteValueEditor.h index f65783f7de..d7c377ca08 100644 --- a/smtk/extension/qt/qtDiscreteValueEditor.h +++ b/smtk/extension/qt/qtDiscreteValueEditor.h @@ -35,7 +35,7 @@ public: ~qtDiscreteValueEditor() override; bool useSelectionManger() const { return m_useSelectionManager; } void updateContents(); - QWidget* widget() const; // the internal editing widget (QComboBox) + QWidget* editingWidget() const; // the internal editing widget (QComboBox) public Q_SLOTS: void onInputValueChanged(); diff --git a/smtk/extension/qt/qtGroupItem.cxx b/smtk/extension/qt/qtGroupItem.cxx index 20635b219f..3e2edf0790 100644 --- a/smtk/extension/qt/qtGroupItem.cxx +++ b/smtk/extension/qt/qtGroupItem.cxx @@ -574,13 +574,16 @@ void qtGroupItem::addItemsToTable(int index) m_internals->ItemsTable->setObjectName(QString("ItemsTable%1").arg(index)); m_internals->ItemsTable->setSizePolicy(QSizePolicy::Expanding, QSizePolicy::Expanding); + // Disable interaction with QTableWidget items + m_internals->ItemsTable->setSelectionMode(QAbstractItemView::NoSelection); + m_internals->ItemsTable->setTabKeyNavigation(false); + m_internals->ItemsTable->setColumnCount(1); // for minus button m_internals->ItemsTable->setHorizontalHeaderItem(0, new QTableWidgetItem(" ")); m_internals->ItemsTable->horizontalHeader()->setSectionResizeMode( 0, QHeaderView::ResizeToContents); m_internals->ItemsTable->verticalHeader()->setSectionResizeMode(QHeaderView::ResizeToContents); frameLayout->addWidget(m_internals->ItemsTable); - m_internals->ItemsTable->setTabKeyNavigation(false); } m_internals->ItemsTable->blockSignals(true); diff --git a/smtk/extension/qt/qtInputsItem.cxx b/smtk/extension/qt/qtInputsItem.cxx index 4565678a62..75d662d5f4 100644 --- a/smtk/extension/qt/qtInputsItem.cxx +++ b/smtk/extension/qt/qtInputsItem.cxx @@ -1202,7 +1202,7 @@ QWidget* qtInputsItem::createInputWidget(int elementIdx, QLayout* childLayout) QObject::connect(editor, SIGNAL(widgetSizeChanged()), this, SIGNAL(widgetSizeChanged())); // editor->setUseSelectionManager(m_useSelectionManager); m_internals->DiscreteEditors.append(editor); - m_internals->m_editors[elementIdx] = editor->widget(); + m_internals->m_editors[elementIdx] = editor->editingWidget(); return editor; } diff --git a/smtk/extension/qt/qtReferenceItemEditor.cxx b/smtk/extension/qt/qtReferenceItemEditor.cxx index 991d88f708..e13dcc87c9 100644 --- a/smtk/extension/qt/qtReferenceItemEditor.cxx +++ b/smtk/extension/qt/qtReferenceItemEditor.cxx @@ -480,6 +480,19 @@ smtk::resource::PersistentObjectPtr qtReferenceItemEditor::object(int index) return selectedObj; } +QWidget* qtReferenceItemEditor::lastEditor() const +{ + return m_internals->m_comboBox; +} + +void qtReferenceItemEditor::setPreviousEditor(QWidget* widget) +{ + if (m_internals->m_comboBox != nullptr) + { + QWidget::setTabOrder(widget, m_internals->m_comboBox); + } +} + void qtReferenceItemEditor::highlightItem(int index) { // Are we dealing with the create new option diff --git a/smtk/extension/qt/qtReferenceItemEditor.h b/smtk/extension/qt/qtReferenceItemEditor.h index 8ebcd3f2cd..484e7e4efa 100644 --- a/smtk/extension/qt/qtReferenceItemEditor.h +++ b/smtk/extension/qt/qtReferenceItemEditor.h @@ -61,6 +61,9 @@ public: void setDefinitionForCreation(smtk::attribute::DefinitionPtr& def); void setOkToCreate(bool val) { m_okToCreate = val; } smtk::resource::PersistentObjectPtr object(int index); + + QWidget* lastEditor() const override; + void setPreviousEditor(QWidget*) override; public Q_SLOTS: void updateItemData() override; void highlightItem(int index); -- GitLab From 0cf79ef41a64ef413dc885f6ef5cf4d9b3ed548d Mon Sep 17 00:00:00 2001 From: John Tourtellott Date: Tue, 3 Jan 2023 13:18:52 -0500 Subject: [PATCH 4/9] Add logic for expression items (editing widget changes) Adds signal qtItem::editingWidgetChanged --- .../attribute_collection/tabOrder.sbt | 45 +++++++++++++++++++ smtk/extension/qt/qtGroupItem.cxx | 19 ++++++++ smtk/extension/qt/qtGroupItem.h | 1 + smtk/extension/qt/qtInputsItem.cxx | 24 +++++++++- smtk/extension/qt/qtItem.h | 10 ++++- 5 files changed, 96 insertions(+), 3 deletions(-) diff --git a/data/attribute/attribute_collection/tabOrder.sbt b/data/attribute/attribute_collection/tabOrder.sbt index 44e90c7d9c..1fd4bd243a 100644 --- a/data/attribute/attribute_collection/tabOrder.sbt +++ b/data/attribute/attribute_collection/tabOrder.sbt @@ -56,6 +56,39 @@ + + + 1 + + + + 1 + + 2.71828 + Example + + + string item + + + String data + + + @@ -70,6 +103,7 @@ + @@ -85,5 +119,16 @@ + + + + + + + + + + + diff --git a/smtk/extension/qt/qtGroupItem.cxx b/smtk/extension/qt/qtGroupItem.cxx index 3e2edf0790..8c2320e85f 100644 --- a/smtk/extension/qt/qtGroupItem.cxx +++ b/smtk/extension/qt/qtGroupItem.cxx @@ -619,6 +619,14 @@ void qtGroupItem::addItemsToTable(int index) } if (childItem) { + // If item uses expressions, listen for editing widget changes + auto valueItem = std::dynamic_pointer_cast(citem); + if (valueItem && valueItem->allowsExpressions()) + { + QObject::connect( + childItem, &qtItem::editingWidgetChanged, this, &qtGroupItem::updateTabOrder); + } + this->addChildItem(childItem); if (added == 0) { @@ -828,3 +836,14 @@ void qtGroupItem::onImportFromFile() QMessageBox::warning(m_widget, tr("GroupItem Import Log)"), logger.convertToString().c_str()); } } + +void qtGroupItem::updateTabOrder() +{ + // Brute force iterate over all qtItem instances + QWidget* previousEditor = m_internals->m_previousEditor; + Q_FOREACH (qtItem* item, m_childItems) + { + item->setPreviousEditor(previousEditor); + previousEditor = item->lastEditor(); + } +} diff --git a/smtk/extension/qt/qtGroupItem.h b/smtk/extension/qt/qtGroupItem.h index 865c9b77b3..7cbe099c3f 100644 --- a/smtk/extension/qt/qtGroupItem.h +++ b/smtk/extension/qt/qtGroupItem.h @@ -45,6 +45,7 @@ protected Q_SLOTS: void onChildWidgetSizeChanged() override; virtual void onChildItemModified(); void onImportFromFile(); + void updateTabOrder(); protected: void createWidget() override; diff --git a/smtk/extension/qt/qtInputsItem.cxx b/smtk/extension/qt/qtInputsItem.cxx index 75d662d5f4..ae6085fc9a 100644 --- a/smtk/extension/qt/qtInputsItem.cxx +++ b/smtk/extension/qt/qtInputsItem.cxx @@ -247,7 +247,11 @@ public: QWidget* qtInputsItem::lastEditor() const { - if (m_internals->m_editors.isEmpty()) + if (m_internals->m_expressionCombo && m_internals->m_expressionCombo->isVisible()) + { + return m_internals->m_expressionCombo; + } + else if (m_internals->m_editors.isEmpty()) { return nullptr; } @@ -257,6 +261,15 @@ QWidget* qtInputsItem::lastEditor() const void qtInputsItem::setPreviousEditor(QWidget* editor) { QWidget* previousEd = editor; + + // Expression editor + if (m_internals->m_expressionCombo && m_internals->m_expressionCombo->isVisible()) + { + QWidget::setTabOrder(previousEd, m_internals->m_expressionCombo); + return; + } + + // Value editor(s) for (QWidget* ed : m_internals->m_editors) { if (ed != nullptr) // needed? @@ -750,6 +763,7 @@ void qtInputsItem::addInputEditor(int i) minusButton->setIcon(QIcon(iconName)); minusButton->setSizePolicy(sizeFixedPolicy); minusButton->setToolTip("Remove value"); + minusButton->setFocusPolicy(Qt::ClickFocus); editorLayout->addWidget(minusButton); connect(minusButton, SIGNAL(clicked()), this, SLOT(onRemoveValue())); QPair, QPointer> pair; @@ -943,6 +957,7 @@ QFrame* qtInputsItem::createLabelFrame( m_internals->m_expressionButton->setSizePolicy(sizeFixedPolicy); m_internals->m_expressionButton->setToolTip( "Switch between a constant value or function instance"); + m_internals->m_expressionButton->setFocusPolicy(Qt::ClickFocus); QObject::connect( m_internals->m_expressionButton, SIGNAL(toggled(bool)), @@ -1377,8 +1392,15 @@ void qtInputsItem::displayExpressionWidget(bool checkstate) } } + bool widgetChanged = checkstate == m_internals->m_valuesFrame->isVisible(); + m_internals->m_valuesFrame->setVisible(!checkstate); m_internals->m_expressionFrame->setVisible(checkstate); + + if (widgetChanged) + { + QTimer::singleShot(0, [this]() { Q_EMIT this->editingWidgetChanged(); }); + } } void qtInputsItem::onExpressionReferenceChanged() diff --git a/smtk/extension/qt/qtItem.h b/smtk/extension/qt/qtItem.h index d684560462..9db71399c6 100644 --- a/smtk/extension/qt/qtItem.h +++ b/smtk/extension/qt/qtItem.h @@ -97,9 +97,9 @@ public: virtual void markForDeletion(); /** \brief Returns editor widget, used when setting tab order */ - virtual QWidget* lastEditor() const { return nullptr; } // future: make pure virtual? + virtual QWidget* lastEditor() const { return nullptr; } /** \brief Sets previous widget for tabbing order */ - virtual void setPreviousEditor(QWidget* w) {} // future: make pure virtual? + virtual void setPreviousEditor(QWidget* /*w*/) {} public Q_SLOTS: // Controls whether the Selection Manager should be used for setting model @@ -121,6 +121,12 @@ Q_SIGNALS: /// void childModified(qtItem* item); + /** \brief Indicates editing widget changed + * + * In some cases, parent item should update tab order. + */ + void editingWidgetChanged(); + protected Q_SLOTS: virtual void onAdvanceLevelChanged(int levelIdx); virtual void onChildWidgetSizeChanged() { ; } -- GitLab From 30aac41146dc6358ba1053dcdbb830f2c6a7747c Mon Sep 17 00:00:00 2001 From: John Tourtellott Date: Wed, 4 Jan 2023 11:52:20 -0500 Subject: [PATCH 5/9] Simplify last-editor logic in qtGroupItem * Don't need to store the widget * Also remote "Add Row" button from tab focus --- smtk/extension/qt/qtGroupItem.cxx | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/smtk/extension/qt/qtGroupItem.cxx b/smtk/extension/qt/qtGroupItem.cxx index 8c2320e85f..1bea567f2b 100644 --- a/smtk/extension/qt/qtGroupItem.cxx +++ b/smtk/extension/qt/qtGroupItem.cxx @@ -56,7 +56,6 @@ public: QPointer m_alertLabel; std::map m_itemViewMap; QWidget* m_previousEditor = nullptr; - qtItem* m_lastItem = nullptr; }; qtItem* qtGroupItem::createItemWidget(const qtAttributeItemInfo& info) @@ -109,7 +108,7 @@ void qtGroupItem::setLabelVisible(bool visible) QWidget* qtGroupItem::lastEditor() const { - return m_internals->m_lastItem ? m_internals->m_lastItem->lastEditor() : nullptr; + return m_childItems.isEmpty() ? nullptr : m_childItems.last()->lastEditor(); } void qtGroupItem::createWidget() @@ -311,6 +310,7 @@ void qtGroupItem::updateItemData() m_internals->AddItemButton = new QToolButton(m_internals->ButtonsFrame); m_internals->AddItemButton->setObjectName("AddItemButton"); m_internals->AddItemButton->setToolButtonStyle(Qt::ToolButtonTextBesideIcon); + m_internals->AddItemButton->setFocusPolicy(Qt::ClickFocus); QString iconName(":/icons/attribute/plus.png"); std::string extensibleLabel = "Add Row"; m_itemInfo.component().attribute("ExtensibleLabel", extensibleLabel); @@ -557,7 +557,7 @@ void qtGroupItem::addItemsToTable(int index) } else if (index == m_internals->ItemsTable->rowCount()) { - previousEditor = m_internals->m_lastItem->lastEditor(); + previousEditor = this->lastEditor(); } else { @@ -674,11 +674,6 @@ void qtGroupItem::addItemsToTable(int index) } // if (childItem) } // for (j) - if (!itemList.isEmpty() && index == (m_internals->ItemsTable->rowCount() - 1)) - { - m_internals->m_lastItem = itemList.last(); - } - // Check to see if the last column is not fixed width and set the table to stretch // the last column if that is the case if (!(itemList.isEmpty() || itemList.back()->isFixedWidth())) -- GitLab From b524208b56244b7ade7ccaed9963592a23832c47 Mon Sep 17 00:00:00 2001 From: John Tourtellott Date: Wed, 4 Jan 2023 15:11:30 -0500 Subject: [PATCH 6/9] Update qtGroupItem tab order when QTableWidget is created * In order to include any expression editors. * Also update qtInputsItem to store expression editor state (more reliable than widget visibility) --- .../attribute_collection/tabOrder.sbt | 20 +++++++++++++++---- smtk/extension/qt/qtGroupItem.cxx | 10 ++++++++++ smtk/extension/qt/qtInputsItem.cxx | 11 ++++++++-- 3 files changed, 35 insertions(+), 6 deletions(-) diff --git a/data/attribute/attribute_collection/tabOrder.sbt b/data/attribute/attribute_collection/tabOrder.sbt index 1fd4bd243a..dfbc0f1079 100644 --- a/data/attribute/attribute_collection/tabOrder.sbt +++ b/data/attribute/attribute_collection/tabOrder.sbt @@ -59,7 +59,7 @@ 1 - + 1 @@ -79,9 +79,9 @@ 2.71828 Example - + string item @@ -104,6 +104,7 @@ + @@ -130,5 +131,16 @@ + + + + + + + + + + + diff --git a/smtk/extension/qt/qtGroupItem.cxx b/smtk/extension/qt/qtGroupItem.cxx index 1bea567f2b..05e03cbdcc 100644 --- a/smtk/extension/qt/qtGroupItem.cxx +++ b/smtk/extension/qt/qtGroupItem.cxx @@ -34,6 +34,7 @@ #include #include #include +#include #include #include @@ -227,6 +228,9 @@ void qtGroupItem::createWidget() { m_internals->m_titleCheckbox->setVisible(false); } + + // Update tab ordering on next event loop + QTimer::singleShot(0, this, &qtGroupItem::updateTabOrder); } void qtGroupItem::setEnabledState(int state) @@ -834,6 +838,12 @@ void qtGroupItem::onImportFromFile() void qtGroupItem::updateTabOrder() { + // Only applicable to QTableWidget + if (m_internals->ItemsTable == nullptr) + { + return; + } + // Brute force iterate over all qtItem instances QWidget* previousEditor = m_internals->m_previousEditor; Q_FOREACH (qtItem* item, m_childItems) diff --git a/smtk/extension/qt/qtInputsItem.cxx b/smtk/extension/qt/qtInputsItem.cxx index ae6085fc9a..3d9119fa54 100644 --- a/smtk/extension/qt/qtInputsItem.cxx +++ b/smtk/extension/qt/qtInputsItem.cxx @@ -242,12 +242,14 @@ public: QString m_lastExpression; int m_editPrecision; + // Store expression on/off state in lieu of widget visibility which isn't immediate + bool m_usingExpression = false; QList m_editors; }; QWidget* qtInputsItem::lastEditor() const { - if (m_internals->m_expressionCombo && m_internals->m_expressionCombo->isVisible()) + if (m_internals->m_usingExpression) { return m_internals->m_expressionCombo; } @@ -1032,6 +1034,10 @@ void qtInputsItem::updateUI() m_internals->m_expressionButton->setChecked(dataObj->isExpression() || expressionOnly); this->displayExpressionWidget(dataObj->isExpression() || expressionOnly); m_internals->m_expressionButton->setEnabled(!expressionOnly); + if (expressionOnly) + { + m_internals->m_usingExpression = true; + } } if (this->parentWidget() && this->parentWidget()->layout()) @@ -1276,6 +1282,7 @@ void qtInputsItem::displayExpressionWidget(bool checkstate) { return; } + m_internals->m_usingExpression = checkstate; ResourcePtr sourceAttResource = inputitem->attribute()->attributeResource(); @@ -1397,7 +1404,7 @@ void qtInputsItem::displayExpressionWidget(bool checkstate) m_internals->m_valuesFrame->setVisible(!checkstate); m_internals->m_expressionFrame->setVisible(checkstate); - if (widgetChanged) + if (QObject::sender() && widgetChanged) { QTimer::singleShot(0, [this]() { Q_EMIT this->editingWidgetChanged(); }); } -- GitLab From 50165658717348c352163a44d88ccda49ff07d2d Mon Sep 17 00:00:00 2001 From: John Tourtellott Date: Wed, 4 Jan 2023 16:07:43 -0500 Subject: [PATCH 7/9] Add non-working examples with optional items and children items Also add release note --- .../attribute_collection/tabOrder.sbt | 76 +++++++++++++++++++ .../notes/qt-extensible-group-tab-order.rst | 17 +++++ 2 files changed, 93 insertions(+) create mode 100644 doc/release/notes/qt-extensible-group-tab-order.rst diff --git a/data/attribute/attribute_collection/tabOrder.sbt b/data/attribute/attribute_collection/tabOrder.sbt index dfbc0f1079..5dcd81ba0a 100644 --- a/data/attribute/attribute_collection/tabOrder.sbt +++ b/data/attribute/attribute_collection/tabOrder.sbt @@ -1,6 +1,34 @@ + + + + + + 1 + 2 + + + + 2.71828 + + Child1 + + + + 3.14159 + + Child1 + Child2 + + + + + + + + @@ -89,6 +117,40 @@ + + + + 1 + 99 + 3.14159 + + + 1 + 2.71828 + string item + + + String data + + + + + + + 1 + + 2 + + + 1 + + string item + + + String data + + + @@ -104,6 +166,8 @@ + + @@ -131,6 +195,18 @@ + + + + + + + + + + + + diff --git a/doc/release/notes/qt-extensible-group-tab-order.rst b/doc/release/notes/qt-extensible-group-tab-order.rst new file mode 100644 index 0000000000..20295b5233 --- /dev/null +++ b/doc/release/notes/qt-extensible-group-tab-order.rst @@ -0,0 +1,17 @@ +Tab order in extensible group items +------------------------------------ + +Extensible group items are displayed in a Qt table widget instance with +one row for each subgroup. Previously, the table widget would not +respond to tab key input (typically used for navigating between cells). +This has been corrected for most mainstream cases: double items, +integer items, string items, and reference items (dropdown box format). +Tab-ordering also works with the discrete and expression versions of +value items. + +There are some cases that have not yet been implemented. The overall +tab order might not be consistently preserved between the table widget +and other items before and after it in the attribute editor. For optional +items, the checkbox is not reliably included in the tab order. The tab +behavior is also not defined for discrete items that include conditional +children. -- GitLab From fa25961d719b5e05f1f2f676ec431ef8ec262a40 Mon Sep 17 00:00:00 2001 From: John Tourtellott Date: Thu, 5 Jan 2023 14:04:29 -0500 Subject: [PATCH 8/9] Rename setPreviousEditor() method to updateTabOrder() * More accurate description of the function * Also rename "previousEditor" variables to "precedingEditor" --- smtk/extension/qt/qtGroupItem.cxx | 24 ++++++++++----------- smtk/extension/qt/qtGroupItem.h | 2 +- smtk/extension/qt/qtInputsItem.cxx | 6 ++---- smtk/extension/qt/qtInputsItem.h | 2 +- smtk/extension/qt/qtItem.h | 2 +- smtk/extension/qt/qtReferenceItemEditor.cxx | 4 ++-- smtk/extension/qt/qtReferenceItemEditor.h | 2 +- 7 files changed, 20 insertions(+), 22 deletions(-) diff --git a/smtk/extension/qt/qtGroupItem.cxx b/smtk/extension/qt/qtGroupItem.cxx index 05e03cbdcc..a02a5c1cf2 100644 --- a/smtk/extension/qt/qtGroupItem.cxx +++ b/smtk/extension/qt/qtGroupItem.cxx @@ -56,7 +56,7 @@ public: QPointer m_titleLabel; QPointer m_alertLabel; std::map m_itemViewMap; - QWidget* m_previousEditor = nullptr; + QWidget* m_precedingEditor = nullptr; }; qtItem* qtGroupItem::createItemWidget(const qtAttributeItemInfo& info) @@ -230,7 +230,7 @@ void qtGroupItem::createWidget() } // Update tab ordering on next event loop - QTimer::singleShot(0, this, &qtGroupItem::updateTabOrder); + QTimer::singleShot(0, [this]() { this->updateTabOrder(m_internals->m_precedingEditor); }); } void qtGroupItem::setEnabledState(int state) @@ -554,14 +554,14 @@ void qtGroupItem::addItemsToTable(int index) return; } - QWidget* previousEditor = nullptr; + QWidget* precedingEditor = nullptr; if (index == 0) { - previousEditor = m_internals->m_previousEditor; + precedingEditor = m_internals->m_precedingEditor; } else if (index == m_internals->ItemsTable->rowCount()) { - previousEditor = this->lastEditor(); + precedingEditor = this->lastEditor(); } else { @@ -628,7 +628,7 @@ void qtGroupItem::addItemsToTable(int index) if (valueItem && valueItem->allowsExpressions()) { QObject::connect( - childItem, &qtItem::editingWidgetChanged, this, &qtGroupItem::updateTabOrder); + childItem, &qtItem::editingWidgetChanged, this, &qtGroupItem::onEditingWidgetChanged); } this->addChildItem(childItem); @@ -659,8 +659,8 @@ void qtGroupItem::addItemsToTable(int index) m_internals->ItemsTable->setCellWidget(index, added + 1, childItem->widget()); itemList.push_back(childItem); - childItem->setPreviousEditor(previousEditor); - previousEditor = childItem->lastEditor(); + childItem->updateTabOrder(precedingEditor); + precedingEditor = childItem->lastEditor(); connect( childItem, @@ -836,7 +836,7 @@ void qtGroupItem::onImportFromFile() } } -void qtGroupItem::updateTabOrder() +void qtGroupItem::onEditingWidgetChanged() { // Only applicable to QTableWidget if (m_internals->ItemsTable == nullptr) @@ -845,10 +845,10 @@ void qtGroupItem::updateTabOrder() } // Brute force iterate over all qtItem instances - QWidget* previousEditor = m_internals->m_previousEditor; + QWidget* precedingEditor = m_internals->m_precedingEditor; Q_FOREACH (qtItem* item, m_childItems) { - item->setPreviousEditor(previousEditor); - previousEditor = item->lastEditor(); + item->updateTabOrder(precedingEditor); + precedingEditor = item->lastEditor(); } } diff --git a/smtk/extension/qt/qtGroupItem.h b/smtk/extension/qt/qtGroupItem.h index 7cbe099c3f..01bcfe5fb2 100644 --- a/smtk/extension/qt/qtGroupItem.h +++ b/smtk/extension/qt/qtGroupItem.h @@ -45,7 +45,7 @@ protected Q_SLOTS: void onChildWidgetSizeChanged() override; virtual void onChildItemModified(); void onImportFromFile(); - void updateTabOrder(); + void onEditingWidgetChanged(); protected: void createWidget() override; diff --git a/smtk/extension/qt/qtInputsItem.cxx b/smtk/extension/qt/qtInputsItem.cxx index 3d9119fa54..8f91d36a4b 100644 --- a/smtk/extension/qt/qtInputsItem.cxx +++ b/smtk/extension/qt/qtInputsItem.cxx @@ -260,9 +260,9 @@ QWidget* qtInputsItem::lastEditor() const return m_internals->m_editors.last(); } -void qtInputsItem::setPreviousEditor(QWidget* editor) +void qtInputsItem::updateTabOrder(QWidget* precedingEditor) { - QWidget* previousEd = editor; + QWidget* previousEd = precedingEditor; // Expression editor if (m_internals->m_expressionCombo && m_internals->m_expressionCombo->isVisible()) @@ -1210,7 +1210,6 @@ QWidget* qtInputsItem::createInputWidget(int elementIdx, QLayout* childLayout) { return nullptr; } - // m_internals->m_editors.reserve(elementIdx + 1); while (m_internals->m_editors.count() <= elementIdx) { m_internals->m_editors << nullptr; @@ -1774,7 +1773,6 @@ QWidget* qtInputsItem::createIntWidget( spinbox->setValue(iitem->value(elementIdx)); } connect(spinbox, SIGNAL(valueChanged(int)), this, SLOT(intValueChanged(int))); - return spinbox; } return nullptr; diff --git a/smtk/extension/qt/qtInputsItem.h b/smtk/extension/qt/qtInputsItem.h index 14564bdae0..784e538a65 100644 --- a/smtk/extension/qt/qtInputsItem.h +++ b/smtk/extension/qt/qtInputsItem.h @@ -52,7 +52,7 @@ public: bool eventFilter(QObject* filterObj, QEvent* ev) override; QWidget* lastEditor() const override; - void setPreviousEditor(QWidget*) override; + void updateTabOrder(QWidget* precedingEditor) override; public Q_SLOTS: void setOutputOptional(int); diff --git a/smtk/extension/qt/qtItem.h b/smtk/extension/qt/qtItem.h index 9db71399c6..cd77891499 100644 --- a/smtk/extension/qt/qtItem.h +++ b/smtk/extension/qt/qtItem.h @@ -99,7 +99,7 @@ public: /** \brief Returns editor widget, used when setting tab order */ virtual QWidget* lastEditor() const { return nullptr; } /** \brief Sets previous widget for tabbing order */ - virtual void setPreviousEditor(QWidget* /*w*/) {} + virtual void updateTabOrder(QWidget* /*precedingEditor*/) {} public Q_SLOTS: // Controls whether the Selection Manager should be used for setting model diff --git a/smtk/extension/qt/qtReferenceItemEditor.cxx b/smtk/extension/qt/qtReferenceItemEditor.cxx index e13dcc87c9..1193afdb59 100644 --- a/smtk/extension/qt/qtReferenceItemEditor.cxx +++ b/smtk/extension/qt/qtReferenceItemEditor.cxx @@ -485,11 +485,11 @@ QWidget* qtReferenceItemEditor::lastEditor() const return m_internals->m_comboBox; } -void qtReferenceItemEditor::setPreviousEditor(QWidget* widget) +void qtReferenceItemEditor::updateTabOrder(QWidget* precedingEditor) { if (m_internals->m_comboBox != nullptr) { - QWidget::setTabOrder(widget, m_internals->m_comboBox); + QWidget::setTabOrder(precedingEditor, m_internals->m_comboBox); } } diff --git a/smtk/extension/qt/qtReferenceItemEditor.h b/smtk/extension/qt/qtReferenceItemEditor.h index 484e7e4efa..e810695a6b 100644 --- a/smtk/extension/qt/qtReferenceItemEditor.h +++ b/smtk/extension/qt/qtReferenceItemEditor.h @@ -63,7 +63,7 @@ public: smtk::resource::PersistentObjectPtr object(int index); QWidget* lastEditor() const override; - void setPreviousEditor(QWidget*) override; + void updateTabOrder(QWidget* precedingEditor) override; public Q_SLOTS: void updateItemData() override; void highlightItem(int index); -- GitLab From 2ac17871ad5a60d0bae4fea40c4ff7be6a94a976 Mon Sep 17 00:00:00 2001 From: John Tourtellott Date: Thu, 5 Jan 2023 16:45:01 -0500 Subject: [PATCH 9/9] Update code to recognize that discrete & ref items can have children Tab ordering is not implemented for children items, as noted by comments in the code. --- smtk/extension/qt/qtDiscreteValueEditor.cxx | 9 ++++++++- smtk/extension/qt/qtDiscreteValueEditor.h | 11 ++++++++++- smtk/extension/qt/qtInputsItem.cxx | 8 +++++++- smtk/extension/qt/qtReferenceItemEditor.cxx | 7 +++++++ smtk/extension/qt/qtReferenceItemEditor.h | 5 +++++ 5 files changed, 37 insertions(+), 3 deletions(-) diff --git a/smtk/extension/qt/qtDiscreteValueEditor.cxx b/smtk/extension/qt/qtDiscreteValueEditor.cxx index b6b0d134c3..968ec30b04 100644 --- a/smtk/extension/qt/qtDiscreteValueEditor.cxx +++ b/smtk/extension/qt/qtDiscreteValueEditor.cxx @@ -86,8 +86,9 @@ qtDiscreteValueEditor::~qtDiscreteValueEditor() delete this->Internals; } -QWidget* qtDiscreteValueEditor::editingWidget() const +QWidget* qtDiscreteValueEditor::lastEditingWidget() const { + // The current logic does not support conditional children items. return this->Internals->m_combo; } @@ -487,4 +488,10 @@ void qtDiscreteValueEditor::updateContents() } this->Internals->m_inputItem->m_itemInfo.baseView()->childrenResized(); Q_EMIT this->widgetSizeChanged(); + + // If there are children items, the editing widget(s) could have changed + if (item->numberOfChildrenItems() > 0) + { + Q_EMIT this->editingWidgetChanged(); + } } diff --git a/smtk/extension/qt/qtDiscreteValueEditor.h b/smtk/extension/qt/qtDiscreteValueEditor.h index d7c377ca08..923c978db8 100644 --- a/smtk/extension/qt/qtDiscreteValueEditor.h +++ b/smtk/extension/qt/qtDiscreteValueEditor.h @@ -35,7 +35,13 @@ public: ~qtDiscreteValueEditor() override; bool useSelectionManger() const { return m_useSelectionManager; } void updateContents(); - QWidget* editingWidget() const; // the internal editing widget (QComboBox) + + /** Tab ordering methods. + * + * The current logic does not support conditional children items. + */ + QWidget* lastEditingWidget() const; // the last internal editing widget + void updateTabOrder(QWidget* /*precedingEditor*/) {} public Q_SLOTS: void onInputValueChanged(); @@ -47,6 +53,9 @@ Q_SIGNALS: /// /brief Signal indicates that the underlying widget's size has been modified void widgetSizeChanged(); + /** \brief Indicates that editing widget changed. */ + void editingWidgetChanged(); + protected Q_SLOTS: protected: diff --git a/smtk/extension/qt/qtInputsItem.cxx b/smtk/extension/qt/qtInputsItem.cxx index 8f91d36a4b..195880fed0 100644 --- a/smtk/extension/qt/qtInputsItem.cxx +++ b/smtk/extension/qt/qtInputsItem.cxx @@ -1222,10 +1222,16 @@ QWidget* qtInputsItem::createInputWidget(int elementIdx, QLayout* childLayout) QObject::connect(editor, SIGNAL(widgetSizeChanged()), this, SIGNAL(widgetSizeChanged())); // editor->setUseSelectionManager(m_useSelectionManager); m_internals->DiscreteEditors.append(editor); - m_internals->m_editors[elementIdx] = editor->editingWidget(); + m_internals->m_editors[elementIdx] = editor->lastEditingWidget(); + QObject::connect( + editor, + &qtDiscreteValueEditor::editingWidgetChanged, + this, + &qtInputsItem::editingWidgetChanged); return editor; } + // (else) QWidget* editorWidget = this->createEditBox(elementIdx, m_widget); m_internals->m_editors[elementIdx] = editorWidget; return editorWidget; diff --git a/smtk/extension/qt/qtReferenceItemEditor.cxx b/smtk/extension/qt/qtReferenceItemEditor.cxx index 1193afdb59..02bf814708 100644 --- a/smtk/extension/qt/qtReferenceItemEditor.cxx +++ b/smtk/extension/qt/qtReferenceItemEditor.cxx @@ -482,11 +482,13 @@ smtk::resource::PersistentObjectPtr qtReferenceItemEditor::object(int index) QWidget* qtReferenceItemEditor::lastEditor() const { + // The current logic does not support children items. return m_internals->m_comboBox; } void qtReferenceItemEditor::updateTabOrder(QWidget* precedingEditor) { + // The current logic does not support children items. if (m_internals->m_comboBox != nullptr) { QWidget::setTabOrder(precedingEditor, m_internals->m_comboBox); @@ -871,4 +873,9 @@ void qtReferenceItemEditor::updateContents() m_itemInfo.baseView()->childrenResized(); Q_EMIT this->widgetSizeChanged(); + + if (item->numberOfChildrenItems() > 0) + { + Q_EMIT this->editingWidgetChanged(); + } } diff --git a/smtk/extension/qt/qtReferenceItemEditor.h b/smtk/extension/qt/qtReferenceItemEditor.h index e810695a6b..e9568fc3c6 100644 --- a/smtk/extension/qt/qtReferenceItemEditor.h +++ b/smtk/extension/qt/qtReferenceItemEditor.h @@ -62,8 +62,13 @@ public: void setOkToCreate(bool val) { m_okToCreate = val; } smtk::resource::PersistentObjectPtr object(int index); + /** Tab ordering methods. + * + * The current logic does not support children items. + */ QWidget* lastEditor() const override; void updateTabOrder(QWidget* precedingEditor) override; + public Q_SLOTS: void updateItemData() override; void highlightItem(int index); -- GitLab