Merge pull request gnab#496 from languitar/fix-494

Make keyboard navigation respect non-countable slides

src/remark/components/slide-number/slide-number.js

@@ -28,13 +28,5 @@ function formatSlideNumber (slide, slideshow) {
 }
 
 function getSlideNo (slide, slideshow) {
-  var slides = slideshow.getSlides(), i, slideNo = 0;
-
-  for (i = 0; i <= slide.getSlideIndex() && i < slides.length; ++i) {
-    if (slides[i].properties.count !== 'false') {
-      slideNo += 1;
-    }
-  }
-
-  return Math.max(1, slideNo);
+  return slide.getSlideNumber();
 }

src/remark/controllers/inputs/keyboard.js

@@ -55,7 +55,7 @@ Keyboard.prototype.addKeyboardEventListeners = function () {
         break;
       case 13: // Return
         if (self._gotoSlideNumber) {
-          events.emit('gotoSlide', self._gotoSlideNumber);
+          events.emit('gotoSlideNumber', self._gotoSlideNumber);
           self._gotoSlideNumber = '';
         }
         break;

src/remark/models/slide.js

@@ -2,7 +2,7 @@ var converter = require('../converter');
 
 module.exports = Slide;
 
-function Slide (slideIndex, slide, template) {
+function Slide (slideIndex, slideNumber, slide, template) {
   var self = this;
 
   self.properties = slide.properties || {};
@@ -11,6 +11,7 @@ function Slide (slideIndex, slide, template) {
   self.notes = slide.notes || '';
 
   self.getSlideIndex = function () { return slideIndex; };
+  self.getSlideNumber = function () { return slideNumber; };
 
   if (template) {
     inherit(self, template);

src/remark/models/slideshow.js

@@ -28,6 +28,7 @@ function Slideshow (events, dom, options, callback) {
   self.getSlides = getSlides;
   self.getSlideCount = getSlideCount;
   self.getSlideByName = getSlideByName;
+  self.getSlidesByNumber = getSlidesByNumber;
 
   self.togglePresenterMode = togglePresenterMode;
   self.toggleHelp = toggleHelp;
@@ -130,6 +131,10 @@ function Slideshow (events, dom, options, callback) {
     return slides.byName[name];
   }
 
+  function getSlidesByNumber (number) {
+    return slides.byNumber[number];
+  }
+
   function togglePresenterMode () {
     events.emit('togglePresenterMode');
   }
@@ -178,7 +183,9 @@ function createSlides (slideshowSource, options) {
     ;
 
   slides.byName = {};
+  slides.byNumber = {};
 
+  var slideNumber = 0;
   parsedSlides.forEach(function (slide, i) {
     var template, slideViewModel;
 
@@ -201,30 +208,35 @@ function createSlides (slideshowSource, options) {
       slide.properties.count = 'false';
     }
 
-    slideViewModel = new Slide(slides.length, slide, template);
+    var slideClasses = (slide.properties['class'] || '').split(/,| /)
+      , excludedClasses = options.excludedClasses || []
+      , slideIsIncluded = slideClasses.filter(function (c) {
+          return excludedClasses.indexOf(c) !== -1;
+        }).length === 0;
 
-    if (slide.properties.layout === 'true') {
-      layoutSlide = slideViewModel;
+    if (slideIsIncluded && slide.properties.layout !== 'true' && slide.properties.count !== 'false') {
+      slideNumber++;
+      slides.byNumber[slideNumber] = [];
     }
 
+    slideViewModel = new Slide(slides.length, slideNumber, slide, template);
+
     if (slide.properties.name) {
       byName[slide.properties.name] = slideViewModel;
     }
 
-    var slideClasses = (slide.properties['class'] || '').split(/,| /)
-      , excludedClasses = options.excludedClasses || []
-      , slideIsIncluded = slideClasses.filter(function (c) {
-          return excludedClasses.indexOf(c) !== -1;
-        }).length === 0;
-
-    if (slide.properties.layout !== 'true') {
+    if (slide.properties.layout === 'true') {
+      layoutSlide = slideViewModel;
+    } else {
       if (slideIsIncluded) {
         slides.push(slideViewModel);
+        slides.byNumber[slideNumber].push(slideViewModel);
       }
       if (slide.properties.name) {
         slides.byName[slide.properties.name] = slideViewModel;
       }
     }
+
   });
 
   return slides;

src/remark/models/slideshow/navigation.js

@@ -8,6 +8,7 @@ function Navigation (events) {
 
   self.getCurrentSlideIndex = getCurrentSlideIndex;
   self.gotoSlide = gotoSlide;
+  self.gotoSlideNumber = gotoSlideNumber;
   self.gotoPreviousSlide = gotoPreviousSlide;
   self.gotoNextSlide = gotoNextSlide;
   self.gotoFirstSlide = gotoFirstSlide;
@@ -16,6 +17,7 @@ function Navigation (events) {
   self.resume = resume;
 
   events.on('gotoSlide', gotoSlide);
+  events.on('gotoSlideNumber', gotoSlideNumber);
   events.on('gotoPreviousSlide', gotoPreviousSlide);
   events.on('gotoNextSlide', gotoNextSlide);
   events.on('gotoFirstSlide', gotoFirstSlide);
@@ -103,6 +105,13 @@ function Navigation (events) {
     gotoSlideByIndex(slideIndex, noMessage);
   }
 
+  function gotoSlideNumber (slideNumber, noMessage) {
+    var slides = self.getSlidesByNumber(parseInt(slideNumber, 10));
+    if (slides && slides.length) {
+      gotoSlideByIndex(slides[0].getSlideIndex(), noMessage);
+    }
+  }
+
   function gotoPreviousSlide() {
     gotoSlideByIndex(currentSlideIndex - 1);
   }

test/remark/components/slide-number_test.js

@@ -22,34 +22,11 @@ describe('Slide number', function () {
     slideNumber.element.innerHTML.should.equal('2 / 3');
   });
 
-  it('should not count slide marked not to be counted', function () {
-    var slide = createSlide(1)
-      , slideshow = {
-          getSlideNumberFormat: function () {
-            return '%current% / %total%';
-          }
-        , getSlides: function () { return [
-            createSlide(0, false),
-            slide
-          ];
-        }
-      };
-
-    slideNumber = new SlideNumber(slide, slideshow);
-
-    slideNumber.element.innerHTML.should.equal('1 / 1');
-  });
-
-  function createSlide (index, count) {
-    var slide = {
+  function createSlide (index) {
+    return {
       getSlideIndex: function () { return index; },
+      getSlideNumber: function () { return index + 1; },
       properties: {}
     }
-
-    if (count === false) {
-      slide.properties.count = 'false';
-    }
-
-    return slide;
   }
 });

test/remark/controllers/defaultController_test.js

@@ -113,13 +113,13 @@ describe('Controller', function () {
 
       events.emit.should.be.calledWithExactly('gotoLastSlide');
     });
-    
+
     it('should navigate to slide N when pressing N followed by return', function () {
       events.emit('keypress', {which: 49}); // 1
       events.emit('keypress', {which: 50}); // 2
       events.emit('keydown', {keyCode: 13}); // return
-      
-      events.emit.should.be.calledWithExactly('gotoSlide', '12');
+
+      events.emit.should.be.calledWithExactly('gotoSlideNumber', '12');
     });
 
     beforeEach(function () {

test/remark/models/slide_test.js

@@ -3,7 +3,7 @@ var Slide = require('../../../src/remark/models/slide');
 describe('Slide', function () {
   describe('properties', function () {
     it('should be extracted', function () {
-      var slide = new Slide(1, {
+      var slide = new Slide(1, 1, {
             content: [''],
             properties: {a: 'b', c: 'd'}
           });
@@ -15,12 +15,12 @@ describe('Slide', function () {
 
   describe('inheritance', function () {
     it('should inherit properties, content and notes', function () {
-      var template = new Slide(1, {
+      var template = new Slide(1, 1, {
             content: ['Some content.'],
             properties: {prop1: 'val1'},
             notes: 'template notes'
           })
-        , slide = new Slide(2, {
+        , slide = new Slide(2, 2, {
             content: ['More content.'],
             properties: {prop2: 'val2'},
             notes: 'slide notes'
@@ -33,31 +33,31 @@ describe('Slide', function () {
     });
 
     it('should not inherit name property', function () {
-      var template = new Slide(1, {
+      var template = new Slide(1, 1, {
             content: ['Some content.'],
             properties: {name: 'name'}
           })
-        , slide = new Slide(1, {content: ['More content.']}, template);
+        , slide = new Slide(1, 1, {content: ['More content.']}, template);
 
       slide.properties.should.not.have.property('name');
     });
 
     it('should not inherit layout property', function () {
-      var template = new Slide(1, {
+      var template = new Slide(1, 1, {
             content: ['Some content.'],
             properties: {layout: true}
           })
-        , slide = new Slide(1, {content: ['More content.']}, template);
+        , slide = new Slide(1, 1, {content: ['More content.']}, template);
 
       slide.properties.should.not.have.property('layout');
     });
 
     it('should aggregate class property value', function () {
-      var template = new Slide(1, {
+      var template = new Slide(1, 1, {
             content: ['Some content.'],
             properties: {'class': 'a'}
           })
-        , slide = new Slide(1, {
+        , slide = new Slide(1, 1, {
             content: ['More content.'],
             properties: {'class': 'b'}
           }, template);
@@ -66,11 +66,11 @@ describe('Slide', function () {
     });
 
     it('should not expand regular properties when inheriting template', function () {
-      var template = new Slide(1, {
+      var template = new Slide(1, 1, {
             content: ['{{name}}'],
             properties: {name: 'a'}
           })
-        , slide = new Slide(1, {
+        , slide = new Slide(1, 1, {
             content: [''],
             properites: {name: 'b'}
           }, template);
@@ -81,7 +81,7 @@ describe('Slide', function () {
 
   describe('variables', function () {
     it('should be expanded to matching properties', function () {
-      var slide = new Slide(1, {
+      var slide = new Slide(1, 1, {
         content: ['prop1 = {{ prop1 }}'],
         properties: {prop1: 'val1'}
       });
@@ -92,7 +92,7 @@ describe('Slide', function () {
     });
 
     it('should ignore escaped variables', function () {
-      var slide = new Slide(1, {
+      var slide = new Slide(1, 1, {
         content: ['prop1 = \\{{ prop1 }}'],
         properties: {prop1: 'val1'}
       });
@@ -103,7 +103,7 @@ describe('Slide', function () {
     });
 
     it('should ignore undefined variables', function () {
-      var slide = new Slide(1, {content: ['prop1 = {{ prop1 }}']});
+      var slide = new Slide(1, 1, {content: ['prop1 = {{ prop1 }}']});
 
       slide.expandVariables();