From 7aa71f7887fe58fba81107dc9dc55ed9c3b5ba7e Mon Sep 17 00:00:00 2001
From: Margaree Peacocke <margaree.p@gmail.com>
Date: Mon, 23 Sep 2024 09:22:06 -0400
Subject: [PATCH] feat: Dialog and Dialog-fullscreen: Make title-text required

---
 components/dialog/demo/dialog-confirm.html       |  2 +-
 components/dialog/dialog-fullscreen.js           |  7 ++++++-
 components/dialog/dialog.js                      |  8 +++++++-
 components/dialog/test/dialog-fullscreen.axe.js  |  5 -----
 components/dialog/test/dialog-fullscreen.test.js | 13 ++++++++++++-
 components/dialog/test/dialog.axe.js             |  5 -----
 components/dialog/test/dialog.test.js            | 11 +++++++++++
 7 files changed, 37 insertions(+), 14 deletions(-)

diff --git a/components/dialog/demo/dialog-confirm.html b/components/dialog/demo/dialog-confirm.html
index a0097d6b722..fa8e595e974 100644
--- a/components/dialog/demo/dialog-confirm.html
+++ b/components/dialog/demo/dialog-confirm.html
@@ -22,7 +22,7 @@ <h2>Confirm Dialog</h2>
 			<d2l-demo-snippet>
 				<template>
 					<d2l-button id="openConfirm">Show Confirm</d2l-button>
-					<d2l-dialog-confirm id="confirm" title-text="Confirm Title" text="Are you sure you want more cookies?">
+					<d2l-dialog-confirm id="confirm" text="Are you sure you want more cookies?">
 						<d2l-button slot="footer" primary data-dialog-action="ok">Yes</d2l-button>
 						<d2l-button slot="footer" data-dialog-action>No</d2l-button>
 					</d2l-dialog-confirm>
diff --git a/components/dialog/dialog-fullscreen.js b/components/dialog/dialog-fullscreen.js
index ae7cf61e40c..dfaffcda631 100644
--- a/components/dialog/dialog-fullscreen.js
+++ b/components/dialog/dialog-fullscreen.js
@@ -8,6 +8,7 @@ import { DialogMixin } from './dialog-mixin.js';
 import { dialogStyles } from './dialog-styles.js';
 import { getUniqueId } from '../../helpers/uniqueId.js';
 import { LocalizeCoreElement } from '../../helpers/localize-core-element.js';
+import { PropertyRequiredMixin } from '../../mixins/property-required/property-required-mixin.js';
 import { styleMap } from 'lit/directives/style-map.js';
 
 const mediaQueryList = window.matchMedia('(max-width: 615px), (max-height: 420px) and (max-width: 900px)');
@@ -18,7 +19,7 @@ const mediaQueryList = window.matchMedia('(max-width: 615px), (max-height: 420px
  * @slot - Default slot for content inside dialog
  * @slot footer - Slot for footer content such as workflow buttons
  */
-class DialogFullscreen extends LocalizeCoreElement(AsyncContainerMixin(DialogMixin(LitElement))) {
+class DialogFullscreen extends PropertyRequiredMixin(LocalizeCoreElement(AsyncContainerMixin(DialogMixin(LitElement)))) {
 
 	static get properties() {
 		return {
@@ -32,6 +33,10 @@ class DialogFullscreen extends LocalizeCoreElement(AsyncContainerMixin(DialogMix
 			 * @type {boolean}
 			 */
 			noPadding: { type: Boolean, reflect: true, attribute: 'no-padding' },
+			/**
+			 * REQUIRED: the title for the dialog
+			 */
+			titleText: { type: String, attribute: 'title-text', required: true },
 			/**
 			 * The preferred width (unit-less) for the dialog. Maximum 1170.
 			 */
diff --git a/components/dialog/dialog.js b/components/dialog/dialog.js
index 4b24c8ccd49..6dbc9317b94 100644
--- a/components/dialog/dialog.js
+++ b/components/dialog/dialog.js
@@ -9,6 +9,7 @@ import { dialogStyles } from './dialog-styles.js';
 import { getUniqueId } from '../../helpers/uniqueId.js';
 import { ifDefined } from 'lit/directives/if-defined.js';
 import { LocalizeCoreElement } from '../../helpers/localize-core-element.js';
+import { PropertyRequiredMixin } from '../../mixins/property-required/property-required-mixin.js';
 import { styleMap } from 'lit/directives/style-map.js';
 
 const mediaQueryList = window.matchMedia('(max-width: 615px), (max-height: 420px) and (max-width: 900px)');
@@ -19,7 +20,7 @@ const mediaQueryList = window.matchMedia('(max-width: 615px), (max-height: 420px
  * @slot - Default slot for content inside dialog
  * @slot footer - Slot for footer content such as workflow buttons
  */
-class Dialog extends LocalizeCoreElement(AsyncContainerMixin(DialogMixin(LitElement))) {
+class Dialog extends PropertyRequiredMixin(LocalizeCoreElement(AsyncContainerMixin(DialogMixin(LitElement)))) {
 
 	static get properties() {
 		return {
@@ -43,6 +44,11 @@ class Dialog extends LocalizeCoreElement(AsyncContainerMixin(DialogMixin(LitElem
 			 */
 			fullHeight: { type: Boolean, attribute: 'full-height' },
 
+			/**
+			 * REQUIRED: the title for the dialog
+			 */
+			titleText: { type: String, attribute: 'title-text', required: true },
+
 			/**
 			 * The preferred width (unit-less) for the dialog
 			 */
diff --git a/components/dialog/test/dialog-fullscreen.axe.js b/components/dialog/test/dialog-fullscreen.axe.js
index 1cd78678406..52b872687de 100644
--- a/components/dialog/test/dialog-fullscreen.axe.js
+++ b/components/dialog/test/dialog-fullscreen.axe.js
@@ -25,11 +25,6 @@ describe('d2l-dialog-fullscreen', () => {
 		await expect(el).to.be.accessible();
 	});
 
-	it.skip('no title-text', async() => {
-		const el = await fixture(html`<d2l-dialog-fullscreen opened>My content</d2l-dialog-fullscreen>`);
-		await expect(el).to.be.accessible();
-	});
-
 	it.skip('tall content', async() => {
 		const el = await fixture(html`
 			<d2l-dialog-fullscreen opened title-text="My Dialog">
diff --git a/components/dialog/test/dialog-fullscreen.test.js b/components/dialog/test/dialog-fullscreen.test.js
index 02fea06b648..b03684c4641 100644
--- a/components/dialog/test/dialog-fullscreen.test.js
+++ b/components/dialog/test/dialog-fullscreen.test.js
@@ -1,5 +1,6 @@
 import '../dialog-fullscreen.js';
-import { runConstructor } from '@brightspace-ui/testing';
+import { expect, fixture, html, runConstructor } from '@brightspace-ui/testing';
+import { createMessage } from '../../../mixins/property-required/property-required-mixin.js';
 
 describe('d2l-dialog-fullscreen', () => {
 
@@ -11,4 +12,14 @@ describe('d2l-dialog-fullscreen', () => {
 
 	});
 
+	describe('properties', () => {
+
+		it('throws error when no title-text', async() => {
+			const el = await fixture(html`<d2l-dialog-fullscreen></d2l-dialog-fullscreen>`);
+			expect(() => el.flushRequiredPropertyErrors())
+				.to.throw(TypeError, createMessage(el, 'title-text'));
+		});
+
+	});
+
 });
diff --git a/components/dialog/test/dialog.axe.js b/components/dialog/test/dialog.axe.js
index 30e4918ca44..0137adbcf74 100644
--- a/components/dialog/test/dialog.axe.js
+++ b/components/dialog/test/dialog.axe.js
@@ -30,11 +30,6 @@ describe('d2l-dialog', () => {
 		await expect(el).to.be.accessible();
 	});
 
-	it.skip('no title-text', async() => {
-		const el = await fixture(html`<d2l-dialog opened>My content</d2l-dialog>`);
-		await expect(el).to.be.accessible();
-	});
-
 	it.skip('tall content', async() => {
 		const el = await fixture(html`
 			<d2l-dialog opened title-text="My Dialog">
diff --git a/components/dialog/test/dialog.test.js b/components/dialog/test/dialog.test.js
index 787a56320b4..20ffc069255 100644
--- a/components/dialog/test/dialog.test.js
+++ b/components/dialog/test/dialog.test.js
@@ -1,5 +1,6 @@
 import '../dialog.js';
 import { expect, fixture, oneEvent, runConstructor } from '@brightspace-ui/testing';
+import { createMessage } from '../../../mixins/property-required/property-required-mixin.js';
 import { getComposedActiveElement } from '../../../helpers/focus.js';
 import { html } from 'lit';
 
@@ -13,6 +14,16 @@ describe('d2l-dialog', () => {
 
 	});
 
+	describe('properties', () => {
+
+		it('throws error when no title-text', async() => {
+			const el = await fixture(html`<d2l-dialog></d2l-dialog>`);
+			expect(() => el.flushRequiredPropertyErrors())
+				.to.throw(TypeError, createMessage(el, 'title-text'));
+		});
+
+	});
+
 	describe('focus management', () => {
 
 		it('should focus on close button if no focusable elements inside', async() => {