From 6453d023e803c505baa017c5619611b74c8da2cf Mon Sep 17 00:00:00 2001 From: Kamran Ahmed Date: Tue, 20 Mar 2018 00:22:58 +0100 Subject: [PATCH] Fix - Breaks the UI when element has fixed or absolute position --- .eslintrc.json | 1 + src/common/constants.js | 1 + src/common/utils.js | 21 ++++++++++ src/core/element.js | 87 ++++++++++++++++++++++------------------- src/driver.scss | 3 ++ types/index.d.ts | 16 +++++--- 6 files changed, 83 insertions(+), 46 deletions(-) diff --git a/.eslintrc.json b/.eslintrc.json index 4a2a051..5720a25 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -11,6 +11,7 @@ "func-names": "off", "no-bitwise": "off", "class-methods-use-this": "off", + "prefer-destructuring": "off", "no-param-reassign": [ "off" ], diff --git a/src/common/constants.js b/src/common/constants.js index 01610d8..14b3ba8 100644 --- a/src/common/constants.js +++ b/src/common/constants.js @@ -13,6 +13,7 @@ export const ID_STAGE = 'driver-highlighted-element-stage'; export const ID_POPOVER = 'driver-popover-item'; export const CLASS_DRIVER_HIGHLIGHTED_ELEMENT = 'driver-highlighted-element'; +export const CLASS_POSITION_RELATIVE = 'driver-position-relative'; export const CLASS_NO_ANIMATION = 'driver-no-animation'; export const CLASS_POPOVER_TIP = 'driver-popover-tip'; diff --git a/src/common/utils.js b/src/common/utils.js index 524217b..60ab9b2 100644 --- a/src/common/utils.js +++ b/src/common/utils.js @@ -11,3 +11,24 @@ export const createNodeFromString = (htmlString) => { // Change this to div.childNodes to support multiple top-level nodes return div.firstChild; }; + +/** + * Gets the CSS property from the given element + * @param {HTMLElement|Node} element + * @param {string} propertyName + * @return {string} + */ +export const getStyleProperty = (element, propertyName) => { + let propertyValue = ''; + + if (element.currentStyle) { + propertyValue = element.currentStyle[propertyName]; + } else if (document.defaultView && document.defaultView.getComputedStyle) { + propertyValue = document.defaultView + .getComputedStyle(element, null) + .getPropertyValue(propertyName); + } + + return propertyValue && propertyValue.toLowerCase ? propertyValue.toLowerCase() : propertyValue; +}; + diff --git a/src/core/element.js b/src/core/element.js index fcd22a8..4130a35 100644 --- a/src/core/element.js +++ b/src/core/element.js @@ -1,5 +1,6 @@ +import { ANIMATION_DURATION_MS, CLASS_DRIVER_HIGHLIGHTED_ELEMENT, CLASS_POSITION_RELATIVE } from '../common/constants'; +import { getStyleProperty } from '../common/utils'; import Position from './position'; -import { ANIMATION_DURATION_MS, CLASS_DRIVER_HIGHLIGHTED_ELEMENT } from '../common/constants'; /** * Wrapper around DOMElements to enrich them @@ -36,27 +37,6 @@ export default class Element { this.animationTimeout = null; } - /** - * Gets the screen co-ordinates (x,y) for the current dom element - * @returns {{x: number, y: number}} - * @private - */ - getScreenCoordinates() { - let tempNode = this.node; - - let x = this.document.documentElement.offsetLeft; - let y = this.document.documentElement.offsetTop; - - if (tempNode.offsetParent) { - do { - x += tempNode.offsetLeft; - y += tempNode.offsetTop; - } while (tempNode = tempNode.offsetParent); - } - - return { x, y }; - } - /** * Checks if the current element is visible in viewport * @returns {boolean} @@ -128,24 +108,20 @@ export default class Element { * @public */ getCalculatedPosition() { - const coordinates = this.getScreenCoordinates(); - const position = new Position({ - left: Number.MAX_VALUE, - top: Number.MAX_VALUE, - right: 0, - bottom: 0, - }); + const body = this.document.body; + const documentElement = this.document.documentElement; + const window = this.window; - // If we have the position for this element - // and the element is visible on screen (has some height) - if (typeof coordinates.x === 'number' && typeof coordinates.y === 'number' && (this.node.offsetWidth > 0 || this.node.offsetHeight > 0)) { - position.left = Math.min(position.left, coordinates.x); - position.top = Math.min(position.top, coordinates.y); - position.right = Math.max(position.right, coordinates.x + this.node.offsetWidth); - position.bottom = Math.max(position.bottom, coordinates.y + this.node.offsetHeight); - } + const scrollTop = this.window.pageYOffset || documentElement.scrollTop || body.scrollTop; + const scrollLeft = window.pageXOffset || documentElement.scrollLeft || body.scrollLeft; + const elementRect = this.node.getBoundingClientRect(); - return position; + return new Position({ + top: elementRect.top + scrollTop, + left: elementRect.left + scrollLeft, + right: elementRect.left + scrollLeft + elementRect.width, + bottom: elementRect.top + scrollTop + elementRect.height, + }); } /** @@ -160,7 +136,7 @@ export default class Element { this.hideStage(); } - this.node.classList.remove(CLASS_DRIVER_HIGHLIGHTED_ELEMENT); + this.removeHighlightClasses(); // If there was any animation in progress, cancel that this.window.clearTimeout(this.animationTimeout); @@ -170,6 +146,10 @@ export default class Element { } } + removeHighlightClasses() { + this.node.classList.remove(CLASS_DRIVER_HIGHLIGHTED_ELEMENT); + } + /** * Checks if the given element is same as the current element * @param {Element} element @@ -202,7 +182,7 @@ export default class Element { this.showPopover(); this.showStage(); - this.node.classList.add(CLASS_DRIVER_HIGHLIGHTED_ELEMENT); + this.addHighlightClasses(); const highlightedElement = this; const popoverElement = this.popover; @@ -220,6 +200,33 @@ export default class Element { } } + addHighlightClasses() { + this.node.classList.add(CLASS_DRIVER_HIGHLIGHTED_ELEMENT); + + if (this.canMakeRelative()) { + this.node.classList.add(CLASS_POSITION_RELATIVE); + } + } + + canMakeRelative() { + const currentPosition = this.getStyleProperty('position'); + const avoidPositionsList = ['absolute', 'fixed', 'relative']; + + // Because if the element has any of these positions, making it + // relative will break the UI + return !avoidPositionsList.includes(currentPosition); + } + + /** + * Get an element CSS property on the page + * @param {string} property + * @returns string + * @private + */ + getStyleProperty(property) { + return getStyleProperty(this.node, property); + } + /** * Shows the stage behind the element * @public diff --git a/src/driver.scss b/src/driver.scss index 5916a55..f6a4540 100644 --- a/src/driver.scss +++ b/src/driver.scss @@ -172,5 +172,8 @@ div#driver-highlighted-element-stage { .driver-highlighted-element { z-index: $highlighted-element-zindex !important; +} + +.driver-position-relative { position: relative; } \ No newline at end of file diff --git a/types/index.d.ts b/types/index.d.ts index 317c410..cadf1de 100644 --- a/types/index.d.ts +++ b/types/index.d.ts @@ -199,12 +199,6 @@ declare module 'driver.js' { window: Window, document: Document); - /** - * Gets the screen coordinates for the current DOM Element - * @return {Driver.ScreenCoordinates} - */ - public getScreenCoordinates(): Driver.ScreenCoordinates; - /** * Checks if the give element is in view port or not * @return {boolean} @@ -280,6 +274,16 @@ declare module 'driver.js' { * @return {Driver.ElementSize} */ public getSize(): Driver.ElementSize; + + /** + * Removes the highlight classes from current element if any + */ + private removeHighlightClasses(): void; + + /** + * Adds the highlight classes to current element if required + */ + private addHighlightClasses(): void; } class Overlay { -- GitLab