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 ability to toggle status bar visibility #5990

Merged
merged 3 commits into from Feb 15, 2019

Conversation

@mbektas
Copy link
Member

@mbektas mbektas commented Feb 15, 2019

Fixes #5982

  • Adds a menu item into View menu and a Command Palette command to toggle Status Bar visibility
  • Selection is persisted using settingRegistry

status-bar-toggle

@jupyterlab-dev-mode
Copy link

@jupyterlab-dev-mode jupyterlab-dev-mode bot commented Feb 15, 2019

Thanks for making a pull request to JupyterLab!

To try out this branch on binder, follow this link: Binder

@jasongrout jasongrout self-requested a review Feb 15, 2019
@jasongrout jasongrout assigned jasongrout and unassigned jasongrout Feb 15, 2019
"visible": {
"type": "boolean",
"title": "Status Bar Visibility",
"description": "Whether to show status bar at launch",
Copy link
Contributor

@jasongrout jasongrout Feb 15, 2019

it's any time, so I would delete "at launch"

@@ -46,8 +50,15 @@ export const STATUSBAR_PLUGIN_ID = '@jupyterlab/statusbar-extension:plugin';
const statusBar: JupyterFrontEndPlugin<IStatusBar> = {
id: STATUSBAR_PLUGIN_ID,
provides: IStatusBar,
requires: [ISettingRegistry, IMainMenu, ICommandPalette],
Copy link
Contributor

@jasongrout jasongrout Feb 15, 2019

Can you make the menu optional, and maybe even the command palette? Like in the apputils-extension/src/index.ts with the themesPaletteMenu extension? We're trying to make more things optional so plugins work in more contexts.

Copy link
Member

@ian-r-rose ian-r-rose left a comment

Thanks @mbektasbbg! This looks great, just a couple of minor comments.

activate: (app: JupyterFrontEnd, labShell: ILabShell | null) => {
activate: (
app: JupyterFrontEnd,
settingRegistry: ISettingRegistry,
Copy link
Member

@ian-r-rose ian-r-rose Feb 15, 2019

Ever since #5845 we have been trying to make the core plugins less interdependent by making a lot of these tokens optional. Can you move ISettingRegistry, IMainMenu, and ICommandPalette to the optional list, and then check for them before using them in the activate function?

@@ -59,6 +70,41 @@ const statusBar: JupyterFrontEndPlugin<IStatusBar> = {
});
}

const category: string = 'Main Area';
const command: string = 'toggle-jp-main-statusbar';
Copy link
Member

@ian-r-rose ian-r-rose Feb 15, 2019

Our convention for command IDs has been to use something like 'plugin:action', so can we make this 'statusbar:toggle'?

@mbektas
Copy link
Member Author

@mbektas mbektas commented Feb 15, 2019

thanks @ian-r-rose @jasongrout . I made the changes you suggested.

Copy link
Member

@ian-r-rose ian-r-rose left a comment

Looks good to me, thanks @mbektasbbg.

@ian-r-rose ian-r-rose merged commit bdd751b into jupyterlab:master Feb 15, 2019
9 checks passed
@jasongrout jasongrout added this to the 1.0 milestone Feb 26, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Aug 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants