Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add drag support for dockwidgets sharing same position #2369

Merged
merged 8 commits into from
May 3, 2015
Merged

Add drag support for dockwidgets sharing same position #2369

merged 8 commits into from
May 3, 2015

Conversation

goanpeca
Copy link
Member

Description

This related to dockwidgets that are tabified. Dockwidgets, when tabified, are put inside a QTabBar, but there is no access method to this bar, so to get it and process the event an eventFilter plus some search is needed.

I attach a video so you get the idea.

@Nodd
Copy link
Contributor

Nodd commented Apr 26, 2015

Nice ! Is it you playing the guitar on the video ? 😄
What would you put in the contextual menu ? Entries for closing/setfloating/maximizing ? I think it would be helpful only it the titlebars are hidden as per #2345 (comment)

@goanpeca
Copy link
Member Author

Hehe, no, the software I use puts the audio in, but i do play the guitar actually :-)

Now ... well I am considering two things.. if ALL widgets have a corner menu (most do on the upper right corner...) then ybe the options should go there... or also in the tab if right clicking...

I guess is natural to try to right click on a tab...

@Nodd
Copy link
Contributor

Nodd commented Apr 26, 2015

It's natural to right click but I'd expect to see options about tab management, not about the content of the page. I don't know, maybe people would use it anyway...

@goanpeca
Copy link
Member Author

Yes that is what I want, to put tab related stuff... move to right, move to right end..., move to left, move to left end..., close tab, float panel... etc

@Nodd
Copy link
Contributor

Nodd commented Apr 27, 2015

Honnestly I'm not sure the menu is worth it. In FIrefox I regularely have more than 50 tabs opened and it's a mess to manage, but in spyder I have at most 5 dockpanels at the same place, so actions like move to left or move to end are somewhat useless.

@goanpeca
Copy link
Member Author

I see what you mean, then this is almost ready :p

@ccordoba12
Copy link
Member

First let me say that this is terrific addition!! Right now the way to reorganize dock widgets is to undock and dock them again, so this improves things by an order of magnitude!!

I also agree with @Nodd, I think adding a contextual menu is too much. Let's leave things simple for now :-)

@goanpeca
Copy link
Member Author

Ok, I added some docstrings and is ready for a thorough revision ;-) @ccordoba12

Update

I have a small bug I need to fix first

@goanpeca
Copy link
Member Author

@ccordoba12 this one is good to go, and easier to check (than the editor ine... that is still WIP)

@goanpeca goanpeca changed the title [WIP] Add drag support for dockwidgets sharing same position Add drag support for dockwidgets sharing same position Apr 27, 2015
@@ -124,6 +126,14 @@ def __init__(self, *args, **kwargs):
if sys.platform == 'darwin':
self.setStyleSheet(self.DARWIN_STYLE)

# Needed for the installation of the event filter
self.title = args[0]
self.main = args[1]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is args coming from?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when create_dockwidget is called the args passed are the title and the parent (the main window in this case)

I agree is not so clear, but I am not so sure how to make it better...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But we have get_plugin_title and self.main already. I don't get it ;-)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But those things belong to the plugin, not to the dockwidget,

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. Then you need to redefine the init method of SpyderDockWidget to accept the right args, i.e. something like

def __init__(self, title, main, **kwargs):
    super(SpyderDockWidget, self).__init__(**kwargs)
    self.title = title
    self.main = main
    ...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@goanpeca
Copy link
Member Author

@ccordoba12 added corrections.

With regard to the question of the dockwidget... SpyderPlugin has a method create_dockwidget that uses SpyderDockwidget but the methods are from the plugin (SpyderPlugin) not from the widget, that is why I need to store those values when an instance if SpyderDockwidget is created...

def tab_moved(self, event):
"""Method called when a tab from a QTabBar has been moved."""
# If the left button isn't pressed anymore then return
if not event.buttons() & Qt.LeftButton:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not a regular and, instead of & here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm True, proabably a leftover form the C++ code I took as example

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No I checked... buttons is an Or combination so I need the Binary And there...

http://doc.qt.io/qt-4.8/qmouseevent.html#buttons

self.moving = False

def move_tab(self, start_tab, end_tab):
"""Move a tab from a start to and end position."""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to an end, not to and end :-)

end_plugin = self._get_plugin(end_tab)

start = plugins.index(start_plugin)
end = plugins.index(end_plugin)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about start_idx and end_idx for these variable names? I think they should be clearer ;-)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure I can change...

@goanpeca
Copy link
Member Author

goanpeca commented May 3, 2015

I renamed variables to be more consistent with the other PR (editor draggable tabs) and added support to call context menus if needed. For the moment the methods are empty, but I added them in case they are useful in the future.

I also included the cursor fix so that it compensates when the moved tabs have different size.

I made suggested corrections and everything now is working as expected :-), so if @ccordoba12 can check and approve, this one is ready.


def _get_plugins(self):
"""
Get a list of all the plugins references in the QTabBar to which this
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please change this to Get a list of all plugin references in the QTabbar ...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure 👍

@ccordoba12
Copy link
Member

Nothing more from me to say :-) Please make my suggested changes so I can start testing.

@goanpeca
Copy link
Member Author

goanpeca commented May 3, 2015

@ccordoba12 done


def show_nontab_menu(self):
"""Show the context menu assigned to nontabs section."""
pass
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Las thing, I think I have the perfect suggestion for this entry

self.main.createPopupMenu()

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ehh ? what does it do?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It shows the same context menu as when you do a right click over a toolbar.

Essentially, It shows the Help menu but with a few entries.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it does not work...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok fixed it :), now this will only popup when clicking in an area where no tabs are present...

In this image I right clicked to the right of the File explorer Tab

image

@goanpeca
Copy link
Member Author

goanpeca commented May 3, 2015

@ccordoba12 you want to get the same context menu when right clicking on a tab as well??

I have two methods in this PR, one is show_tab_menu (right click on a tab) and show_nontab_menu (right click on a QTabBar empty space, generally to the right of the tabs)

@ccordoba12
Copy link
Member

I'd say yes, for now. But leave the two methods, so if we come up with a better context menu for tabs in the future, we can change it easily :-)

@goanpeca
Copy link
Member Author

goanpeca commented May 3, 2015

Ready for testing @ccordoba12

@ccordoba12
Copy link
Member

Working great, even in PyQt5!

Nothing else than merging ;-)

ccordoba12 added a commit that referenced this pull request May 3, 2015
Add drag and drop support for dockwidgets sharing the same position
@ccordoba12 ccordoba12 merged commit 79f8e36 into spyder-ide:master May 3, 2015
@goanpeca goanpeca deleted the tabs branch May 3, 2015 20:43
@goanpeca goanpeca mentioned this pull request May 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants