iodide-project / iodide

Literate scientific computing and communication for the web
Mozilla Public License 2.0
1.49k stars 141 forks source link

should we save javascript DOM manipulations in rendered cells? #71

Closed bcolloran closed 6 years ago

bcolloran commented 6 years ago

accessing history undoes render state of MD cells-- in the e.g. THREE example, going to look at history somehow removes the reference to targetDiv and the cell defining it has to be run again.

maybe history can be presented in a little window of some sort rather than paving over the whole window?

hamilton commented 6 years ago

@bcolloran The notebook does save the render state of the MD cell, but the creation of the canvas element happens in javascript in another cell. What gets saved in the value field of the MD cell upon render doesn't capture any subsequent DOM manipulation. So when the cells get redrawn, we lose the appending of the canvas element. This is actually a very interesting issue.

There are two solutions to this, not mutually exclusive.

  1. Having the DOMCell type will be a good practice going forward for many of the reasons I've stated vocally. It creates enough separation between the textual presentation of a MarkdownCell and the "data presentation" that a canvas rendering won't get destroyed upon reload.
  2. We can also create a listener for the MarkdownCell to watch the inner HTML for changes, and save any changes to that in the value field of the cell. This will maintain any subsequent DOM manipulations. There is a drawback that we are introducing new complexity to the MD rendering that might be hard to debug for the user.

For the time being, I have a fix for this so we don't have to do any additional UI treatment while we implement the above. Essentially if in history view we'll still render the other cells, but will hide the container entirely so that the canvas elements don't get clobbered.

hamilton commented 6 years ago

The workaround alluded to in the first comment is in c9ed5b483f6113fdfbfdf3117fca8dc0ba85f592.

hamilton commented 6 years ago

Another reason why 1 above is my preferred solution is this: if I add a canvas element to a Markdown cell via a javascript cell, and then I edit the content of the markdown cell, the canvas element will be destroyed.

And of course if you add the canvas element IN the MD cell, everything works as expected.

Should this be the expected behavior?

bcolloran commented 6 years ago

i'm actually happy with the behavior now that

  1. we have history as a popover, and
  2. we keep the rendered MD in the background when editing an MD cell

I think we should close this, but i'll leave it to you since you broadened the scope (i'm happy to keep the scope narrow until needed, but if you want to keep this discussion open that's fine)

hamilton commented 6 years ago

Yes, I think especially your 2nd point means we can close this.