diff --git a/data/attribute/attribute_collection/tabOrder.sbt b/data/attribute/attribute_collection/tabOrder.sbt new file mode 100644 index 0000000000000000000000000000000000000000..5dcd81ba0a754d17bf7750ee53dc356888fdc0b9 --- /dev/null +++ b/data/attribute/attribute_collection/tabOrder.sbt @@ -0,0 +1,222 @@ + + + + + + + + + 1 + 2 + + + + 2.71828 + + Child1 + + + + 3.14159 + + Child1 + Child2 + + + + + + + + + + + + 1 + 3.14159 + + + 1 + + + 1 + 2 + 3 + + + + + + + + 3.33,6.66 + + Fourth + + + String data + 2 + + + + + + 1 + + + 1 + + + + + + + + + + + + + string item + + + String data + + + + + + 1 + + + 1 + + 2.71828 + Example + + + Example + + string item + + + + + 1 + + 2.71828 + Example + + + Example + + string item + + + String data + + + + + + + 1 + 99 + 3.14159 + + + 1 + 2.71828 + string item + + + String data + + + + + + + 1 + + 2 + + + 1 + + string item + + + String data + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + 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 0000000000000000000000000000000000000000..20295b5233ba180b279f17f5f6d9a813d0e3a056 --- /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. diff --git a/smtk/extension/qt/qtDiscreteValueEditor.cxx b/smtk/extension/qt/qtDiscreteValueEditor.cxx index a99c8195aad47e6d3bb77e530c686823f431f5be..968ec30b043526e210b574284d76caa2676ab070 100644 --- a/smtk/extension/qt/qtDiscreteValueEditor.cxx +++ b/smtk/extension/qt/qtDiscreteValueEditor.cxx @@ -86,6 +86,12 @@ qtDiscreteValueEditor::~qtDiscreteValueEditor() delete this->Internals; } +QWidget* qtDiscreteValueEditor::lastEditingWidget() const +{ + // The current logic does not support conditional children items. + return this->Internals->m_combo; +} + void qtDiscreteValueEditor::createWidget() { smtk::attribute::ResourcePtr attResource = this->Internals->m_inputItem->attributeResource(); @@ -482,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 4df022109970e36b5cd078461fa0da711a2c17d1..923c978db829a418f6ceb1ef44b91a2387004bd2 100644 --- a/smtk/extension/qt/qtDiscreteValueEditor.h +++ b/smtk/extension/qt/qtDiscreteValueEditor.h @@ -36,6 +36,13 @@ public: bool useSelectionManger() const { return m_useSelectionManager; } void updateContents(); + /** 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(); virtual void updateItemData(); @@ -46,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/qtGroupItem.cxx b/smtk/extension/qt/qtGroupItem.cxx index a334a7032601883c333a600efad22735506fb0dd..a02a5c1cf2bc4db91ecf3aaf87b881f50a04ff1b 100644 --- a/smtk/extension/qt/qtGroupItem.cxx +++ b/smtk/extension/qt/qtGroupItem.cxx @@ -25,6 +25,7 @@ #include #include +#include #include #include #include @@ -33,6 +34,7 @@ #include #include #include +#include #include #include @@ -54,6 +56,7 @@ public: QPointer m_titleLabel; QPointer m_alertLabel; std::map m_itemViewMap; + QWidget* m_precedingEditor = 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_childItems.isEmpty() ? nullptr : m_childItems.last()->lastEditor(); +} + void qtGroupItem::createWidget() { auto item = m_itemInfo.itemAs(); @@ -220,6 +228,9 @@ void qtGroupItem::createWidget() { m_internals->m_titleCheckbox->setVisible(false); } + + // Update tab ordering on next event loop + QTimer::singleShot(0, [this]() { this->updateTabOrder(m_internals->m_precedingEditor); }); } void qtGroupItem::setEnabledState(int state) @@ -303,6 +314,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); @@ -542,6 +554,23 @@ void qtGroupItem::addItemsToTable(int index) return; } + QWidget* precedingEditor = nullptr; + if (index == 0) + { + precedingEditor = m_internals->m_precedingEditor; + } + else if (index == m_internals->ItemsTable->rowCount()) + { + precedingEditor = this->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) { @@ -549,6 +578,10 @@ 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( @@ -590,6 +623,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::onEditingWidgetChanged); + } + this->addChildItem(childItem); if (added == 0) { @@ -617,6 +658,10 @@ void qtGroupItem::addItemsToTable(int index) childItem->setLabelVisible(false); m_internals->ItemsTable->setCellWidget(index, added + 1, childItem->widget()); itemList.push_back(childItem); + + childItem->updateTabOrder(precedingEditor); + precedingEditor = childItem->lastEditor(); + connect( childItem, SIGNAL(widgetSizeChanged()), @@ -630,8 +675,9 @@ void qtGroupItem::addItemsToTable(int index) this, SLOT(onChildItemModified()), static_cast(Qt::AutoConnection | Qt::UniqueConnection)); - } - } + } // if (childItem) + } // for (j) + // 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())) @@ -652,6 +698,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); @@ -784,3 +835,20 @@ void qtGroupItem::onImportFromFile() QMessageBox::warning(m_widget, tr("GroupItem Import Log)"), logger.convertToString().c_str()); } } + +void qtGroupItem::onEditingWidgetChanged() +{ + // Only applicable to QTableWidget + if (m_internals->ItemsTable == nullptr) + { + return; + } + + // Brute force iterate over all qtItem instances + QWidget* precedingEditor = m_internals->m_precedingEditor; + Q_FOREACH (qtItem* item, m_childItems) + { + item->updateTabOrder(precedingEditor); + precedingEditor = item->lastEditor(); + } +} diff --git a/smtk/extension/qt/qtGroupItem.h b/smtk/extension/qt/qtGroupItem.h index b2ebecc4b20be35b3b1c792c65eab3425dc7f8b9..01bcfe5fb2301055104c0b7fe0fde1d9f571564d 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; @@ -44,6 +45,7 @@ protected Q_SLOTS: void onChildWidgetSizeChanged() override; virtual void onChildItemModified(); void onImportFromFile(); + void onEditingWidgetChanged(); protected: void createWidget() override; diff --git a/smtk/extension/qt/qtInputsItem.cxx b/smtk/extension/qt/qtInputsItem.cxx index ead928f347db4fe5492c8b07058f44155b79579e..195880fed0cb0a59e558cdb67e35a2f04bcdf4fe 100644 --- a/smtk/extension/qt/qtInputsItem.cxx +++ b/smtk/extension/qt/qtInputsItem.cxx @@ -241,8 +241,47 @@ public: QPointer m_expressionResultLineEdit; 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_usingExpression) + { + return m_internals->m_expressionCombo; + } + else if (m_internals->m_editors.isEmpty()) + { + return nullptr; + } + return m_internals->m_editors.last(); +} + +void qtInputsItem::updateTabOrder(QWidget* precedingEditor) +{ + QWidget* previousEd = precedingEditor; + + // 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? + { + QWidget::setTabOrder(previousEd, ed); + } + previousEd = ed; + } +} + qtItem* qtInputsItem::createItemWidget(const qtAttributeItemInfo& info) { // So we support this type of item? @@ -726,6 +765,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; @@ -919,6 +959,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)), @@ -939,6 +980,7 @@ void qtInputsItem::updateUI() { return; } + m_internals->m_editors.clear(); m_widget = new QFrame(this->parentWidget()); m_widget->setObjectName(dataObj->name().c_str()); @@ -992,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()) @@ -1164,6 +1210,10 @@ QWidget* qtInputsItem::createInputWidget(int elementIdx, QLayout* childLayout) { return nullptr; } + while (m_internals->m_editors.count() <= elementIdx) + { + m_internals->m_editors << nullptr; + } if (item->isDiscrete()) { @@ -1172,9 +1222,19 @@ 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->lastEditingWidget(); + QObject::connect( + editor, + &qtDiscreteValueEditor::editingWidgetChanged, + this, + &qtInputsItem::editingWidgetChanged); return editor; } - return this->createEditBox(elementIdx, m_widget); + + // (else) + QWidget* editorWidget = this->createEditBox(elementIdx, m_widget); + m_internals->m_editors[elementIdx] = editorWidget; + return editorWidget; } QFrame* qtInputsItem::createExpressionRefFrame() @@ -1227,6 +1287,7 @@ void qtInputsItem::displayExpressionWidget(bool checkstate) { return; } + m_internals->m_usingExpression = checkstate; ResourcePtr sourceAttResource = inputitem->attribute()->attributeResource(); @@ -1343,8 +1404,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 (QObject::sender() && widgetChanged) + { + QTimer::singleShot(0, [this]() { Q_EMIT this->editingWidgetChanged(); }); + } } void qtInputsItem::onExpressionReferenceChanged() @@ -1684,6 +1752,7 @@ QWidget* qtInputsItem::createIntWidget( { editBox->setText(vitem->valueAsString(elementIdx).c_str()); } + return editBox; } diff --git a/smtk/extension/qt/qtInputsItem.h b/smtk/extension/qt/qtInputsItem.h index 4d9f329a1f54ce2e7e71fac8c53649a947accd31..784e538a65dca8fae59cd234a59f09dab25b3ba0 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 updateTabOrder(QWidget* precedingEditor) override; + public Q_SLOTS: void setOutputOptional(int); void onExpressionReferenceChanged(); diff --git a/smtk/extension/qt/qtItem.h b/smtk/extension/qt/qtItem.h index 8f33d47a6313039156743e13bfee39be0594d7d2..cd77891499332f9df7a0ffd5b564ae3776d661c7 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; } + /** \brief Sets previous widget for tabbing order */ + virtual void updateTabOrder(QWidget* /*precedingEditor*/) {} + 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 @@ -116,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() { ; } diff --git a/smtk/extension/qt/qtReferenceItemEditor.cxx b/smtk/extension/qt/qtReferenceItemEditor.cxx index 991d88f708e14b6f71c8b735c5157feb84821cd6..02bf81470818444500ddb8c8a5b4a2d10f6082a4 100644 --- a/smtk/extension/qt/qtReferenceItemEditor.cxx +++ b/smtk/extension/qt/qtReferenceItemEditor.cxx @@ -480,6 +480,21 @@ smtk::resource::PersistentObjectPtr qtReferenceItemEditor::object(int index) return selectedObj; } +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); + } +} + void qtReferenceItemEditor::highlightItem(int index) { // Are we dealing with the create new option @@ -858,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 8ebcd3f2cd6aff446be5dee9b8b93c2ccdd1dbc8..e9568fc3c68ffbb9d3ed77762300a9f1b371d21a 100644 --- a/smtk/extension/qt/qtReferenceItemEditor.h +++ b/smtk/extension/qt/qtReferenceItemEditor.h @@ -61,6 +61,14 @@ public: void setDefinitionForCreation(smtk::attribute::DefinitionPtr& def); 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);