Skip to content

Commit

Permalink
Ensure arguments are passed down to trigger (#381)
Browse files Browse the repository at this point in the history
When we switched our `drop` implementation away from lodash, we accidentally stopped passing things like `step` and `previous` down to subsequent trigger calls. This fixes the `drop` call by ensuring `arguments` is now an array.

Fixes #371
  • Loading branch information
RobbieTheWagner authored May 26, 2019
1 parent df2804e commit 566bdbf
Show file tree
Hide file tree
Showing 5 changed files with 310 additions and 297 deletions.
1 change: 1 addition & 0 deletions babel.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ module.exports = function(api) {
]
],
plugins: [
'rewire-exports',
'transform-es2015-modules-commonjs'
]
}
Expand Down
11 changes: 6 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -53,17 +53,18 @@
"tippy.js": "^4.3.0"
},
"devDependencies": {
"@babel/core": "^7.4.4",
"@babel/core": "^7.4.5",
"@babel/plugin-transform-object-assign": "^7.2.0",
"@babel/preset-env": "^7.4.4",
"@babel/preset-env": "^7.4.5",
"autoprefixer": "^9.5.1",
"babel-core": "^7.0.0-bridge.0",
"babel-jest": "^24.7.1",
"babel-plugin-rewire-exports": "^1.1.0",
"babel-plugin-transform-es2015-modules-commonjs": "^6.26.2",
"chai": "^4.2.0",
"codeclimate-test-reporter": "^0.5.1",
"cssnano": "^4.1.10",
"cypress": "^3.2.0",
"cypress": "^3.3.1",
"del": "^4.1.1",
"esdoc": "^1.1.0",
"esdoc-ecmascript-proposal-plugin": "^1.0.0",
Expand All @@ -87,7 +88,7 @@
"rollup-plugin-commonjs": "^9.3.4",
"rollup-plugin-css-only": "^1.0.0",
"rollup-plugin-eslint": "^6.0.0",
"rollup-plugin-filesize": "^6.0.1",
"rollup-plugin-filesize": "^6.1.0",
"rollup-plugin-license": "^0.8.1",
"rollup-plugin-multi-entry": "^2.1.0",
"rollup-plugin-node-resolve": "^5.0.0",
Expand All @@ -96,7 +97,7 @@
"rollup-plugin-terser": "^5.0.0",
"rollup-plugin-visualizer": "^1.1.1",
"sinon": "^7.3.2",
"start-server-and-test": "^1.9.0",
"start-server-and-test": "^1.9.1",
"stylelint": "^10.0.1",
"stylelint-config-ship-shape": "^0.5.2"
},
Expand Down
2 changes: 1 addition & 1 deletion src/js/evented.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export class Evented {

trigger(event) {
if (!isUndefined(this.bindings) && this.bindings[event]) {
const args = drop(arguments);
const args = drop(Array.prototype.slice.call(arguments));

this.bindings[event].forEach((binding, index) => {
const { ctx, handler, once } = binding;
Expand Down
18 changes: 18 additions & 0 deletions test/unit/tour.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { Step } from '../../src/js/step.js';
import tippy from 'tippy.js';
import { defaults as tooltipDefaults } from '../../src/js/utils/tooltip-defaults';
import { spy } from 'sinon';
import { drop, rewire$drop, restore } from '../../src/js/utils/general';

// since importing non UMD, needs assignment
window.Shepherd = Shepherd;
Expand Down Expand Up @@ -379,6 +380,14 @@ describe('Tour | Top-Level Class', function() {
});

describe('.show()', function() {
let dropSpy;

beforeEach(() => {
rewire$drop(dropSpy = spy(drop));
});

afterEach(restore);

it('show short-circuits if next is not found', function() {
let showFired = false;
instance.start();
Expand All @@ -399,6 +408,15 @@ describe('Tour | Top-Level Class', function() {
shouldShowStep = true;
instance.next();
expect(instance.getCurrentStep().id, 'step shown because `showOn` returns true').toBe('skipped-step');

// This spy checks that, when we call `trigger` with `show`, that we pass `arguments` down.
const dropArguments = dropSpy.args[2][0];
const dropReturnValue = dropSpy.returnValues[2][0];
expect(dropArguments[0]).toBe('show');
expect(dropArguments[1].previous).toBe(null);
expect(dropArguments[1].step.id).toBe('test');
expect(dropReturnValue.previous).toBe(null);
expect(dropReturnValue.step.id).toBe('test');
});

it(`sets the instance on \`Shepherd.activeTour\` if it's not already set`, function() {
Expand Down
Loading

0 comments on commit 566bdbf

Please sign in to comment.