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 8e4f85a
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 18 deletions.
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
17 changes: 12 additions & 5 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 Down

0 comments on commit 8e4f85a

Please sign in to comment.