fix: OPL separators on hidden items and edge cases

BrightspaceUI · Jan 19, 2023 · 488ba6a · 488ba6a
1 parent 96785f3
commit 488ba6a
Show file tree

Hide file tree

Showing 6 changed files with 98 additions and 9 deletions.
diff --git a/components/object-property-list/demo/object-property-list.html b/components/object-property-list/demo/object-property-list.html
@@ -59,6 +59,19 @@ <h2>Object Property List: Skeleton item count set</h2>
 				</template>
 			</d2l-demo-snippet>
 
+			<h2>Object Property List: Hidden items</h2>
+
+			<d2l-demo-snippet overflow-hidden>
+				<template>
+					<d2l-object-property-list>
+						<d2l-object-property-list-item text="Item 1"></d2l-object-property-list-item>
+						<d2l-object-property-list-item text="Item 2 (Hidden)" hidden></d2l-object-property-list-item>
+						<d2l-object-property-list-item text="Item 3"></d2l-object-property-list-item>
+						<d2l-object-property-list-item text="Item 4 (Hidden)" hidden></d2l-object-property-list-item>
+					</d2l-object-property-list>
+				</template>
+			</d2l-demo-snippet>
+
 			<h2>Object Property List: No external whitespace (still contains internal whitespace in the component renderers)</h2>
 
 			<d2l-demo-snippet overflow-hidden>

diff --git 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`<d2l-icon icon="${this.icon}" class="item-icon"></d2l-icon>` : nothing;
 	}
 
 	_renderSeparator() {
-		return html`
+		return this._showSeparator ? html`
 			<span class="separator">
 				<d2l-screen-reader-pause></d2l-screen-reader-pause>
 				<d2l-icon icon="tier1:bullet"></d2l-icon>
 			</span>
-		`;
+		` : nothing;
 	}
 
 	_renderText() {

diff --git 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`
 			<d2l-object-property-list-item text="${this.localize('components.object-property-list.item-placeholder-text')}" skeleton></d2l-object-property-list-item>
@@ -50,10 +52,20 @@ class ObjectPropertyList extends LocalizeCoreElement(SkeletonMixin(LitElement))
 			<div class="d2l-body-small">
 				<slot name="status"></slot>
 				<d2l-screen-reader-pause></d2l-screen-reader-pause>
-				<slot>${slotContents}</slot>
+				<slot @slotchange="${this._onItemsChanged}">${slotContents}</slot>
 			</div>
 		`;
 	}
+
+	_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
@@ -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`
+				<d2l-object-property-list>
+					<d2l-object-property-list-item text="Example item 1"></d2l-object-property-list-item>
+					<d2l-object-property-list-item text="Example item 2"></d2l-object-property-list-item>
+					<d2l-status-indicator slot="status" state="default" text="Status"></d2l-status-indicator>
+				</d2l-object-property-list>
+			`);
+			validateSeparators(elem, 2);
+		});
+
+		it('should respond to hidden items', async() => {
+			const elem = await fixture(html`
+				<d2l-object-property-list>
+					<d2l-status-indicator slot="status" state="default" text="Status"></d2l-status-indicator>
+					<d2l-object-property-list-item text="Example item 1"></d2l-object-property-list-item>
+					<d2l-object-property-list-item text="Example item 2"></d2l-object-property-list-item>
+					<d2l-object-property-list-item text="Example item 3" hidden id="hidden"></d2l-object-property-list-item>
+				</d2l-object-property-list>
+			`);
+			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
@@ -69,5 +69,14 @@
 		</d2l-object-property-list>
 	</div>
 
+	<div class="visual-diff" id="hidden-items">
+		<d2l-object-property-list>
+			<d2l-object-property-list-item text="Item 1"></d2l-object-property-list-item>
+			<d2l-object-property-list-item text="Item 2 (Hidden)" hidden></d2l-object-property-list-item>
+			<d2l-object-property-list-item text="Item 3"></d2l-object-property-list-item>
+			<d2l-object-property-list-item text="Item 4 (Hidden)" hidden></d2l-object-property-list-item>
+		</d2l-object-property-list>
+	</div>
+
 </body>
 </html>
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
@@ -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);