From c65a5d049e8676912c93de828935b46dc1d8d831 Mon Sep 17 00:00:00 2001 From: Chuck Carpenter Date: Mon, 17 Feb 2020 15:16:09 -0700 Subject: [PATCH 1/4] =?UTF-8?q?=F0=9F=90=9B=20Fix=20initial=20scroll=20to?= =?UTF-8?q?=20top=20before=20scrolling=20to=20target?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/js/components/shepherd-element.svelte | 6 +++ src/js/step.js | 18 ++++---- src/js/utils/general.js | 54 ++++++++++++++++++++++- src/js/utils/popper-options.js | 6 +-- 4 files changed, 70 insertions(+), 14 deletions(-) diff --git a/src/js/components/shepherd-element.svelte b/src/js/components/shepherd-element.svelte index 5ec1124c3..d5fb13ce2 100644 --- a/src/js/components/shepherd-element.svelte +++ b/src/js/components/shepherd-element.svelte @@ -127,6 +127,12 @@ opacity: 1; } + .shepherd-element[data-popper-reference-hidden]:not(.shepherd-centered) { + visibility: hidden; + pointer-events: none; + opacity: 0; + } + .shepherd-element, .shepherd-element *, .shepherd-element *:after, .shepherd-element *:before { diff --git a/src/js/step.js b/src/js/step.js index 1d2347b40..5ca42dc0f 100644 --- a/src/js/step.js +++ b/src/js/step.js @@ -7,7 +7,6 @@ import ShepherdElement from './components/shepherd-element.svelte'; // Polyfills import smoothscroll from 'smoothscroll-polyfill'; - smoothscroll.polyfill(); /** @@ -329,7 +328,6 @@ export class Step extends Evented { if (this.options.advanceOn) { bindAdvance(this); } - setupTooltip(this); } @@ -345,22 +343,22 @@ export class Step extends Evented { this.tour.modal.setupForStep(this); this._styleTargetElementForStep(this); - this.el.hidden = false; - this.tooltip.update(); - - const content = this.shepherdElementComponent.getElement(); - const target = this.target || document.body; - target.classList.add(`${this.classPrefix}shepherd-enabled`, `${this.classPrefix}shepherd-target`); - content.classList.add('shepherd-enabled'); - + // start scrolling to target before showing the step if (this.options.scrollTo) { setTimeout(() => { this._scrollTo(this.options.scrollTo); }); } + this.el.hidden = false; + + const content = this.shepherdElementComponent.getElement(); + const target = this.target || document.body; + target.classList.add(`${this.classPrefix}shepherd-enabled`, `${this.classPrefix}shepherd-target`); + content.classList.add('shepherd-enabled'); + this.trigger('show'); this.el.focus(); } diff --git a/src/js/utils/general.js b/src/js/utils/general.js index 73effaceb..f728de481 100644 --- a/src/js/utils/general.js +++ b/src/js/utils/general.js @@ -1,6 +1,57 @@ import { createPopper } from '@popperjs/core'; import { isString } from './type-check'; import { makeCenteredPopper } from './popper-options'; +import getNodeName from '@popperjs/core/lib/dom-utils/getNodeName.js'; +import { isHTMLElement } from '@popperjs/core/lib/dom-utils/instanceOf.js'; +import applyStylesModifier from '@popperjs/core/lib/modifiers/applyStyles'; + +function effectFixed({ state }) { + const initialStyles = { + position: 'absolute', + left: '0', + top: '0', + margin: '0' + }; + + Object.assign(state.elements.popper.style, initialStyles); + + return () => { + Object.keys(state.elements).forEach((name) => { + const element = state.elements[name]; + const styleProperties = Object.keys( + Object.prototype.hasOwnProperty.call(state.styles, name) + ? { ...state.styles[name] } + : initialStyles + ); + const attributes = state.attributes[name] || {}; + + // Set all values to an empty string to unset them + const style = styleProperties.reduce( + (style, property) => ({ + ...style, + [String(property)]: '' + }), + {} + ); + + // arrow is optional + virtual elements + if (!isHTMLElement(element) || !getNodeName(element)) { + return; + } + + // Flow doesn't support to extend this property, but it's the most + // effective way to apply styles to an HTMLElement + // $FlowFixMe + Object.assign(element.style, style); + + Object.keys(attributes).forEach((attribute) => + element.removeAttribute(attribute) + ); + }); + }; +} +// work around for default state values +applyStylesModifier.effect = effectFixed; /** * Ensure class prefix ends in `-` @@ -83,8 +134,9 @@ export function uuid() { */ export function getPopperOptions(attachToOptions, step) { let popperOptions = { - // required to keep the Step in the viewport modifiers: [ + applyStylesModifier, + // required to keep the Step in the viewport { name: 'preventOverflow', options: { diff --git a/src/js/utils/popper-options.js b/src/js/utils/popper-options.js index 088f4d56d..a70a8b9e6 100644 --- a/src/js/utils/popper-options.js +++ b/src/js/utils/popper-options.js @@ -47,11 +47,10 @@ function _getCenteredStylePopperModifier() { */ export function makeCenteredPopper(step) { const centeredStylePopperModifier = _getCenteredStylePopperModifier(step); + const content = step.shepherdElementComponent.getElement(); let popperOptions = _makeCommonPopperOptions(step); - popperOptions.placement = 'top'; - popperOptions.strategy = 'absolute'; - + content.classList.add('shepherd-centered'); popperOptions = { ...popperOptions, modifiers: Array.from(new Set([...popperOptions.modifiers, ...centeredStylePopperModifier])) @@ -62,6 +61,7 @@ export function makeCenteredPopper(step) { function _makeCommonPopperOptions() { const popperOptions = { + placement: 'top', strategy: 'fixed', modifiers: [] }; From f3a8b07b59eaf47a1686069e915a66b67bf93332 Mon Sep 17 00:00:00 2001 From: Chuck Carpenter Date: Wed, 19 Feb 2020 13:48:58 -0700 Subject: [PATCH 2/4] =?UTF-8?q?=F0=9F=90=9B=20Update=20to=20use=20focus=20?= =?UTF-8?q?in=20onFirstUpdate=20as=20suggested?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/js/components/shepherd-element.svelte | 2 - src/js/step.js | 1 - src/js/utils/general.js | 64 +++-------------------- src/js/utils/popper-options.js | 8 ++- test/unit/utils/general.spec.js | 8 +-- 5 files changed, 16 insertions(+), 67 deletions(-) diff --git a/src/js/components/shepherd-element.svelte b/src/js/components/shepherd-element.svelte index d5fb13ce2..6daea4e2b 100644 --- a/src/js/components/shepherd-element.svelte +++ b/src/js/components/shepherd-element.svelte @@ -128,8 +128,6 @@ } .shepherd-element[data-popper-reference-hidden]:not(.shepherd-centered) { - visibility: hidden; - pointer-events: none; opacity: 0; } diff --git a/src/js/step.js b/src/js/step.js index 5ca42dc0f..70fbd3dc3 100644 --- a/src/js/step.js +++ b/src/js/step.js @@ -360,7 +360,6 @@ export class Step extends Evented { content.classList.add('shepherd-enabled'); this.trigger('show'); - this.el.focus(); } /** diff --git a/src/js/utils/general.js b/src/js/utils/general.js index f728de481..ec75de012 100644 --- a/src/js/utils/general.js +++ b/src/js/utils/general.js @@ -1,57 +1,6 @@ import { createPopper } from '@popperjs/core'; import { isString } from './type-check'; import { makeCenteredPopper } from './popper-options'; -import getNodeName from '@popperjs/core/lib/dom-utils/getNodeName.js'; -import { isHTMLElement } from '@popperjs/core/lib/dom-utils/instanceOf.js'; -import applyStylesModifier from '@popperjs/core/lib/modifiers/applyStyles'; - -function effectFixed({ state }) { - const initialStyles = { - position: 'absolute', - left: '0', - top: '0', - margin: '0' - }; - - Object.assign(state.elements.popper.style, initialStyles); - - return () => { - Object.keys(state.elements).forEach((name) => { - const element = state.elements[name]; - const styleProperties = Object.keys( - Object.prototype.hasOwnProperty.call(state.styles, name) - ? { ...state.styles[name] } - : initialStyles - ); - const attributes = state.attributes[name] || {}; - - // Set all values to an empty string to unset them - const style = styleProperties.reduce( - (style, property) => ({ - ...style, - [String(property)]: '' - }), - {} - ); - - // arrow is optional + virtual elements - if (!isHTMLElement(element) || !getNodeName(element)) { - return; - } - - // Flow doesn't support to extend this property, but it's the most - // effective way to apply styles to an HTMLElement - // $FlowFixMe - Object.assign(element.style, style); - - Object.keys(attributes).forEach((attribute) => - element.removeAttribute(attribute) - ); - }); - }; -} -// work around for default state values -applyStylesModifier.effect = effectFixed; /** * Ensure class prefix ends in `-` @@ -135,15 +84,17 @@ export function uuid() { export function getPopperOptions(attachToOptions, step) { let popperOptions = { modifiers: [ - applyStylesModifier, - // required to keep the Step in the viewport { name: 'preventOverflow', options: { altAxis: true } } - ] + ], + strategy: 'absolute', + onFirstUpdate() { + step.el.focus(); + } }; let target = document.body; @@ -167,10 +118,7 @@ export function getPopperOptions(attachToOptions, step) { delete step.options.popperOptions.modifiers; } - popperOptions = { - ...popperOptions, - ...step.options.popperOptions - }; + popperOptions = Object.assign(...popperOptions, ...step.options.popperOptions); } return { element: step.el, popperOptions, target }; diff --git a/src/js/utils/popper-options.js b/src/js/utils/popper-options.js index a70a8b9e6..bee63d5d7 100644 --- a/src/js/utils/popper-options.js +++ b/src/js/utils/popper-options.js @@ -8,6 +8,7 @@ function _getCenteredStylePopperModifier() { return; } const style = { + position: 'fixed', left: '50%', top: '50%', transform: 'translate(-50%, -50%)' @@ -59,11 +60,14 @@ export function makeCenteredPopper(step) { return popperOptions; } -function _makeCommonPopperOptions() { +function _makeCommonPopperOptions(step) { const popperOptions = { placement: 'top', strategy: 'fixed', - modifiers: [] + modifiers: [], + onFirstUpdate() { + step.el.focus(); + } }; return popperOptions; diff --git a/test/unit/utils/general.spec.js b/test/unit/utils/general.spec.js index 8bb235e3b..2fed4b2b0 100644 --- a/test/unit/utils/general.spec.js +++ b/test/unit/utils/general.spec.js @@ -13,10 +13,10 @@ describe('General Utils', function() { }); }); - describe.only('getPopperOptions', function() { + describe('getPopperOptions', function() { it('modifiers can be overridden', function() { const step = new Step({}, { - attachTo: { element: '.scroll-test', on: 'center' }, + attachTo: { element: '.scroll-test', on: 'right' }, popperOptions: { modifiers: [ { @@ -29,7 +29,7 @@ describe('General Utils', function() { } }); - const { popperOptions } = getPopperOptions(parseAttachTo(step), step); + const { popperOptions } = getPopperOptions(step.options.attachTo, step); expect(popperOptions.modifiers[0].options.altAxis).toBe(false); }); @@ -43,7 +43,7 @@ describe('General Utils', function() { } }); - const { popperOptions } = getPopperOptions(parseAttachTo(step), step); + const { popperOptions } = getPopperOptions(step.options.attachTo, step); expect(popperOptions.strategy).toBe('absolute'); }); }); From f21b22acec4b58e5715601dd8c03b323baebbbc3 Mon Sep 17 00:00:00 2001 From: Chuck Carpenter Date: Wed, 19 Feb 2020 14:40:57 -0700 Subject: [PATCH 3/4] =?UTF-8?q?=F0=9F=90=9B=20Fix=20testing=20issue?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/js/utils/general.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/js/utils/general.js b/src/js/utils/general.js index ec75de012..e9aa75778 100644 --- a/src/js/utils/general.js +++ b/src/js/utils/general.js @@ -117,8 +117,10 @@ export function getPopperOptions(attachToOptions, step) { delete step.options.popperOptions.modifiers; } + console.log(popperOptions); + console.log(step.options); - popperOptions = Object.assign(...popperOptions, ...step.options.popperOptions); + popperOptions = Object.assign(popperOptions, step.options.popperOptions); } return { element: step.el, popperOptions, target }; From f294510c425451ec7b935dda6e325f069ad396b9 Mon Sep 17 00:00:00 2001 From: Chuck Carpenter Date: Wed, 19 Feb 2020 14:42:42 -0700 Subject: [PATCH 4/4] =?UTF-8?q?=F0=9F=94=87=20Remove=20logs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/js/utils/general.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/js/utils/general.js b/src/js/utils/general.js index e9aa75778..cb5add4e5 100644 --- a/src/js/utils/general.js +++ b/src/js/utils/general.js @@ -117,8 +117,6 @@ export function getPopperOptions(attachToOptions, step) { delete step.options.popperOptions.modifiers; } - console.log(popperOptions); - console.log(step.options); popperOptions = Object.assign(popperOptions, step.options.popperOptions); }