Control when a Cell is executing #279

[MarcosVn]

Implements the proposed solution for the described scenario in #279 issue.

• Added the isExecuting attribute in CellState to track/control when a cell is executing;
• Also added isAnyCellExecuting to the cells map to control if there is any execution in progress (or if all cells are idle);
• Improved related comments;
• Improved the order of CellState attributes (individual cell attributes first, others for all cells last);
• Updated CellAdapter and CellCommands to receive the cellId in order to update the isExecuting state.

[MarcosVn]

Hey @echarles, could you review this PR when possible, please?

Here is some functional evidence:

state-for-cells-executing.mp4

A few additional points to discuss:

• We currently have the logic for executing cells from CodeCell or MarkdownCell instances replicated in at least three places. This issue has further exacerbated this replication. Do you see any problem with keeping it this way (as it was) or would it be better to centralize it using the execute method from the store?

• Do you see any issue with injecting the cellStore into the CellAdapter and CellCommands classes? I ended up needing to do this.

• Please check if you agree with the nomenclature as well. I was unsure between executing or idle, but I ended up going with executing to be more explicit.

• For now, I have also considered markdown cells when defining the state (just to standardize, simplify, and avoid conditionals to check for type X or Y when looking at the state). Please let me know if you disagree.

The use of state in a final component, as in the evidence above, would be something like:

const [isExecutingCells, setIsExecutingCells] = React.useState(false);
useEffect(() => {
  const handleChange = (newState: any) => {
    setIsExecutingCells(newState.isAnyCellExecuting);
  };

  const unsubscribe = cellStore.subscribe(handleChange);
  return () => {
    unsubscribe();
  };
}, []);

const onExecuteClick = () => {
  cellStore.getState().execute();
}

return (
  <Jupyter>
    <Box>
      <Cell id={'1'} type={'code'} autoStart={false} showToolbar={false}/>
      <Cell id={'2'} type={'code'} autoStart={false} showToolbar={false}/>
      <Button 
        onClick={onExecuteClick} 
        disabled={isExecutingCells}>Execute all</Button>
    </Box>
  </Jupyter>
)

[eric]

I think you have the wrong eric

[echarles]

@MarcosVn Overall looks good to me.

1. We may need first Kernel state for Cell and Output #282 in, so you would need to rebase/merge or create a new one.
2. Would be great to have the Cells example updated with the demo you show in the video.

[MarcosVn]

Hey @echarles, thanks for the feedback.

I will probably open a new PR; however, I will need some time to test and evaluate the changes from #282.
I will do this as soon as possible and once I have done it, I'll comment here so a new review/tests can be done.

PS: I will also include the evolution in the examples.

Thank you!

[echarles]

@MarcosVn #282 is merged. I hope it will not give you too many troubles to rebase.

[MarcosVn]

Hey, @echarles - how are you?
I opened a new PR #285 for #279 (after #282 changes). I believe it's much simpler now!

I have a single issue I'd like to align on. I pointed it out in this new PR.
As far as I'm concerned, we can close this one and discuss it over there.