Let the user add extra setup to slides

[gureckis]

Let's see, my first attempt at a pull request on github... and it is one line of code (starting to understand things)!

In addition to the idea in issue #10 which lets the user run some custom code when a slide appears (or hides), it may also be useful to execute some code after remark has been initialized. One example, for me, was that I wanted to programmatically add some styling to all the slides (e.g., insert a div into all the slides which contains a logo).

This pull request just adds a "remarkReady" event which is emitted after remark finishes setting up.

This can be used following kjbekkelund's approach like this:

var remark = this.remark;

$(document).ready(function () {
     remark.addListener("remarkReady", function() {
            // do some set up (such as apply additional styling to all slides)
            // for example using d3.js (https://github.com/mbostock/d3)
            // d3.selectAll(".slide").append("div").attr("class","lablogo").html("hi");
        });
});

[gnab]

Okay, so I'll try commenting each commit:

• The remarkReady event is a great idea. The only change I can think of is renaming it to just ready, in line with the DOM ready event. Since the event is bound via the remark global variable, I don't think there should be any confusion of the two.
• I get the rationale behind the title property. However, I'm thinking that making the entire set of slide properties available in the slidein (and other) events would be a more general and extendable approach. That way, you may have slideshow-specific custom properties handled in a consistent manner. (Regarding the removal of the call to slides.map in slideshowView.js:28, the callback function is actually supplied an index in addition to the element itself, so there's no need for the for loop to keep track of the indices.)
• The remark dom object is a only a DOM interaction wrapper needed when running the tests in node, where there is no DOM available (but console actually is). So basically you can access console directly / as usual in the slideshow-specific JavaScript running in the browser.
• The api extensions to disable interaction is a good idea. I've discussed the usage of eventing in previous pull request, concluding that events should be used to notify about rather than trigger actions, at least for the external api. Thus, something like api.disableInteraction() that in turn calls dispatcher.disable() would be better approach. Internally, however, using events as a means of inter-module communication is a better approach, as it promotes a decoupled architecture. Something that might be a bigger issue, though, is any custom event listeners possibly added by slideshow-specific JavaScript. Thus, instead of calling removeAllListeners for each event, I guess only the internal remark event listeners should be removed.
• The slidein and slideout are intentionally triggered before showing / hiding the slide in question, so I would suggest having something like beforeSlideIn and afterSlideIn.
• Finally, the api extensions for programmatically navigating slides should be changed in the same manner as the ones to disable interaction; instead of adding a listener for gotoPrevSlide there should be a function named gotoPrevSlide. Using the dispatcher to emit a gotoPreviousSlide event is perfectly fine, though, as it is used internally only.

I really haven't given the whole API stuff that much thought so far - it has evolved through pull request, more or less. It definitely deserves more attention for the upcoming releases :)

Ole Petter

[gureckis]

Hi, all of these points sound reasonable to me. When I get a chance I will whip up some altered commits that follow these suggestions.

One thought about api.disableInteraction() : Perhaps what would be better is not if the user actively disables the remark event handler, but instead if the user was simply notified about these events.

For example, in my example deck, it would be nice to have all of remark's touch handling code still trigger events in my custom javascript. In this case, I guess the function would be more like api.handOffEvents() or something(?). Basically remark stops automatically calling goToNextSlide()/gotoPrevSlide() when an arrow button/spacebar/etc... is pressed. Instead, in this mode, it would just notify the user with an event so they can decide how to handle it. (?)

I guess the design question is how much remark should "get out of the way" and let the user do the heavy lifting in coding new event handlers from scratch, or if it should stay active and just let the user know what's going on...

[gnab]

I agree, I seems useful that remark handles all the events and still signals when an event triggering slide navigation has occured. Currently, remark only notifies when the slide is being changed, so some new events would perhaps be required.

An alternative, and perhaps better (?), approach than deactivating a part of remark like this, is to make some event.preventDefault() function available. That way, when something like a beforeSlideIn event (which would be nice to implement anyways) gets triggered, the slideshow-specific JavaScript handling the event via the api could prevent the default action from happening, effectively preventing remark from going through with the slide change. That would make it easier to, for instance, only ignore navigation forwards, and instead show some slide animation, while moving backwards might still move to the previous slide.

What do you think?

[gureckis]

Was working on a couple of these things and came up with a minor comment: without explicitly importing the console into the dom object I get build errors. For example, if I call console.log() in api.js I guess the following build error.

mymachine:remark gureckis$ make bundle
node build/remark.js
path.existsSync is now called `fs.existsSync`.
../src/remark/api.js
 - 08:05: 'console' is not defined.
make: *** [bundle] Error 255

I should probably be able to sort this out myself, but importing explicitly works (e.g., you can call dom.console.log())

[gnab]

You get that error due to the linting with jshint during build. console is not registered as a predefined global variable, thus the failure. The .jshintrc file defines the settings for jshint. You simply need to add console to the predef entry and you should be fine.

[gnab]

A 'ready' event has now been added via issue #25.