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 exit entry to File menu #5226

Merged
merged 4 commits into from Aug 29, 2018
Merged

Add exit entry to File menu #5226

merged 4 commits into from Aug 29, 2018

Conversation

@fcollonval
Copy link
Member

@fcollonval fcollonval commented Aug 28, 2018

Fix for #4647

Introduction of a Exit entry in the File menu. The entry can be removed through the LabApp.quit_button configuration option introduced by the notebook server.

Questions:

  • I named the entry Exit as it seems more used than Quit in desktop applications. Should I keep Quit for consistency with the classical notebook?
  • Should the new command be added to the palette?
  • I like to add unit test for this. Could somebody point me to some resources to test the presence of the entry depending on the configuration option?
@blink1073
Copy link
Member

@blink1073 blink1073 commented Aug 28, 2018

Thanks for this!

  • I'd prefer to keep the classic notebook name. MacOS uses Quit for applications as well.
  • Yes, a command palette entry sounds good
  • We don't yet have any "end-to-end" tests, but they would most likely be in a new top level directory called browser-tests and use puppeteer

@fcollonval
Copy link
Member Author

@fcollonval fcollonval commented Aug 28, 2018

I renamed the entry and add it into the Main Area category of the commands palette. For the test, I still have to learn the unit test part of JS world. So I unfortunately will skip it for lack of knowledge.

@blink1073
Copy link
Member

@blink1073 blink1073 commented Aug 28, 2018

No worries, I'll check it locally. Is this still WIP?

@fcollonval fcollonval changed the title WIP: Add exit entry to File menu Add exit entry to File menu Aug 28, 2018
@fcollonval
Copy link
Member Author

@fcollonval fcollonval commented Aug 28, 2018

It is good for me. I was waiting for the checks to pass 😉

@blink1073 blink1073 added this to the 0.35 milestone Aug 29, 2018
@blink1073
Copy link
Member

@blink1073 blink1073 commented Aug 29, 2018

Works well, thanks again!

@blink1073 blink1073 merged commit f6ee668 into jupyterlab:master Aug 29, 2018
2 checks passed
@fcollonval fcollonval deleted the quit-entry branch Aug 29, 2018
@blink1073 blink1073 mentioned this pull request Aug 30, 2018
blink1073 added a commit that referenced this issue Aug 30, 2018
* Add exit entry to File menu

* Rename Exit in Quit

* Add quit command to the palette

* Correct packages integrity
@blink1073 blink1073 removed this from the 0.35 milestone Sep 5, 2018
@blink1073 blink1073 added this to the 1.0 milestone Sep 5, 2018
@blink1073 blink1073 removed this from the 1.0 milestone Sep 28, 2018
@blink1073 blink1073 added this to the 0.35 milestone Sep 28, 2018
@blink1073 blink1073 mentioned this pull request Sep 28, 2018
31 tasks
@vykozlov vykozlov mentioned this pull request Feb 8, 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.

None yet

2 participants