-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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 lock/unlock option for panes #2345
Conversation
@dougthor42 can you give it a try? |
Hmmm I think we should change places where it says 'dockwidget' to pane... otherwise the preferences looks funny....
So maybe it should say
|
I agree on the location in the menu, but I wouldn't add this option in the preferences at all. The lock state should be remembered, that's all.(and disabled by defalut, new users should be able toe asily change the layout). Going back to the menu, what do you think about grouping options dirferentely (it could be another PR). The point is to better group what goes together:
I don't know where the attached console window comes from so I don't know where to put it. |
👍 makes sense. Yes i agree on the menu change, what do you think of changing the word dockwidgt by pane? It would be more coherent |
If the others agree with @Nodd I can make the corrections he suggests to arrange the menu, as it indeed would be better... I think I would like
|
👍 for pane instead of dockwidget. |
👍 sure ;-) |
Works wonderfully. Thanks a bunch |
Just a minor comment: I think the lock option should be on by default, not off :-) Will come back later. |
@ccordoba12 by on you mean having panes locked by default (no possibility of rearranging them?), I thought you preferred off. I prefer on by default as well... but I had the impression the rest of @spyder-ide/collaborators (including you) preferred off |
Personally, one of the first things I do when launching spyder on a new computer is to rearrange the panes. Maybe I'm too much of a "power user". |
I personally find super annoying that sometimes the panes get crazy because I missed a mouse click... but when in keyboard only mode (most of the time)... that is hardly a problem. Now off goes against discover-ability maybe, but I also think first impressions are lasting ones, so if a panes goes crazy for a new user the user might think the software is buggy... |
I think that the default, unconfigured value should be Locked Panes = Off. For new users: For users coming from PyCharm, Visual Studio, Eclipse, etc: Or it could just be added to the tutorial. |
@goanpeca, I changed my mind after altering my layout several times because of misplaced mouse clicks ;-) @dougthor42, it's true that power users always want to rearrange things to their liking. But I think Spyder is picked up by a lot of newbies, so I'd prefer to adjust things for them. But power user can always lookup for the option to unlock panels and adjust things again ;-) |
So... change to locked by default? I say 👍 to that. @ccordoba12 any comments on reorganizing the menu? |
I think |
👍 Should I add this change to this PR, or should I open a new one. Any thought on changing the entries in the preferences dialog where it says: dockwidgets... -> it should read panes... (dockwidget is not a very clear concept for someone not use to Qt) |
I originally put the panes near the layouts because the layout change the panes. But I agree that they are more important so at the top is also fine.
I vote that reordering the menu and changing a few labels can be done in this PR, as long as they are separate commits. |
Yes, please do those changes in this PR too. |
@ccordoba12 done! |
'_/preferences': "Ctrl+Alt+Shift+P", | ||
'_/maximize pane': "Ctrl+Alt+Shift+M", | ||
'_/fullscreen mode': "F11", | ||
'_/toggle next layout': "Shift+Alt+PgDown", | ||
'_/toggle previous layout': "Shift+Alt+PgUp", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since you're removing these options, you need to bump CONF_VERSION
below
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was actually renaming them,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know, but that's exactly the same as removing some options and adding new ones. And the removing part needs a bump in CONF_VERSION
:-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, will make the bump then
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now I have 16.1.0 I should change it to 17.0.0 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes
Should we hide the dockwidget titlebar when locked? This was discussed and suggested in #2353 to improve usable space in Spyder. |
I vote yes (I would use such a feature) but as an option and disabled by default, because locked is the default, which means that a new user would no see the titlebars at all. |
Sure, I would use it too, is it that problematic to make it on by default? |
Imagine a new user, first time launching spyder an looking at the interface: there would be no indication of what all the dockwidgets are (unless there are multiple widgets in the same area, there would be tabs). I see this a a feature for advanced users who already know what each pane is for. |
I just like to say: new features = new PRs :-) Please keep things simple in every PR to make my life easier :-) Since the purpose of this PR is to add a lock/unlock menu entry, you should leave things up to that point. |
Well in that case please merge :-) |
@@ -485,7 +485,7 @@ def __init__(self, options=None): | |||
self.is_starting_up = True | |||
self.is_setting_up = True | |||
|
|||
self.dockwidgets_locked = False | |||
self.dockwidgets_locked = CONF.get('main', 'panes_locked', True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't add default values to CONF.get
(i.e. remove the True
at the end). This messes up with updates in CONF_VERSION
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe the argument should be removed from the fuction then, to avoid future similar problems ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops.. that is a remnant from the first code ... 👍 fixed
I tested this locally, and it's working as expected. Thanks for this, great work as usual! |
Add lock/unlock option for panes
👍 |
Fixes #2096
Description
This PR fixes three things:
Menu entry
Before
Now
Preferences dialog
New wording (Replaced dockwidgets by panes)
Todo: