Add context menus to sections containing runnable code cells #141
Conversation
There was a problem hiding this comment.
Just tried this locally and it worked as expected! One behavior I'm noticing is that this doesn't run markdown cells - is that part of the limitations that required ToC to be in code snippet mode for this functionality to be available? I don't really think this is a big deal, just something I noticed while playing around with it - could be a nice touch. Also, I wonder if it'd be useful to have the same context menu appear for all toc items, but just have the "Run cell(s)" button be disabled for items without runnable code cells (for consistency). I could definitely see an argument for either side but thought I'd bring it up. Otherwise this looks good to me!
| /** | ||
| * Returns a component which renders a table of contents tree. | ||
| * | ||
| * @param props - component properties | ||
| * @returns component | ||
| */ | ||
| constructor(props: IProperties) { | ||
| super(props); | ||
| } | ||
|
|
There was a problem hiding this comment.
No, it is not necessary. I usually include it as a matter of course so that the constructor is documented and setup in the event that component is necessary (e.g., as in toc.tsx). I can remove, if this is a blocker.
|
@marthacryan Thanks for the review! Regarding Markdown cells, I did not think that adding the ability to run Markdown cells was necessary, as not clear to me what the purpose/use case of this would be. In what situation would a user want to run Markdown cells and how common would that be? Re: context menu for all cells. Yes, probably. Otherwise, a user has to intuit why a context menu is possible for some ToC items, but not others. I did not include here, as I did not want an empty context menu for non-code cells. And I did not add a context menu with the item greyed out, as was not clear to me that this was better than not showing a context menu at all. I can update this PR to accommodate either design pattern, if desired. |
|
@kgryte I'm sorry I forgot to follow up on this - just noticed as I was checking that everything would be in order to archive the repo. Would you be interested in moving this PR onto core? I can make the PR if you don't have time, but I think this is a good addition to the ToC! |
|
I don't have time at the moment, and, if you have the bandwidth, go for it! :) |
This PR should partially address #138 by
I recognize that this PR is not likely to be merged given jupyterlab/jupyterlab#8538; however, this PR may prove illustrative for demonstrating a path forward in core.
Demo