Skip to content

Commit

Permalink
Allow exit animations to play before step tooltip disappears.
Browse files Browse the repository at this point in the history
Closes #277

This removes our own manual management of `this.el`'s appearance/presence
in the DOM -- since tippy is handling it already.
  • Loading branch information
BrianSipple committed Oct 17, 2018
1 parent 870df77 commit 3283f4f
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 81 deletions.
7 changes: 4 additions & 3 deletions src/js/bind.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@ import { forOwn, isString, isUndefined } from 'lodash';
* @private
*/
function _setupAdvanceOnHandler(selector) {
return (e) => {
return (event) => {
if (this.isOpen()) {
const targetIsEl = this.el && e.target === this.el;
const targetIsSelector = !isUndefined(selector) && e.target.matches(selector);
const targetIsEl = this.el && event.target === this.el;
const targetIsSelector = !isUndefined(selector) && event.target.matches(selector);

if (targetIsSelector || targetIsEl) {
this.tour.next();
}
Expand Down
18 changes: 6 additions & 12 deletions src/js/step.js
Original file line number Diff line number Diff line change
Expand Up @@ -247,14 +247,10 @@ export class Step extends Evented {
* Triggers `destroy` event
*/
destroy() {
if (isElement(this.el) && this.el.parentNode) {
this.el.parentNode.removeChild(this.el);
delete this.el;
}

if (this.tooltip) {
this.tooltip.destroy();
this.tooltip = null;
this.el = null;
}

this.trigger('destroy');
Expand All @@ -266,12 +262,6 @@ export class Step extends Evented {
hide() {
this.trigger('before-hide');

if (this.el) {
this.el.hidden = true;
// We need to manually set styles for < IE11 support
this.el.style.display = 'none';
}

document.body.removeAttribute('data-shepherd-step');

if (this.target) {
Expand All @@ -290,7 +280,11 @@ export class Step extends Evented {
* @return {boolean} True if the step is open and visible
*/
isOpen() {
return Boolean(this.el && !this.el.hidden);
return Boolean(
this.tooltip &&
this.tooltip.state &&
this.tooltip.state.isVisible
);
}

/**
Expand Down
1 change: 0 additions & 1 deletion src/js/utils/tooltip-defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ export const defaults = {
arrow: true,
arrowTransform: 'scale(2)',
animation: 'fade',
delay: 200,
duration: 420,
flip: true,
animateFill: false, // https://atomiks.github.io/tippyjs/#animate-fill-option
Expand Down
134 changes: 69 additions & 65 deletions test/unit/test.step.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,21 @@ import defaultButtons from '../cypress/utils/default-buttons';
// since importing non UMD, needs assignment
window.Shepherd = Shepherd;

const DEFAULT_STEP_CLASS = 'shepherd-step-tooltip';

describe('Step', () => {
describe('Shepherd.Step()', () => {
const instance = new Shepherd.Tour({
defaultStepOptions: {
classes: 'shepherd-theme-arrows',
classes: DEFAULT_STEP_CLASS,
scrollTo: true
}
});

const testStep = instance.addStep('test', {
attachTo: 'body',
id: 'test',
text: 'This is a step for testing',
classes: 'example-step-extra-class',
buttons: [
{
text: 'Next',
Expand Down Expand Up @@ -67,16 +69,21 @@ describe('Step', () => {


it('has all the correct properties', () => {
const values = ['classes', 'scrollTo', 'id', 'text', 'buttons'];
const values = ['classes', 'scrollTo', 'attachTo', 'id', 'text', 'buttons'];
assert.deepEqual(values, Object.keys(testStep.options));
});

describe('.hide()', () => {
it('shows step evoking method, regardless of order', () => {
it('detaches from the step target', () => {
instance.start();

const targetElem = document.body;

assert.equal(targetElem.classList.contains('shepherd-enabled'), true);

testStep.hide();

assert.notEqual(document.querySelector('[data-id=test]').getAttribute('hidden'), null);
assert.equal(targetElem.classList.contains('shepherd-enabled'), false);
});
});

Expand All @@ -95,111 +102,108 @@ describe('Step', () => {
});

describe('bindAdvance()', () => {
it('should trigger the advanceOn option via string', () => {
const el = document.createElement('div');
const event = new Event('test');
const link = document.createElement('a');
let advanced = false;
link.classList.add('click-test');
let tooltipElem;
let event;
let link;
let hasAdvanced = false;

const advanceOnSelector = 'test-selector';
const advanceOnEventName = 'test-event';
const tourProto = {
next() { hasAdvanced = true; }
};

before(() => {
tooltipElem = document.createElement('div');
event = new Event(advanceOnEventName);

link = document.createElement('a');
link.classList.add(advanceOnSelector);
link.textContent = 'Click Me 👋';

document.body.appendChild(link);
});

const step = new Step({
next: () => advanced = true
}, {
advanceOn: '.click-test test'
after(() => {
link.remove();
});

it('triggers the `advanceOn` option via string', () => {
const step = new Step(tourProto, {
advanceOn: `.${advanceOnSelector} ${advanceOnEventName}`
});
step.el = el;
step.el.hidden = false;

step.isOpen = () => true;

step.bindAdvance();
link.dispatchEvent(event);

assert.isOk(link.classList.contains('click-test'));
assert.isOk(advanced);
assert.equal(link.classList.contains(advanceOnSelector), true);
assert.equal(hasAdvanced, true);
});

it('should trigger the advanceOn option via object', () => {
const el = document.createElement('div');
const event = new Event('test');
const link = document.createElement('a');
let advanced = false;
link.classList.add('object-test');
document.body.appendChild(link);

const step = new Step({
next: () => advanced = true
}, {
advanceOn: { selector: '.object-test', event: 'test' }
it('triggers the `advanceOn` option via object', () => {
const step = new Step(tourProto, {
advanceOn: { selector: `.${advanceOnSelector}`, event: advanceOnEventName }
});
step.el = el;
step.el.hidden = false;

step.isOpen = () => true;

step.bindAdvance();
link.dispatchEvent(event);

assert.isOk(link.classList.contains('object-test'));
assert.isOk(advanced, 'next triggered for advanceOn');
assert.equal(link.classList.contains(advanceOnSelector), true);
assert.equal(hasAdvanced, true, '`next()` triggered for advanceOn');
});

it('should capture events attached to no selector', () => {
const event = new Event('test');
let advanced = false;

const step = new Step({
next: () => advanced = true
}, {
advanceOn: { event: 'test' }
it('captures events attached to no element', () => {
const step = new Step(tourProto, {
advanceOn: { event: advanceOnEventName }
});

step.el = document.body;
step.el.hidden = false;
step.isOpen = () => true;

step.bindAdvance();
document.body.dispatchEvent(event);

assert.isOk(advanced, 'next triggered for advanceOn');
assert.isOk(hasAdvanced, '`next()` triggered for advanceOn');
});

it('should support bubbling events for nodes that do not exist yet', () => {
const event = new Event('blur');
let advanced = false;

const step = new Step({
next: () => advanced = true
}, {
const step = new Step(tourProto, {
text: 'Lorem ipsum dolor: <a href="https://example.com">sit amet</a>',
advanceOn: {
selector: 'a[href="https://example.com"]',
event: 'blur'
}
});
step.el = document.body;
step.el.hidden = false;

step.isOpen = () => true;

step.bindAdvance();
document.body.dispatchEvent(event);

assert.isOk(advanced, 'next triggered for advanceOn');
assert.isOk(hasAdvanced, '`next()` triggered for advanceOn');
});

it('it should call removeEventListener when destoryed', function(done){
const el = document.createElement('div');
const body = spy(document.body, 'removeEventListener');
const step = new Step({
next: () => true
}, {
advanceOn: { event: 'test' }
it('calls `removeEventListener` when destroyed', function(done){
const bodySpy = spy(document.body, 'removeEventListener');
const step = new Step(tourProto, {
advanceOn: { event: advanceOnEventName }
});
step.el = el;
step.el.hidden = false;

step.isOpen = () => true;

step.bindAdvance();
step.trigger('destroy');
assert.ok(body.called);
body.restore();

assert.equal(bodySpy.called, true);
bodySpy.restore();

done();
});

});

describe('bindButtonEvents()', () => {
Expand Down

0 comments on commit 3283f4f

Please sign in to comment.