PR: Fix several issues in the Layout plugin

spyder/plugins/layout/plugin.py

@@ -9,6 +9,7 @@
 """
 # Standard library imports
 import configparser as cp
+from functools import lru_cache
 import logging
 import os
 
@@ -95,7 +96,6 @@ def get_icon(cls):
 
     def on_initialize(self):
         self._last_plugin = None
-        self._first_spyder_run = False
         self._fullscreen_flag = None
         # The following flag remember the maximized state even when
         # the window is in fullscreen mode:
@@ -105,6 +105,15 @@ def on_initialize(self):
         self._saved_normal_geometry = None
         self._state_before_maximizing = None
         self._interface_locked = self.get_conf('panes_locked', section='main')
+        # The following flag is used to apply the window settings only once
+        # during the first run
+        self._window_settings_applied_on_first_run = False
+
+        # If Spyder has already been run once, this option needs to be False.
+        # Note: _first_spyder_run needs to be accessed at least once in this
+        # method to be computed at startup.
+        if not self._first_spyder_run:
+            self.set_conf("first_time", False)
 
         # Register default layouts
         self.register_layout(self, SpyderLayout)
@@ -234,6 +243,25 @@ def on_mainwindow_visible(self):
 
     # ---- Private API
     # -------------------------------------------------------------------------
+    @property
+    @lru_cache
+    def _first_spyder_run(self):
+        """
+        Check if Spyder is run for the first time.
+
+        Notes
+        -----
+        * We declare this as a property to prevent reassignments in other
+          places of this class.
+        * It only needs to be computed once at startup (i.e. it needs to be
+          accessed in on_initialize).
+        """
+        # We need to do this double check because we were not using the
+        # "first_time" option in 6.0 and older versions.
+        return (
+            self.get_conf("first_time", True) and self.get_conf("names") == []
+        )
+
     def _get_internal_dockable_plugins(self):
         """Get the list of internal dockable plugins"""
         return get_class_values(DockablePlugins)
@@ -383,11 +411,9 @@ def setup_layout(self, default=False):
         settings = self.load_window_settings(prefix, default)
         hexstate = settings[0]
 
-        self._first_spyder_run = False
         if hexstate is None:
             # First Spyder execution:
             self.main.setWindowState(Qt.WindowMaximized)
-            self._first_spyder_run = True
             self.setup_default_layouts(DefaultLayouts.SpyderLayout, settings)
 
             # Restore the original defaults. This is necessary, for instance,
@@ -428,9 +454,7 @@ def setup_default_layouts(self, layout_id, settings):
         main = self.main
         main.setUpdatesEnabled(False)
 
-        first_spyder_run = bool(self._first_spyder_run)  # Store copy
-
-        if first_spyder_run:
+        if self._first_spyder_run:
             self.set_window_settings(*settings)
         else:
             if self._last_plugin:
@@ -451,8 +475,8 @@ def setup_default_layouts(self, layout_id, settings):
         # Apply selected layout
         layout.set_main_window_layout(self.main, self.get_dockable_plugins())
 
-        if first_spyder_run:
-            self._first_spyder_run = False
+        if self._first_spyder_run:
+            self.set_conf("first_time", False)
         else:
             self.main.setMinimumWidth(min_width)
             self.main.setMaximumWidth(max_width)
@@ -598,16 +622,28 @@ def get_window_settings(self):
     def set_window_settings(self, hexstate, window_size, pos, is_maximized,
                             is_fullscreen):
         """
-        Set window settings Symetric to the 'get_window_settings' accessor.
+        Set window settings.
+
+        Symetric to the 'get_window_settings' accessor.
         """
-        main = self.main
-        main.setUpdatesEnabled(False)
-        self.window_size = QSize(window_size[0],
-                                 window_size[1])  # width, height
-        self.window_position = QPoint(pos[0], pos[1])  # x,y
-        main.setWindowState(Qt.WindowNoState)
-        main.resize(self.window_size)
-        main.move(self.window_position)
+        # Prevent calling this method multiple times on first run because it
+        # causes main window flickering on Windows and Mac.
+        # Fixes spyder-ide/spyder#15074
+        if (
+            self._window_settings_applied_on_first_run
+            and self._first_spyder_run
+        ):
+            return
+
+        self.main.setUpdatesEnabled(False)
+
+        # Restore window properties
+        self.window_size = QSize(
+            window_size[0], window_size[1] # width, height
+        )
+        self.window_position = QPoint(pos[0], pos[1]) # x, y
+        self.main.resize(self.window_size)
+        self.main.move(self.window_position)
 
         # Window layout
         if hexstate:
@@ -636,6 +672,9 @@ def set_window_settings(self, hexstate, window_size, pos, is_maximized,
         elif is_maximized:
             self.main.setWindowState(Qt.WindowMaximized)
 
+        # Settings applied at startup
+        self._window_settings_applied_on_first_run = True
+
         self.main.setUpdatesEnabled(True)
 
     def save_current_window_settings(self, prefix, section='main',
@@ -1243,3 +1282,8 @@ def tabify_new_plugins(self):
         for plugin in self.get_dockable_plugins():
             if plugin.get_conf('first_time', True):
                 self.tabify_plugin(plugin, Plugins.Console)
+
+                # This is necessary in case the plugin doesn't set its TABIFY
+                # constant
+                plugin.set_conf("enable", True)
+                plugin.set_conf("first_time", False)

spyder/plugins/layout/widgets/dialog.py

@@ -12,15 +12,30 @@
 # Third party imports
 from qtpy.QtCore import QAbstractTableModel, QModelIndex, QSize, Qt
 from qtpy.compat import from_qvariant, to_qvariant
-from qtpy.QtWidgets import (QAbstractItemView, QDialog,
-                            QDialogButtonBox, QGroupBox, QHBoxLayout,
-                            QPushButton, QTableView, QVBoxLayout)
+from qtpy.QtWidgets import (
+    QAbstractItemView,
+    QDialog,
+    QDialogButtonBox,
+    QHBoxLayout,
+    QMessageBox,
+    QLabel,
+    QTableView,
+    QVBoxLayout,
+)
 
 # Local imports
+from spyder.api.widgets.mixins import SpyderWidgetMixin
 from spyder.api.widgets.comboboxes import SpyderComboBox
 from spyder.api.widgets.dialogs import SpyderDialogButtonBox
 from spyder.config.base import _
 from spyder.py3compat import to_text_string
+from spyder.utils.stylesheet import AppStyle, PANES_TOOLBAR_STYLESHEET
+
+
+class LayoutSettingsToolButtons:
+    MoveUp = 'move_up'
+    MoveDown = 'move_down'
+    Remove = 'remove'
 
 
 class LayoutModel(QAbstractTableModel):
@@ -135,7 +150,8 @@ def set_row(self, rownum, value):
 
 
 class LayoutSaveDialog(QDialog):
-    """ """
+    """Dialog to save a custom layout with a given name."""
+
     def __init__(self, parent, order):
         super(LayoutSaveDialog, self).__init__(parent)
 
@@ -147,6 +163,10 @@ def __init__(self, parent, order):
         self.combo_box.addItems(order)
         self.combo_box.setEditable(True)
         self.combo_box.clearEditText()
+        self.combo_box.lineEdit().setPlaceholderText(
+            _("Give a name to your layout")
+        )
+
         self.button_box = SpyderDialogButtonBox(
             QDialogButtonBox.Ok | QDialogButtonBox.Cancel, Qt.Horizontal, self
         )
@@ -155,8 +175,8 @@ def __init__(self, parent, order):
 
         # widget setup
         self.button_ok.setEnabled(False)
-        self.dialog_size = QSize(300, 100)
-        self.setWindowTitle('Save layout as')
+        self.dialog_size = QSize(350, 100)
+        self.setWindowTitle(_('Save layout as'))
         self.setModal(True)
         self.setMinimumSize(self.dialog_size)
         self.setFixedSize(self.dialog_size)
@@ -180,8 +200,12 @@ def check_text(self, text):
             self.button_ok.setEnabled(True)
 
 
-class LayoutSettingsDialog(QDialog):
+class LayoutSettingsDialog(QDialog, SpyderWidgetMixin):
     """Layout settings dialog"""
+
+    # Dummy variable to avoid a warning
+    CONF_SECTION = ""
+
     def __init__(self, parent, names, ui_names, order, active, read_only):
         super(LayoutSettingsDialog, self).__init__(parent)
         # variables
@@ -193,25 +217,51 @@ def __init__(self, parent, names, ui_names, order, active, read_only):
         self.active = active
         self.read_only = read_only
 
+        # To style the tool buttons
+        self.setStyleSheet(PANES_TOOLBAR_STYLESHEET.to_string())
+
         # widgets
-        self.button_move_up = QPushButton(_('Move up'))
-        self.button_move_down = QPushButton(_('Move down'))
-        self.button_delete = QPushButton(_('Delete layout'))
+        description_label = QLabel(_("Reorder or delete your saved layouts"))
+        description_label.setAlignment(Qt.AlignCenter)
+        description_label.setWordWrap(True)
+
+        self.button_move_up = self.create_toolbutton(
+            LayoutSettingsToolButtons.MoveUp,
+            tip=_('Move up'),
+            icon=self.create_icon('1uparrow'),
+            triggered=lambda: self.move_layout(True),
+            register=False,
+        )
+        self.button_move_down = self.create_toolbutton(
+            LayoutSettingsToolButtons.MoveDown,
+            tip=_('Move down'),
+            icon=self.create_icon('1downarrow'),
+            triggered=lambda: self.move_layout(False),
+            register=False,
+        )
+        self.button_delete = self.create_toolbutton(
+            LayoutSettingsToolButtons.Remove,
+            tip=_('Delete layout'),
+            icon=self.create_icon('editclear'),
+            triggered=self.delete_layout,
+            register=False,
+        )
+
         self.button_box = SpyderDialogButtonBox(
             QDialogButtonBox.Ok | QDialogButtonBox.Cancel, Qt.Horizontal, self
         )
-        self.group_box = QGroupBox(_("Layout display and order"))
+
         self.table = QTableView(self)
-        self.ok_button = self.button_box.button(QDialogButtonBox.Ok)
-        self.cancel_button = self.button_box.button(QDialogButtonBox.Cancel)
-        self.cancel_button.setDefault(True)
-        self.cancel_button.setAutoDefault(True)
 
         # widget setup
-        self.dialog_size = QSize(300, 200)
+        self.dialog_size = QSize(320, 350)
         self.setMinimumSize(self.dialog_size)
         self.setFixedSize(self.dialog_size)
-        self.setWindowTitle('Layout Settings')
+        self.setWindowTitle(_("Layouts display and order"))
+
+        cancel_button = self.button_box.button(QDialogButtonBox.Cancel)
+        cancel_button.setDefault(True)
+        cancel_button.setAutoDefault(True)
 
         self.table.setModel(
             LayoutModel(self.table, names, ui_names, order, active, read_only))
@@ -235,23 +285,22 @@ def __init__(self, parent, names, ui_names, order, active, read_only):
         buttons_layout.addStretch()
         buttons_layout.addWidget(self.button_delete)
 
-        group_layout = QHBoxLayout()
-        group_layout.addWidget(self.table)
-        group_layout.addLayout(buttons_layout)
-        self.group_box.setLayout(group_layout)
+        order_layout = QHBoxLayout()
+        order_layout.addWidget(self.table)
+        order_layout.addLayout(buttons_layout)
 
         layout = QVBoxLayout()
-        layout.addWidget(self.group_box)
+        layout.addWidget(description_label)
+        layout.addSpacing(2 * AppStyle.MarginSize)
+        layout.addLayout(order_layout)
+        layout.addSpacing(2 * AppStyle.MarginSize)
         layout.addWidget(self.button_box)
 
         self.setLayout(layout)
 
         # signals and slots
         self.button_box.accepted.connect(self.accept)
         self.button_box.rejected.connect(self.close)
-        self.button_delete.clicked.connect(self.delete_layout)
-        self.button_move_up.clicked.connect(lambda: self.move_layout(True))
-        self.button_move_down.clicked.connect(lambda: self.move_layout(False))
         self.table.model().dataChanged.connect(
            lambda: self.selection_changed(None, None))
         self._selection_model.selectionChanged.connect(
@@ -273,29 +322,43 @@ def __init__(self, parent, names, ui_names, order, active, read_only):
     def delete_layout(self):
         """Delete layout from the config."""
         names, ui_names, order, active, read_only = (
-            self.names, self.ui_names, self.order, self.active, self.read_only)
+            self.names, self.ui_names, self.order, self.active, self.read_only
+        )
         row = self.table.selectionModel().currentIndex().row()
         ui_name, name, state = self.table.model().row(row)
 
+        answer = QMessageBox.question(
+            self,
+            _("Remove layout"),
+            _("Do you want to remove the layout '<b>{}</b>'?").format(ui_name),
+            QMessageBox.Yes | QMessageBox.No
+        )
+
+        if not answer:
+            return
+
         if name not in read_only:
-            name = from_qvariant(
-                self.table.selectionModel().currentIndex().data(),
-                to_text_string)
             if ui_name in ui_names:
                 index = ui_names.index(ui_name)
             else:
                 # In case nothing has focus in the table
                 return
+
             if index != -1:
-                order.remove(ui_name)
-                names.remove(ui_name)
+                order.remove(name)
+                names.remove(name)
                 ui_names.remove(ui_name)
+
                 if name in active:
-                    active.remove(ui_name)
+                    active.remove(name)
+
                 self.names, self.ui_names, self.order, self.active = (
-                    names, ui_names, order, active)
+                    names, ui_names, order, active
+                )
                 self.table.model().set_data(
-                    names, ui_names, order, active, read_only)
+                    names, ui_names, order, active, read_only
+                )
+
                 index = self.table.model().index(0, 0)
                 self.table.setCurrentIndex(index)
                 self.table.setFocus()