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 menu to nteract-on-jupyter #2420

Merged
merged 2 commits into from
Jan 25, 2018

Conversation

theengineear
Copy link
Contributor

🎉 first pass at some minimal menu functionality

I went with code-completeness > feature completeness. I.e., there's much more to flesh out here, but I tried my best to get this code right on the first pass so that we can easily extend.

TODO:

  • create notebook-menu presentation component (i made it a dir withindex.js so it matches other components there)
  • connect presentation component to redux
  • snapshot test
  • test that we can mount and then assert that the appropriate actions are being called

- adds a dependency to rc-menu*
- hooks up some cell-level actions
- tests that user clicks trigger expected actions
- adds snapshots to lock down structure/style

*note that we lock this dependency down because we copy/alter css
@theengineear theengineear requested a review from rgbkrk January 25, 2018 19:00
@theengineear theengineear changed the title Add jupyter ext menu Add menu to nteract-on-jupyter Jan 25, 2018
@theengineear
Copy link
Contributor Author

screen shot 2018-01-25 at 11 00 40 am

// We export this for testing purposes.
export { PureNotebookMenu };

export default NotebookMenu;
Copy link
Member

Choose a reason for hiding this comment

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

This is beautifully organized.

// to alter how the actions work to just target the currently focused cell.
// That said, we *may* not have a great way of getting around this as we'll need
// information about the current document to decide which menu items are
// available...
Copy link
Member

Choose a reason for hiding this comment

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

Yeah that's a tricky one. We probably need to work with the focused cell and if we need to create more actions (like the "run all").

@rgbkrk rgbkrk merged commit fe09bf9 into nteract:master Jan 25, 2018
@rgbkrk
Copy link
Member

rgbkrk commented Jan 25, 2018

I'm confused as to why that's failing on Appveyor, I'm going to go ahead and merge it, clear the cache on appveyor and figure out the build setup.

@theengineear theengineear deleted the add-jupyter-ext-menu branch January 25, 2018 19:46
@lock
Copy link

lock bot commented May 1, 2018

Howdy! I'm 🔓🤖!

In order to keep information timely (based on the most recent release), we want all activity to be added to either new issues or open issues and PRs. In service to that goal, I, the lock bot close inactive closed issues when they haven't had activity in 120 days.

Feel free to open a new issue for related bugs and link to relevant comments from this thread.

@lock lock bot locked and limited conversation to collaborators May 1, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants