deck.js-markdown icon indicating copy to clipboard operation
deck.js-markdown copied to clipboard

Markdown support breaks nested slides

Open tmbrggmn opened this issue 14 years ago • 7 comments

Using this Markdown extension breaks the nested slides: nested slides are not displayed correctly any more (they are not displayed in the parent-slide, and are displayed as the next full slide).

tmbrggmn avatar Oct 17 '11 17:10 tmbrggmn

This happens because of the value.html(converter.makeHtml(value.html())); call. In fact, calling value.html(value.html()); has the same effect as well. As soon as the HTML content of a nested section changes, the nested slides are broken and don't work any more (despite being identical to an unaltered set of nested slides).

tmbrggmn avatar Oct 17 '11 20:10 tmbrggmn

This is exactly what happens, when going to the next slide which is a nested slide:

  • The slide that contains the nested slides does not get the deck-child-current class
  • The nested slide that is supposed to be shown does not get the deck-current class (it remains slide deck-after)

tmbrggmn avatar Oct 18 '11 13:10 tmbrggmn

@tmbrggmn Here's the problem: When you're processing a nested slide parent, setting the html with the converted value means you're replacing all of the child nodes with new ones. But the slides array has already been built at this point, and still references the old child slides. This is why the deck states never change on the nested slides, they aren't part of the slides array. (The child-current thing will resolve itself when you solve this problem.) Hope this makes sense.

You might have some preference on how to handle this. I can think of a few different approaches if you would like to hear them.

imakewebthings avatar Oct 24 '11 03:10 imakewebthings

@imakewebthings Thanks for the feedback. The obvious solution would be to re-build the slides collection, but that may not be the best solution? I'd love to hear your suggestions!

tmbrggmn avatar Oct 24 '11 06:10 tmbrggmn

@tmbrggmn You certainly could, though you would need to look out for a few things:

  • Recalling init within init will get you into a bit of a loop. You could change the binding to a .one, which makes the plugin not reinit proof if one were to change the slides after init. Not a big deal though.
  • More importantly, deck.js doesn't provide a public method for getting the original argument to init. Since it can be HTML nodes or jQuery objects, as well as your more typical string selector, that scenario would leave you in the same situation since those nodes are no longer attached.

You could construct a tree structure of the slides, then temporarily detach any children, process, reattach, dive down the tree, and repeat, but it seems like a lot of trouble.

You could just force the user to include the converter stuff before they call init, but that defeats the whole purpose of the plugin.

My preferred way to handle this would be: I add a 'deck.beforeInit' event at the top of the init method, after the options are set but before the slides array is created, where preprocessing like this can occur, and you just change the converter to convert the whole container;

$(document).bind('deck.beforeInit', function() {
   var $c = $.deck('getContainer');

   $c.html(new Markdown.Converter().makeHtml($c.html()));
});

If that sounds good I can add this event later today.

imakewebthings avatar Oct 24 '11 08:10 imakewebthings

@imakewebthings Sounds good :-) Thanks for the help.

tmbrggmn avatar Oct 24 '11 09:10 tmbrggmn

@tmbrggmn Try the latest on my master branch. The event name is the same as I mentioned above.

imakewebthings avatar Oct 25 '11 07:10 imakewebthings