From 488ba6a0bf9d20f8ad514f5b62437a928a08b775 Mon Sep 17 00:00:00 2001 From: Erik Koopmans Date: Fri, 6 Jan 2023 01:14:31 -0500 Subject: [PATCH] fix: OPL separators on hidden items and edge cases --- .../demo/object-property-list.html | 13 +++++++ .../object-property-list-item.js | 27 +++++++++++++- .../object-property-list.js | 20 ++++++++-- .../test/object-property-list.test.js | 37 +++++++++++++++++-- .../object-property-list.visual-diff.html | 9 +++++ .../test/object-property-list.visual-diff.js | 1 + 6 files changed, 98 insertions(+), 9 deletions(-) diff --git a/components/object-property-list/demo/object-property-list.html b/components/object-property-list/demo/object-property-list.html index d9e0db65d7a..adf6678987a 100644 --- a/components/object-property-list/demo/object-property-list.html +++ b/components/object-property-list/demo/object-property-list.html @@ -59,6 +59,19 @@

Object Property List: Skeleton item count set

+

Object Property List: Hidden items

+ + + + +

Object Property List: No external whitespace (still contains internal whitespace in the component renderers)

diff --git a/components/object-property-list/object-property-list-item.js b/components/object-property-list/object-property-list-item.js index 6ffe9acc004..ef68460f9b0 100644 --- a/components/object-property-list/object-property-list-item.js +++ b/components/object-property-list/object-property-list-item.js @@ -11,6 +11,10 @@ import { SkeletonMixin } from '../skeleton/skeleton-mixin.js'; export class ObjectPropertyListItem extends SkeletonMixin(LitElement) { static get properties() { return { + /** + * @ignore + */ + hidden: { type: Boolean }, /** * Name of an optional icon to display * @type {string} @@ -21,6 +25,7 @@ export class ObjectPropertyListItem extends SkeletonMixin(LitElement) { * @type {string} */ text: { type: String }, + _showSeparator: { state: true }, }; } @@ -29,6 +34,9 @@ export class ObjectPropertyListItem extends SkeletonMixin(LitElement) { :host { vertical-align: middle; } + :host([hidden]) { + display: none; + } d2l-icon { height: 1.2857em; /* 18px desired height at main font size (14px), but using em to scale properly at smaller breakpoint. */ width: 1.2857em; @@ -57,6 +65,11 @@ export class ObjectPropertyListItem extends SkeletonMixin(LitElement) { `]; } + constructor() { + super(); + this._showSeparator = true; + } + render() { return html` ${this._renderIcon()} @@ -65,17 +78,27 @@ export class ObjectPropertyListItem extends SkeletonMixin(LitElement) { `; } + updated(changedProperties) { + super.updated(changedProperties); + if (changedProperties.has('hidden')) this._onHidden(); + } + + _onHidden() { + /** Dispatched when the visibility of the item changes */ + this.dispatchEvent(new CustomEvent('d2l-object-property-list-item-visibility-change', { bubbles: true, composed: true })); + } + _renderIcon() { return this.icon && !this.skeleton ? html`` : nothing; } _renderSeparator() { - return html` + return this._showSeparator ? html` - `; + ` : nothing; } _renderText() { diff --git a/components/object-property-list/object-property-list.js b/components/object-property-list/object-property-list.js index 278f7635feb..f3eac6727df 100644 --- a/components/object-property-list/object-property-list.js +++ b/components/object-property-list/object-property-list.js @@ -28,9 +28,6 @@ class ObjectPropertyList extends LocalizeCoreElement(SkeletonMixin(LitElement)) :host([hidden]) { display: none; } - ::slotted(:last-child), slot :last-child { - --d2l-object-property-list-item-separator-display: none; - } ::slotted([slot="status"]) { display: none; } @@ -41,6 +38,11 @@ class ObjectPropertyList extends LocalizeCoreElement(SkeletonMixin(LitElement)) `]; } + firstUpdated() { + this._slot = this.shadowRoot.querySelector('slot:not([name])'); + this.addEventListener('d2l-object-property-list-item-visibility-change', this._onItemsChanged); + } + render() { const slotContents = this.skeleton && this.skeletonCount > 0 ? [...Array(this.skeletonCount)].map(() => html` @@ -50,10 +52,20 @@ class ObjectPropertyList extends LocalizeCoreElement(SkeletonMixin(LitElement))
- ${slotContents} + ${slotContents}
`; } + + _onItemsChanged() { + this._setItemSeparatorVisibility(); + } + + _setItemSeparatorVisibility() { + const items = this._slot.assignedNodes().filter(item => item.tagName?.toLowerCase().includes('d2l-object-property-list-') && !item.hidden); + const lastIndex = items.length - 1; + items.forEach((item, i) => item._showSeparator = (i !== lastIndex)); + } } customElements.define('d2l-object-property-list', ObjectPropertyList); diff --git a/components/object-property-list/test/object-property-list.test.js b/components/object-property-list/test/object-property-list.test.js index 897fb7d17ce..a2df0a453da 100644 --- a/components/object-property-list/test/object-property-list.test.js +++ b/components/object-property-list/test/object-property-list.test.js @@ -6,12 +6,12 @@ import { aTimeout, expect, fixture, html } from '@open-wc/testing'; import { runConstructor } from '../../../tools/constructor-test-helper.js'; const validateSeparators = (elem, count) => { - const items = elem.querySelectorAll('d2l-object-property-list-item, d2l-object-property-list-item-link'); + const items = elem.querySelectorAll('d2l-object-property-list-item:not([hidden]), d2l-object-property-list-item-link:not([hidden])'); expect(items.length).to.equal(count); items.forEach((item, i) => { - const expectedDisplay = i === items.length - 1 ? 'none' : 'inline'; + const shouldHaveSeparator = i !== items.length - 1; const separator = item.shadowRoot.querySelector('.separator'); - expect(window.getComputedStyle(separator).display).to.equal(expectedDisplay); + expect(!!separator).to.equal(shouldHaveSeparator); }); }; @@ -65,6 +65,37 @@ describe('d2l-object-property-list', () => { await aTimeout(); // Required for Webkit only. validateSeparators(elem, 2); }); + + it('should display correctly if status slot is after all items', async() => { + const elem = await fixture(html` + + + + + + `); + validateSeparators(elem, 2); + }); + + it('should respond to hidden items', async() => { + const elem = await fixture(html` + + + + + + + `); + validateSeparators(elem, 2); + + elem.querySelector('#hidden').removeAttribute('hidden'); + await elem.updateComplete; + validateSeparators(elem, 3); + + elem.querySelector('#hidden').setAttribute('hidden', ''); + await elem.updateComplete; + validateSeparators(elem, 2); + }); }); }); diff --git a/components/object-property-list/test/object-property-list.visual-diff.html b/components/object-property-list/test/object-property-list.visual-diff.html index 09eb2ecb520..fa1c6d6d613 100644 --- a/components/object-property-list/test/object-property-list.visual-diff.html +++ b/components/object-property-list/test/object-property-list.visual-diff.html @@ -69,5 +69,14 @@ +
+ + + + + + +
+ diff --git a/components/object-property-list/test/object-property-list.visual-diff.js b/components/object-property-list/test/object-property-list.visual-diff.js index 9b899d0dddc..8fec1e2f6b8 100644 --- a/components/object-property-list/test/object-property-list.visual-diff.js +++ b/components/object-property-list/test/object-property-list.visual-diff.js @@ -24,6 +24,7 @@ describe('d2l-object-property-list', () => { { name: 'rtl', selector: '#rtl' }, { name: 'list-skeleton', selector: '#list-skeleton' }, { name: 'item-skeleton', selector: '#item-skeleton' }, + { name: 'hidden-items', selector: '#hidden-items' }, ].forEach((info) => { it(info.name, async function() { const rect = await visualDiff.getRect(page, info.selector);