PR: Create a separate window when undocking plugins #3824

dalthviz · 2016-12-10T18:46:51Z

ccordoba12 · 2016-12-11T00:42:13Z

Please update with the latest changes in the 3.x branch to fix the errors in CircleCI

ccordoba12 · 2016-12-11T21:10:35Z

spyder/app/mainwindow.py

@@ -2209,6 +2210,15 @@ def __update_fullscreen_action(self):
        self.fullscreen_action.setIcon(icon)

    @Slot()
+    def undock_editor(self):
+        """Open a new window instance of the Editor instead of undoking it."""


undoking is misspelled here. It's undocking

ccordoba12 · 2016-12-11T21:12:40Z

There's one thing that worries me about this change. What happens when users press Ctrl+Shift+E to move to the Editor?

I think that makes the Editor pane to appear in the main window, right?

dalthviz · 2016-12-11T22:14:59Z

If you are with the focus in the main window, yes it makes appear the Editor pane. If you, in the other hand, have the focus in a new window it doesn't appear.

ccordoba12 · 2016-12-12T21:01:58Z

If you are with the focus in the main window, yes it makes appear the Editor pane

Then we need to fix this, and make Ctrl+Shift+E to move to the new window, if the pane is hidden and an Editor window is visible.

We also need to prevent to more than window for this to make sense :-)

ccordoba12 · 2017-01-08T21:12:42Z

Sorry @dalthviz for my big delay on reviewing this one. I hope to do it after we release 3.1 :-)

goanpeca · 2017-02-17T21:13:09Z

@dalthviz please rebase or merge with the latest 3,x to fix conflicts

ccordoba12 · 2017-06-21T01:22:40Z

spyder/app/mainwindow.py

+            self.editor.toggle_view_action.setChecked(False)
+            self.editor.dockwidget.setFloating(False)
+        self.editor.undocked = False
+        self.editor.get_current_editorstack().new_window = False


@dalthviz, does this method need to be here in the main window? As far as I can see, it doesn't use any attribute/method of the window, so I think we can safely move it to the Editor plugin file.

ccordoba12 · 2017-06-21T01:23:35Z

spyder/app/mainwindow.py

@@ -799,6 +799,7 @@ def create_edit_action(text, tr_text, icon):
        from spyder.plugins.editor import Editor
        self.editor = Editor(self)
        self.editor.register_plugin()
+        self.editor.dockwidget.topLevelChanged.connect(self.undock_editor)


This line can also be moved to the Editor plugin. We can make it the last line of its register_plugin method.

ccordoba12 · 2017-06-21T01:25:13Z

@dalthviz, please merge with 3.x and finish this one for Spyder 3.2. I think it's almost ready ;-)

ccordoba12 · 2017-06-21T20:53:12Z

@dalthviz, please create an animated gif so that @goanpeca and me could see how this is working.

dalthviz · 2017-06-21T20:59:34Z

Preview:

ccordoba12 · 2017-06-21T21:05:43Z

@goanpeca, what do you think? There is an action in the Editor options menu to undock it so users can move next to other plugins.

ccordoba12 · 2017-06-21T21:06:31Z

Problem is this is not the same behavior for the rest of plugins.

ccordoba12 · 2017-06-22T13:52:01Z

@dalthviz, this needs another merge with 3.x.

goanpeca · 2017-06-22T14:27:03Z

@ccordoba12 yeah, this change would be a bit inconsistent with the rest and is better suited for 4.x

ccordoba12 · 2017-06-22T14:37:24Z

Ok, you're right. @dalthviz, please rebase this PR in top of master and keep working on it, so that when users press the undock button they see a new window for every plugin, and not just the Editor.

This functionality could go to plugins/base.py in master.

…or window. Disabled shortcut for the editor when working in a new window.

ccordoba12 · 2017-10-09T16:32:21Z

the only think that I don't like is that when undocking plugins are somehow small and square

Could you post a screenshot to see what you mean?

rlaverde

Looking great, and pretty concise (I think that the this PR will be longer 🙈)
My only concern is the size of the window when undocking a plugin.

rlaverde · 2017-10-09T15:40:49Z

spyder/api/plugins.py

@@ -88,7 +94,12 @@ def initialize_plugin(self):
        It must be run at the end of __init__
        """
        self.create_toggle_view_action()
-        self.plugin_actions = self.get_plugin_actions()
+        self.create_undock_action()
+        self.plugin_actions = self.get_plugin_actions() + [None,


Please use MENU_SEPARATOR (spyder.utils.qthelpers) instead of None

rlaverde · 2017-10-09T15:43:09Z

spyder/api/plugins.py

+    def refresh_actions(self):
+        """Clear the menu of the plugin and add the actions."""
+        self.menu.clear()
+        self.plugin_actions = self.get_plugin_actions() + [None,


Please also change None for MENU_SEPARATOR here.

rlaverde · 2017-10-09T15:49:39Z

spyder/plugins/base.py

@@ -109,7 +112,7 @@ def switch_to_plugin(self):
            self.toggle_view_action.setChecked(True)
        self.visibility_changed(True)

-    def plugin_closed(self):
+    def plugin_closed(self, event):


I think you revert the changes to this method, so It no longer needs to receive the event.

Please remove the event argument.

rlaverde · 2017-10-09T16:03:09Z

spyder/plugins/editor.py

+        Reimplemented method to desactivate shortcut when
+        opening a new window.
+        """
+        if len(self.editorwindows) == 0 or self.dockwidget.isVisible():


You could use: if not self.editorwindows or ..., It will evaluate to True when len is 0

rlaverde · 2017-10-09T16:08:54Z

spyder/plugins/console.py

-        # Add actions to context menu
-        add_actions(self.shell.menu, plugin_actions)
-
+                          option_menu, None, quit_action, self.undock_action]


MENU_SEPARATOR

rlaverde · 2017-10-09T16:43:26Z

spyder/widgets/dock.py

@@ -153,10 +153,10 @@ def show_nontab_menu(self, event):

 class SpyderDockWidget(QDockWidget):
    """Subclass to override needed methods"""
-    plugin_closed = Signal()
+    plugin_closed = Signal(object)


Revert this, this signal doesn't need to send the event

rlaverde · 2017-10-09T16:43:31Z

spyder/widgets/dock.py

@@ -173,7 +173,7 @@ def closeEvent(self, event):
        Reimplement Qt method to send a signal on close so that "Panes" main
        window menu can be updated correctly
        """
-        self.plugin_closed.emit()
+        self.plugin_closed.emit(event)


Revert this, this signal doesn't need to send the event

rlaverde · 2017-10-09T16:45:20Z

spyder/widgets/dock.py


    def __init__(self, title, parent):
-        super(SpyderDockWidget, self).__init__(title, parent)
+        super(SpyderDockWidget, self).__init__(title)


Why this change is necessary?

I think this change is not necessary. I will revert this 👍

rlaverde · 2017-10-09T16:46:05Z

spyder/widgets/editor.py

@@ -1339,8 +1341,12 @@ def __get_split_actions(self):
                triggered=lambda: self.split_horizontally.emit())
        self.close_action = create_action(self, _("Close this panel"),
                icon=ima.icon('close_panel'), triggered=self.close)
-        return [None, self.newwindow_action, None,
-                self.versplit_action, self.horsplit_action, self.close_action]
+        actions = [None, self.undock_action, None, self.versplit_action,


MENU_SEPARATOR

rlaverde · 2017-10-09T16:53:24Z

spyder/widgets/explorer.py

        QWidget.__init__(self, parent)

        # Widgets
        self.treewidget = ExplorerTreeWidget(self, show_cd_only=show_cd_only)
        button_previous = QToolButton(self)
        button_next = QToolButton(self)
        button_parent = QToolButton(self)
-        self.button_menu = QToolButton(self)
-        menu = QMenu(self)
+        if options_button:


I think this could be changed for a pythonic shortcut:

self.button_menu = options_button or QToolButton(self)

rlaverde · 2017-10-09T17:10:03Z

Could you post a screenshot to see what you mean?

If you see the video that @dalthviz posted, after undocking the plugin is necessary to resize the window because it is somehow small

ccordoba12 · 2017-10-11T16:56:24Z

spyder/plugins/base.py

+    def create_undock_action(self):
+        """Create the undock action for the plugin."""
+        self.undock_action = create_action(self,
+                                           _("Undock the plugin"),


Let's change this to simply Undock, instead of Undock the plugin.

ccordoba12 · 2017-10-11T16:57:09Z

spyder/plugins/base.py

+        self.undock_action.setDisabled(True)
+
+
+class PluginMainWindow(QMainWindow):


Please move this class to be before BasePluginWidget

ccordoba12 · 2017-10-11T17:20:37Z

spyder/api/plugins.py

+        self.options_button = create_toolbutton(self, text=_('Options'),
+                                                icon=ima.icon('tooloptions'))
+        self.options_button.setPopupMode(QToolButton.InstantPopup)
+        self.menu = QMenu(self)


menu is too generic. Please change it to options_menu.

ccordoba12 · 2017-10-11T17:23:07Z

@dalthviz, I left some simple comments, then I'll merge.

…nu'.

ccordoba12 · 2017-10-12T20:17:22Z

@dalthviz, please squash the last two commits into one with the message "Fix tests".

ccordoba12

Another great improvement @dalthviz!! Thanks a lot for all your work and patience with this one ;-)

ccordoba12 reviewed Dec 11, 2016

View reviewed changes

ccordoba12 added this to the v3.2 milestone Jan 8, 2017

goanpeca assigned dalthviz Jan 13, 2017

ccordoba12 modified the milestones: v3.2, v3.3 Feb 14, 2017

ccordoba12 modified the milestones: v3.2, v4.0beta2 Jun 21, 2017

ccordoba12 reviewed Jun 21, 2017

View reviewed changes

ccordoba12 modified the milestones: v4.0beta1, v3.2 Jun 22, 2017

dalthviz added 3 commits July 13, 2017 22:47

New behavior when undoking Editor window.

6f45e71

Fixed misspell.

ece67e1

Added undock function. Added restriction to create more than one edit…

238ffd6

…or window. Disabled shortcut for the editor when working in a new window.

rlaverde reviewed Oct 9, 2017

View reviewed changes

jnsebgosselin mentioned this pull request Oct 10, 2017

Hide the titlebar from panes/dockwidgets if locked #2855

Closed

Change None for MENU_SEPARATOR, revert some unnecessary changes.

5ebbc84

dalthviz added User Interface (UI) User Experience (UX) type:Enhancement labels Oct 10, 2017

rlaverde approved these changes Oct 11, 2017

View reviewed changes

ccordoba12 reviewed Oct 11, 2017

View reviewed changes

Merge branch 'master' into fixes_issue_3790_2

f9168c4

dalthviz mentioned this pull request Oct 12, 2017

Changes for compatibility with new undocking behavior of Spyder spyder-ide/spyder-notebook#121

Closed

Change 'Undock the plugin' to 'Undock'. Change 'menu' for 'options_me…

c186de6

…nu'.

Fix tests.

3437f75

dalthviz force-pushed the fixes_issue_3790_2 branch from f8e56a0 to 3437f75 Compare October 12, 2017 21:36

ccordoba12 approved these changes Oct 14, 2017

View reviewed changes

ccordoba12 changed the title ~~PR: New behavior when undocking plugins~~ PR: Create a separate window when undocking plugins Oct 14, 2017

ccordoba12 merged commit c501a4c into spyder-ide:master Oct 14, 2017

jnsebgosselin mentioned this pull request Oct 16, 2017

Spyder crash when dragging a plugin with the mouse #5486

Closed

dalthviz mentioned this pull request Oct 18, 2017

PR: Show undock action spyder-ide/spyder-unittest#84

Merged

jnsebgosselin mentioned this pull request Oct 20, 2017

Enhancements proposal related to the undocking and docking of plugins in the Main Window #5515

Closed

jnsebgosselin mentioned this pull request Nov 14, 2017

PR: Prevent Editor dockwidget to become visible when clicking on the outline explorer if the Editor is in a separate window #5742

Merged

jnsebgosselin mentioned this pull request Oct 5, 2018

Restore option to open a new editor window #8011

Closed

		self.undock_action.setDisabled(True)


		class PluginMainWindow(QMainWindow):

PR: Create a separate window when undocking plugins #3824

PR: Create a separate window when undocking plugins #3824

Conversation

dalthviz commented Dec 10, 2016 • edited by ccordoba12

ccordoba12 commented Dec 11, 2016

Choose a reason for hiding this comment

ccordoba12 commented Dec 11, 2016

dalthviz commented Dec 11, 2016

ccordoba12 commented Dec 12, 2016

ccordoba12 commented Jan 8, 2017

goanpeca commented Feb 17, 2017

ccordoba12 Jun 21, 2017 • edited

Choose a reason for hiding this comment

ccordoba12 Jun 21, 2017 • edited

Choose a reason for hiding this comment

ccordoba12 commented Jun 21, 2017

ccordoba12 commented Jun 21, 2017

dalthviz commented Jun 21, 2017

ccordoba12 commented Jun 21, 2017

ccordoba12 commented Jun 21, 2017

ccordoba12 commented Jun 22, 2017

goanpeca commented Jun 22, 2017

ccordoba12 commented Jun 22, 2017

ccordoba12 commented Oct 9, 2017

rlaverde left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rlaverde commented Oct 9, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ccordoba12 commented Oct 11, 2017

ccordoba12 commented Oct 12, 2017

ccordoba12 left a comment

Choose a reason for hiding this comment

dalthviz commented Dec 10, 2016 •

edited by ccordoba12

ccordoba12 Jun 21, 2017 •

edited

ccordoba12 Jun 21, 2017 •

edited