From 73eb90ab96622fb1c268e03c31707679eaab0bb2 Mon Sep 17 00:00:00 2001 From: Thijs Louisse Date: Sun, 17 May 2020 17:36:18 +0200 Subject: [PATCH] feat(tooltip): simplified tooltip component by making arrow a template Co-authored-by: Aymen Ben Amor --- packages/tooltip/index.js | 1 - packages/tooltip/lion-tooltip-arrow.js | 3 - packages/tooltip/src/LionTooltip.js | 96 ++++++++++++++---- packages/tooltip/src/LionTooltipArrow.js | 59 ----------- packages/tooltip/stories/index.stories.mdx | 79 ++++++--------- .../tooltip/test/lion-tooltip-arrow.test.js | 98 ------------------- packages/tooltip/test/lion-tooltip.test.js | 61 +++++++++++- 7 files changed, 165 insertions(+), 232 deletions(-) delete mode 100644 packages/tooltip/lion-tooltip-arrow.js delete mode 100644 packages/tooltip/src/LionTooltipArrow.js delete mode 100644 packages/tooltip/test/lion-tooltip-arrow.test.js diff --git a/packages/tooltip/index.js b/packages/tooltip/index.js index beb5088ad..8668e4823 100644 --- a/packages/tooltip/index.js +++ b/packages/tooltip/index.js @@ -1,2 +1 @@ export { LionTooltip } from './src/LionTooltip.js'; -export { LionTooltipArrow } from './src/LionTooltipArrow.js'; diff --git a/packages/tooltip/lion-tooltip-arrow.js b/packages/tooltip/lion-tooltip-arrow.js deleted file mode 100644 index 69248c729..000000000 --- a/packages/tooltip/lion-tooltip-arrow.js +++ /dev/null @@ -1,3 +0,0 @@ -import { LionTooltipArrow } from './src/LionTooltipArrow.js'; - -customElements.define('lion-tooltip-arrow', LionTooltipArrow); diff --git a/packages/tooltip/src/LionTooltip.js b/packages/tooltip/src/LionTooltip.js index 9dbd67a2c..329ce0efe 100644 --- a/packages/tooltip/src/LionTooltip.js +++ b/packages/tooltip/src/LionTooltip.js @@ -1,7 +1,69 @@ -import { html, LitElement } from '@lion/core'; +import { css, html, LitElement } from '@lion/core'; import { OverlayMixin } from '@lion/overlays'; export class LionTooltip extends OverlayMixin(LitElement) { + static get properties() { + return { + hasArrow: { + type: Boolean, + reflect: true, + attribute: 'has-arrow', + }, + }; + } + + static get styles() { + return css` + :host { + --tooltip-arrow-width: 12px; + --tooltip-arrow-height: 8px; + } + + :host([hidden]) { + display: none; + } + + .arrow { + position: absolute; + width: var(--tooltip-arrow-width); + height: var(--tooltip-arrow-height); + } + + .arrow svg { + display: block; + } + + [x-placement^='bottom'] .arrow { + top: calc(-1 * var(--tooltip-arrow-height)); + transform: rotate(180deg); + } + + [x-placement^='left'] .arrow { + right: calc( + -1 * (var(--tooltip-arrow-height) + + (var(--tooltip-arrow-width) - var(--tooltip-arrow-height)) / 2) + ); + transform: rotate(270deg); + } + + [x-placement^='right'] .arrow { + left: calc( + -1 * (var(--tooltip-arrow-height) + + (var(--tooltip-arrow-width) - var(--tooltip-arrow-height)) / 2) + ); + transform: rotate(90deg); + } + + .arrow { + display: none; + } + + :host([has-arrow]) .arrow { + display: block; + } + `; + } + constructor() { super(); this._mouseActive = false; @@ -12,9 +74,6 @@ export class LionTooltip extends OverlayMixin(LitElement) { connectedCallback() { super.connectedCallback(); this._overlayContentNode.setAttribute('role', 'tooltip'); - // Schedule setting up the arrow element so that it works both on firstUpdated - // and when the tooltip is moved in the DOM (disconnected + reconnected) - this.updateComplete.then(() => this.__setupArrowElement()); } render() { @@ -22,20 +81,20 @@ export class LionTooltip extends OverlayMixin(LitElement) {
- - - +
+ ${this._arrowTemplate()} +
`; } - __setupArrowElement() { - this.__arrowElement = Array.from(this.children).find(child => child.slot === 'arrow'); - if (!this.__arrowElement) { - return; - } - this.__arrowElement.setAttribute('x-arrow', true); - // this._overlayContentWrapperNode.appendChild(this.__arrowElement); + // eslint-disable-next-line class-methods-use-this + _arrowTemplate() { + return html` + + + + `; } // eslint-disable-next-line class-methods-use-this @@ -52,7 +111,7 @@ export class LionTooltip extends OverlayMixin(LitElement) { enabled: true, }, arrow: { - enabled: true, + enabled: this.hasArrow, }, }, onCreate: data => { @@ -71,12 +130,15 @@ export class LionTooltip extends OverlayMixin(LitElement) { }); } + get _arrowNode() { + return this.shadowRoot.querySelector('[x-arrow]'); + } + __syncFromPopperState(data) { if (!data) { return; } - if (this.__arrowElement && data.placement !== this.__arrowElement.placement) { - this.__arrowElement.placement = data.placement; + if (this._arrowNode && data.placement !== this._arrowNode.placement) { this.__repositionCompleteResolver(data.placement); this.__setupRepositionCompletePromise(); } diff --git a/packages/tooltip/src/LionTooltipArrow.js b/packages/tooltip/src/LionTooltipArrow.js deleted file mode 100644 index 0cef9a7ef..000000000 --- a/packages/tooltip/src/LionTooltipArrow.js +++ /dev/null @@ -1,59 +0,0 @@ -import { css, html, LitElement } from '@lion/core'; - -export class LionTooltipArrow extends LitElement { - static get properties() { - return { - placement: { type: String, reflect: true }, - }; - } - - static get styles() { - return css` - :host { - position: absolute; - --tooltip-arrow-width: 12px; - --tooltip-arrow-height: 8px; - width: var(--tooltip-arrow-width); - height: var(--tooltip-arrow-height); - } - - :host([hidden]) { - display: none; - } - - :host svg { - display: block; - } - - :host([placement^='bottom']) { - top: calc(-1 * var(--tooltip-arrow-height)); - transform: rotate(180deg); - } - - :host([placement^='left']) { - right: calc( - -1 * (var(--tooltip-arrow-height) + - (var(--tooltip-arrow-width) - var(--tooltip-arrow-height)) / 2) - ); - transform: rotate(270deg); - } - - :host([placement^='right']) { - left: calc( - -1 * (var(--tooltip-arrow-height) + - (var(--tooltip-arrow-width) - var(--tooltip-arrow-height)) / 2) - ); - transform: rotate(90deg); - } - `; - } - - /* IE11 will not render the arrow without this method. */ - render() { - return html` - - - - `; - } -} diff --git a/packages/tooltip/stories/index.stories.mdx b/packages/tooltip/stories/index.stories.mdx index ed20f4b7f..9a5e87fbe 100644 --- a/packages/tooltip/stories/index.stories.mdx +++ b/packages/tooltip/stories/index.stories.mdx @@ -1,10 +1,8 @@ import { Story, Meta, html, object, withKnobs } from '@open-wc/demoing-storybook'; import { css } from '@lion/core'; import { tooltipDemoStyles } from './tooltipDemoStyles.js'; -import { LionTooltipArrow } from '../src/LionTooltipArrow.js'; +import { LionTooltip } from '../src/LionTooltip.js'; import '../lion-tooltip.js'; -import '../lion-tooltip-arrow.js'; -
- +
Its top placement
- - +
Its right placement
- - +
Its bottom placement
- - +
Its left placement
-
`} ```html - +
Its top placement
 @@ -178,44 +172,40 @@ Modifier explanations: - flip: enables flipping behavior on the primary axis (e.g. if top placement, flipping to bottom if there is not enough space on the top). The padding property defines the margin with the boundariesElement, which is usually the viewport. - offset: enables an offset between the content node and the invoker node. First argument is horizontal marign, second argument is vertical margin. - ### Arrow -Popper also comes with an arrow modifier. -In our tooltip you can pass an arrow element (e.g. an SVG Element) through the `slot="arrow"`. +By default, the arrow is disabled for our tooltip. Via the `has-arrow` property it can be enabled. -We export a `lion-tooltip-arrow` that you can use by default for this. +> As a Subclasser, you can decide to turn the arrow on by default if this fits in your Design System {html` - +
This is a tooltip
- `} ```html - +
This is a tooltip
- ``` #### Use a custom arrow -If you plan on passing your own arrow element, you can extend the `lion-tooltip-arrow`. +If you plan on providing a custom arrow, you can extend the `lion-tooltip`. -All you need to do is override the `render` method to pass your own SVG, and extend the styles to pass the proper dimensions of your arrow. +All you need to do is override the `_arrowTemplate` method to pass your own SVG, and extend the styles to pass the proper dimensions of your arrow. The rest of the work is done by Popper.js (for positioning) and the `lion-tooltip-arrow` (arrow dimensions, rotation, etc.). {() => { - if (!customElements.get('custom-tooltip-arrow')) { - customElements.define('custom-tooltip-arrow', class extends LionTooltipArrow { + if (!customElements.get('custom-tooltip')) { + customElements.define('custom-tooltip', class extends LionTooltip { static get styles() { return [super.styles, css` :host { @@ -224,7 +214,11 @@ The rest of the work is done by Popper.js (for positioning) and the `lion-toolti } `]; } - render() { + constructor() { + super(); + this.hasArrow = true; + } + _arrowTemplate() { return html` @@ -235,11 +229,10 @@ The rest of the work is done by Popper.js (for positioning) and the `lion-toolti } return html` - +
This is a tooltip
- - + `; }} @@ -248,7 +241,7 @@ The rest of the work is done by Popper.js (for positioning) and the `lion-toolti import { html, css } from '@lion/core'; import { LionTooltipArrow } from '@lion/tooltip'; -class CustomTooltipArrow extends LionTooltipArrow { +class CustomTooltip extends LionTooltip { static get styles() { return [super.styles, css` :host { @@ -257,8 +250,11 @@ class CustomTooltipArrow extends LionTooltipArrow { } `]; } - - render() { + constructor() { + super(); + this.hasArrow = true; + } + _arrowTemplate() { return html` @@ -266,27 +262,12 @@ class CustomTooltipArrow extends LionTooltipArrow { `; } } -customElements.define('custom-tooltip-arrow', CustomTooltipArrow); +customElements.define('custom-tooltip', CustomTooltip); ``` ```html - +
This is a tooltip
- - + ``` - -#### Rationale - -**Why a Web Component for the Arrow?** - -Our preferred API is to pass the arrow as a slot. -Popper.JS however, necessitates that the arrow element is **inside** the content node, -otherwise it cannot compute the positioning of the Popper element and the arrow element, so we move the arrow node inside the content node. - -This means that we can no longer style the arrow through the `lion-tooltip` with the `::slotted` selector, -because the arrow element is no longer a direct child of `lion-tooltip`. - -Additionally we now also need to sync the popper placement state to the arrow element, to style it properly. -For all this logic + style encapsulation, we decided a Web Component was the only reasonable solution for the arrow element. diff --git a/packages/tooltip/test/lion-tooltip-arrow.test.js b/packages/tooltip/test/lion-tooltip-arrow.test.js deleted file mode 100644 index f9f6f5a52..000000000 --- a/packages/tooltip/test/lion-tooltip-arrow.test.js +++ /dev/null @@ -1,98 +0,0 @@ -import { expect, fixture, html } from '@open-wc/testing'; -import '../lion-tooltip-arrow.js'; -import '../lion-tooltip.js'; - -describe('lion-tooltip-arrow', () => { - it('has a visual "arrow" element inside the content node', async () => { - const el = await fixture(html` - - -
Hey there
- - - `); - const arrowEl = el.querySelector('lion-tooltip-arrow'); - expect(arrowEl).dom.to.equal(``, { - ignoreAttributes: ['slot', 'placement', 'x-arrow', 'style'], - }); - }); - - it('reflects popper placement in its own placement property and attribute', async () => { - const el = await fixture(html` - - -
Hey there
- - - `); - el.opened = true; - const arrowElement = el.querySelector('lion-tooltip-arrow'); - - await el.repositionComplete; - expect(arrowElement.getAttribute('placement')).to.equal('right'); - - el.config = { popperConfig: { placement: 'bottom' } }; - // TODO: Remove this once changing configurations no longer closes your overlay - // Currently it closes your overlay but doesn't set opened to false >:( - el.opened = false; - el.opened = true; - - await el.repositionComplete; - expect(arrowElement.getAttribute('placement')).to.equal('bottom'); - }); - - it('is hidden when attribute hidden is true', async () => { - const el = await fixture(html` - -
- Hey there -
- - - - `); - const arrowElement = el.querySelector('lion-tooltip-arrow'); - el.opened = true; - - await el.repositionComplete; - expect(arrowElement).not.to.be.displayed; - }); - - it.skip('makes sure positioning of the arrow is correct', async () => { - const el = await fixture(html` - -
- Hey there -
- - - - `); - const arrowElement = el.querySelector('lion-tooltip-arrow'); - el.opened = true; - - await el.repositionComplete; - - expect(getComputedStyle(arrowElement).getPropertyValue('top')).to.equal( - '11px', - '30px (content height) - 8px = 22px, divided by 2 = 11px offset --> arrow is in the middle', - ); - - expect( - getComputedStyle(el.querySelector('lion-tooltip-arrow')).getPropertyValue('left'), - ).to.equal( - '-10px', - ` - arrow height is 8px so this offset should be taken into account to align the arrow properly, - as well as half the difference between width and height ((12 - 8) / 2 = 2) - `, - ); - }); -}); diff --git a/packages/tooltip/test/lion-tooltip.test.js b/packages/tooltip/test/lion-tooltip.test.js index 83151b10f..b3a2374b5 100644 --- a/packages/tooltip/test/lion-tooltip.test.js +++ b/packages/tooltip/test/lion-tooltip.test.js @@ -15,7 +15,7 @@ describe('lion-tooltip', () => { }); describe('Basic', () => { - it('should show content on mouseenter and hide on mouseleave', async () => { + it('shows content on mouseenter and hide on mouseleave', async () => { const el = await fixture(html`
Hey there
@@ -32,7 +32,7 @@ describe('lion-tooltip', () => { expect(el._overlayCtrl.isShown).to.equal(false); }); - it('should show content on mouseenter and remain shown on focusout', async () => { + it('shows content on mouseenter and remain shown on focusout', async () => { const el = await fixture(html`
Hey there
@@ -49,7 +49,7 @@ describe('lion-tooltip', () => { expect(el._overlayCtrl.isShown).to.equal(true); }); - it('should show content on focusin and hide on focusout', async () => { + it('shows content on focusin and hide on focusout', async () => { const el = await fixture(html`
Hey there
@@ -67,7 +67,7 @@ describe('lion-tooltip', () => { expect(el._overlayCtrl.isShown).to.equal(false); }); - it('should show content on focusin and remain shown on mouseleave', async () => { + it('shows content on focusin and remain shown on mouseleave', async () => { const el = await fixture(html`
Hey there
@@ -85,7 +85,7 @@ describe('lion-tooltip', () => { expect(el._overlayCtrl.isShown).to.equal(true); }); - it('should tooltip contains html when specified in tooltip content body', async () => { + it('contains html when specified in tooltip content body', async () => { const el = await fixture(html`
@@ -102,6 +102,57 @@ describe('lion-tooltip', () => { }); }); + describe('Arrow', () => { + it('shows when "has-arrow" is configured', async () => { + const el = await fixture(html` + +
+ This is Tooltip using overlay +
+ + + `); + expect(el._arrowNode).to.be.displayed; + }); + + it('makes sure positioning of the arrow is correct', async () => { + const el = await fixture(html` + +
+ Hey there +
+ + + `); + + el.opened = true; + + await el.repositionComplete; + + // Pretty sure we use flex for this now so that's why it fails + /* expect(getComputedStyle(el.__arrowElement).getPropertyValue('top')).to.equal( + '11px', + '30px (content height) - 8px = 22px, divided by 2 = 11px offset --> arrow is in the middle', + ); */ + + expect(getComputedStyle(el._arrowNode).getPropertyValue('left')).to.equal( + '-10px', + ` + arrow height is 8px so this offset should be taken into account to align the arrow properly, + as well as half the difference between width and height ((12 - 8) / 2 = 2) + `, + ); + }); + }); + describe('Positioning', () => { it('updates popper positioning correctly, without overriding other modifiers', async () => { const el = await fixture(html`