[Tabs] Fixed bug when removing tabs (#15)

src/shared/Modules/Tabs/TabsModule.cpp

@@ -43,11 +43,21 @@ XmlObject TabsModule::toXmlObject()
 {
     XmlObject obj = Module::toXmlObject();
 
-    for (int i = 0; i != contentModules.size(); i++)
+    for (int i = 0; i != tabContents->count(); i++)
     {
         XmlObject tab("tab");
         tab.addAttribute("name", tabNames->item(i)->text());
-        tab.addChild(contentModules.at(i)->toXmlObject());
+        if (auto module = qobject_cast<Module*>(tabContents->widget(i)))
+        {
+            tab.addChild(module->toXmlObject());
+        }
+        else
+        {
+            qCritical() << "Unable to save tab " << tabNames->item(i)->text()
+                        << " due to failed when casting widget to Module*";
+            continue;  // Skip this tab
+        }
+
         obj.addChild(tab);
     }
 
@@ -58,13 +68,8 @@ void TabsModule::fromXmlObject(const XmlObject& xmlObject)
 {
     // Discard previous content
     tabNames->clear();
-    for (int i = contentModules.size() - 1; i >= 0; i--)
-    {
-        auto* module = contentModules[i];
-        contentModules.pop_back();
-        tabContents->removeWidget(tabContents->widget(i));
-        delete module;
-    }
+    for (int i = tabContents->count() - 1; i >= 0; i--)
+        delete tabContents->widget(i);
 
     for (int i = 0; i < xmlObject.childCount(); ++i)
     {
@@ -84,12 +89,14 @@ void TabsModule::fromXmlObject(const XmlObject& xmlObject)
         }
         else
         {
-            // TODO: what should we do?
+            qCritical() << "Failed to instantiate module"
+                        << tabNode.getAttribute("name",
+                                                generateUniqueTabName());
         }
     }
 
     // If the tabs are left empty, add a single tab with an empty module
-    if (contentModules.size() == 0)
+    if (tabNames->count() == 0)
         addTab(generateUniqueTabName(),
                ModulesList::getInstance().instantiateModule(ModuleId::EMPTY));
 
@@ -107,7 +114,6 @@ void TabsModule::addTab(const QString& tabName, Module* module)
     tabNames->addItem(listItem);
 
     tabContents->addWidget(module);
-    contentModules.append(module);
 
     // We set the correct tabName after the widget is added
     tabNames->setCurrentItem(listItem);
@@ -128,45 +134,43 @@ void TabsModule::replaceTab(Module* oldModule, Module* newModule)
     if (oldModule == newModule)
         return;
 
-    int index = contentModules.indexOf(oldModule);
-
-    // Check if the old module is a child
+    // Get the index of the old module
+    int index = tabContents->indexOf(oldModule);
     if (index == -1)
         return;
 
     // Can replace only current tab
-    tabNames->setCurrentItem(tabNames->item(index));
+    // NOTE: This should be useless no?
+    // tabNames->setCurrentItem(tabNames->item(index));
 
-    contentModules.replace(index, newModule);
+    // Replace the module
     tabContents->insertWidget(index, newModule);
-    tabContents->removeWidget(oldModule);
     tabContents->setCurrentWidget(newModule);
+    tabContents->removeWidget(oldModule);
+    delete oldModule;
     tabContents->update();
 
     connect(newModule, &Module::replaceMe, this, &TabsModule::replaceTab);
     connect(newModule, &Module::closeMe, this, &TabsModule::closeTab);
-
-    delete oldModule;
 }
 
 void TabsModule::closeTab(Module* module)
 {
-    int index = contentModules.indexOf(module);
+    int index = tabContents->indexOf(module);
 
     // Check if the given module is a child
     if (index == -1)
         return;
 
     // If there is only one child, then close the entire tabs module
-    if (contentModules.count() == 1)
+    if (tabContents->count() == 1)
     {
         emit closeMe(this);
         return;
     }
 
-    contentModules.removeAt(index);
-    tabContents->removeWidget(tabContents->widget(index));
-    tabNames->takeItem(index);
+    tabContents->removeWidget(module);
+    delete tabNames->item(index);
     tabNames->setCurrentItem(tabNames->item(index == 0 ? 0 : index - 1));
 }
 
@@ -202,8 +206,11 @@ void TabsModule::onMenuNewTabClick()
 
 void TabsModule::onMenuDeleteTabClick()
 {
-    delete contentModules.at(tabNames->currentRow());
-    tabNames->removeItemWidget(tabNames->currentItem());
+    if (auto module = qobject_cast<Module*>(tabContents->currentWidget()))
+        closeTab(module);
+    else
+        qCritical() << "Unable to close tab  due to failed when casting widget "
+                       "to Module*";
 }
 
 void TabsModule::onMenuRenameTabClick()
@@ -231,7 +238,6 @@ void TabsModule::setupUi()
     QBoxLayout* outerLayout = new QBoxLayout(QBoxLayout::TopToBottom, this);
     outerLayout->setContentsMargins(0, 0, 0, 0);
     outerLayout->setSpacing(0);
-
     outerLayout->addWidget(tabNames);
     outerLayout->addWidget(tabContents);
 }

src/shared/Modules/Tabs/TabsModule.h

@@ -53,5 +53,4 @@ private:
 
     QListWidget* tabNames;
     QStackedWidget* tabContents;
-    QList<Module*> contentModules;
 };