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 option to toggle toolbars visibility #2426

Merged
merged 5 commits into from
Jul 21, 2015
Merged

Add option to toggle toolbars visibility #2426

merged 5 commits into from
Jul 21, 2015

Conversation

goanpeca
Copy link
Member

@goanpeca goanpeca commented May 7, 2015

Related to #2353

Description

Adds a toggle toolbars visibility button on view menu. Visibility on (action checked) visibility off (action unchecekd).

The last visible toolbars are updated only when using the toggle action.

image

Todo

  • Saves last visible toolbars when using the toggle button
  • Update config
  • Add a shortcut (Ctrl+Shift+T)
  • Add docstrings

@goanpeca goanpeca self-assigned this May 7, 2015
@goanpeca goanpeca added this to the v3.0 milestone May 7, 2015
@goanpeca
Copy link
Member Author

goanpeca commented May 7, 2015

@ccordoba12, @Nodd what would the expected behavior be after a shutdown?

I think it makes sense to "toggle" as in, if 5 toolbars were on then the toggle will make those (and only those 5) visible/invisible.

My question is... what should be done on startup if all toolbars are invisible? should I keep track of the last set of toolbars that was visible before the disable? or should we just grey out the toggle visibility option if all toolbars are off after a start?

@goanpeca goanpeca mentioned this pull request May 7, 2015
@goanpeca
Copy link
Member Author

Comments @spyder-ide/core-developers ?

I think it makes sense to "toggle" as in, if 5 toolbars were on then the toggle will make those (and only those 5) visible/invisible.

My question is... what should be done on startup if all toolbars are invisible? should I keep track of the last set of toolbars that was visible before the disable? or should we just grey out the toggle visibility option if all toolbars are off after a start?

@blink1073
Copy link
Contributor

Yeah we could keep track of the current/previously visible toolbars, and toggle would bring back the previous known state, that seems intuitive.

@Nodd
Copy link
Contributor

Nodd commented May 11, 2015

👍

@goanpeca goanpeca changed the title [WIP] Add option to toggle toolbars visibility Add option to toggle toolbars visibility May 11, 2015
@@ -192,7 +192,9 @@ def is_ubuntu():
'use_custom_margin': True,
'custom_margin': 0,
'show_internal_console_if_traceback': True,
'check_updates_on_startup': True
'check_updates_on_startup': True,
'last_visible_toolbars': [],
Copy link
Member

Choose a reason for hiding this comment

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

This options is transient, so there's no need to add to it 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.

Yes, I keep forgetting that!

@ccordoba12
Copy link
Member

Ok, first review done.

@goanpeca
Copy link
Member Author

goanpeca commented Jul 2, 2015

I have to add some docstrings still but I fixed all of your comments and now instead of toggling, it uses a trigger (like maximize pane) so that the text in the option changes between Show toolbars and Hide toolbars

@goanpeca
Copy link
Member Author

goanpeca commented Jul 2, 2015

@ccordoba12 updated docstrings!

@@ -484,6 +485,7 @@ def is_ubuntu():
'_/save current layout': "Shift+Alt+S",
'_/toggle default layout': "Shift+Alt+Home",
'_/layout preferences': "Shift+Alt+P",
'_/show toolbars': "Alt+Shift+T",
Copy link
Member

Choose a reason for hiding this comment

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

@goanpeca, can you confirm that this working for you? I remember you told me there was some problem with this change.

Copy link
Member Author

Choose a reason for hiding this comment

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

I checked, it works

ccordoba12 added a commit that referenced this pull request Jul 21, 2015
Add option to toggle toolbars visibility
@ccordoba12 ccordoba12 merged commit 27b79eb into spyder-ide:master Jul 21, 2015
@goanpeca goanpeca deleted the toolbars branch August 3, 2015 04:35
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.

4 participants