Merge pull request #216 from kate2753/gh72Target

Fix `grunt devES` task and split fixture set up and test specs into separate files

Gruntfile.js

@@ -156,7 +156,7 @@ module.exports = function (grunt) {
         tasks: ['test']
       },
       esCode: {
-        files: ['<%=paths.source%>/es/**/*.js', '<%=paths.test%>/es/**/*.js'],
+        files: ['<%=paths.source%>/es/**/*.js', '<%=paths.test%>/es/**/*.js', '<%=paths.source%>/less/*.less'],
         tasks: ['buildES', 'buildESTest'],
         options: {
           livereload: true
@@ -187,7 +187,8 @@ module.exports = function (grunt) {
         options: {
           outfile: '<%=paths.build%>/test/SpecRunner.html',
           keepRunner: true,
-          specs: ['<%=paths.build%>/test/*.js'],
+          specs: ['<%=paths.build%>/test/*.spec.js'],
+          helpers: ['<%=paths.build%>/test/es/helpers/fixtureSetup.js'],
           styles: ['<%=paths.build%>/css/hopscotch.css']
         }
       },
@@ -196,7 +197,8 @@ module.exports = function (grunt) {
         options: {
           outfile: '<%=paths.build%>/test/SpecRunner.html',
           keepRunner: true,
-          specs: ['<%=paths.build%>/test/*.js'],
+          specs: ['<%=paths.build%>/test/*.spec.js'],
+          helpers: ['<%=paths.build%>/test/es/helpers/fixtureSetup.js'],
           styles: ['<%=paths.build%>/css/hopscotch.css'],
           template: require('grunt-template-jasmine-istanbul'),
           templateOptions: {
@@ -298,6 +300,9 @@ module.exports = function (grunt) {
             '<%=paths.build%>/test/es/specs/placement.spec.js',
             '<%=paths.build%>/test/es/helpers/placement.js'
           ],
+          '<%=paths.build%>/test/target.spec.js': [
+            '<%=paths.build%>/test/es/specs/target.spec.js'
+          ]
         }
       }
     },
@@ -333,25 +338,25 @@ module.exports = function (grunt) {
   grunt.registerTask(
     'buildES',
     'Build hopscotch for testing (jshint, minify js, process less to css)',
-    ['clean:build', 'copy:build', 'eslint', 'jst:compile', 'babel:dist', 'browserify:dist', 'includereplace:esSource', 'less']
+    ['clean:build', 'copy:build', 'eslint', 'jst:compile', 'babel', 'browserify', 'includereplace:esSource', 'less']
     );
 
   grunt.registerTask(
     'buildESTest',
     'Build hopscotch for testing (jshint, minify js, process less to css)',
-    ['babel:test', 'browserify:test']
+    ['jasmine:testESDev:build']
     );
 
   grunt.registerTask(
     'testES',
     'Run unit tests against refactored library code',
-    ['buildES', 'buildESTest', 'jasmine:testESDev']
+    ['buildES', 'jasmine:testESDev']
     );
 
   grunt.registerTask(
     'devES',
     'Start test server to allow debugging unminified hopscotch code in a browser',
-    ['buildES', 'buildESTest', 'jasmine:testESDev:build', 'connect:testServer', 'log:devES', 'watch:esCode']
+    ['buildES', 'buildESTest', 'connect:testServer', 'log:devES', 'watch:esCode']
     );
 
   //grunt task aliases

src/es/managers/CalloutManager.js

@@ -17,8 +17,13 @@ export default class CalloutManager {
     }
     callout = new Callouts.StandaloneCallout(configHash, globalCfg);
     callouts[configHash.id] = callout;
-    callout.render();
-    callout.show();
+    try {
+      callout.render();
+      callout.show();
+    } catch (err) {
+      this.removeCallout(configHash.id);
+      throw err;
+    }
     return callout;
   }
   getCallout(calloutId) {

src/es/managers/PlacementManager.js

@@ -152,7 +152,7 @@ function positionCallout(callout, placementStrategy) {
 
   let targetEl = getTarget(callout.config.get('target'));
   if (!targetEl) {
-    return;
+    throw new Error('Must specify an existing target element via \'target\' option.');
   }
 
   let isTargetFixed = isFixedElement(targetEl);

test/es/helpers/fixtureSetup.js

@@ -0,0 +1,25 @@
+function setupFixture() {
+  //create an element for shopping list
+  let shoppingListDiv = document.createElement('div'); 
+  //set up shopping list
+  shoppingListDiv.id = 'shopping-list';
+  shoppingListDiv.style.margin = '40px auto';
+  shoppingListDiv.style.width = '400px';
+  shoppingListDiv.innerHTML =
+  '<style> #shopping-list li { margin-top: 5px; padding: 5px 3px; border: 1px solid black; list-style: none;}</style>' +
+  '<ul>' +
+  '  <li>This is an example list for the sake of having some UI to point to.</li>' +
+  '  <li id="milk">Milk</li>' +
+  '  <li id="eggs">Eggs</li>' +
+  '  <li id="lettuce">Lettuce</li>' +
+  '  <li id="bread">Bread</li>' +
+  '  <li id="yogurt">Yogurt</li>' +
+  '</ul>' +
+  '<div class="fixedTarget" style="position: fixed; right: 300px; top: 150px; background: #CCC; padding: 10px;">' +
+  '  Fixed positioned element' +
+  '  <span>With child element in it</span>' +
+  '</div>';
+  //insert shopping list into the DOM
+  document.body.appendChild(shoppingListDiv);
+}
+setupFixture();

test/es/specs/placement.spec.js

@@ -1,31 +1,5 @@
 import PlacementTestUtils from '../helpers/placement.js';
 
-function setupFixture() {
-  //create an element for shopping list
-  let shoppingListDiv = document.createElement('div'); 
-  //set up shopping list
-  shoppingListDiv.id = 'shopping-list';
-  shoppingListDiv.style.margin = '40px auto';
-  shoppingListDiv.style.width = '400px';
-  shoppingListDiv.innerHTML =
-  '<style> #shopping-list li { margin-top: 5px; padding: 5px 3px; border: 1px solid black; list-style: none;}</style>' +
-  '<ul>' +
-  '  <li>This is an example list for the sake of having some UI to point to.</li>' +
-  '  <li id="milk">Milk</li>' +
-  '  <li id="eggs">Eggs</li>' +
-  '  <li id="lettuce">Lettuce</li>' +
-  '  <li id="bread">Bread</li>' +
-  '  <li id="yogurt">Yogurt</li>' +
-  '</ul>' +
-  '<div class="fixedTarget" style="position: fixed; right: 300px; top: 150px; background: #CCC; padding: 10px;">' +
-  '  Fixed positioned element' +
-  '  <span>With child element in it</span>' +
-  '</div>';
-  //insert shopping list into the DOM
-  document.body.appendChild(shoppingListDiv);
-}
-setupFixture();
-
 let placementTests = [
   {
     placement: 'top',

test/es/specs/target.spec.js

@@ -0,0 +1,17 @@
+describe('Callout missing target', () => {
+  it('Should throw an exception when standalone callout does not have a valid target', () => {
+    let calloutMgr = hopscotch.getCalloutManager();
+    let calloutID = 'callout-no-target';
+    expect(() => {
+      calloutMgr.createCallout({
+        id: calloutID,
+        target: 'totally-does-not-exist',
+        placement: 'bottom',
+        title: 'This test is fun!',
+        content: 'This is how we test this library!'
+      });
+    }).toThrow(new Error('Must specify an existing target element via \'target\' option.'));
+
+    expect(calloutMgr.getCallout(calloutID)).toBeUndefined();
+  });
+});