fix: tooltip > don't immediately display when added to disabled target (

#5369) * +initiallyFocused check, fix redundant target update * add targetDisabled check * swap to queueMicrotask * conditional to updatePos * update expression * use getComposedActiveElement * always update position if showing * revert demo page * Change Implementation The existing solution broke force-show workflow, this seems to solve the issue and still fix the defect on the ticket. * Fix/add tests * Updating vdiff goldens (#5381) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Updating vdiff goldens (#5382) * Handle Property changes before update cycle * Update components/tooltip/demo/tooltip.html Co-authored-by: Dave Lockhart <[email protected]> --------- Co-authored-by: GZolla <[email protected]> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Dave Lockhart <[email protected]>
BrightspaceUI · Feb 21, 2025 · caf950a · caf950a
1 parent 2c9954c
commit caf950a
Show file tree

Hide file tree

Showing 4 changed files with 34 additions and 5 deletions.
diff --git a/.vdiff.json b/.vdiff.json
@@ -7,7 +7,7 @@
 	],
 	"system": {
 		"platform": "linux",
-		"release": "6.8.0-1020-azure",
+		"release": "6.8.0-1021-azure",
 		"arch": "x64"
 	}
 }
diff --git a/components/tooltip/demo/tooltip.html b/components/tooltip/demo/tooltip.html
@@ -202,7 +202,14 @@ <h2>Help Tooltip</h2>
 			</template>
 		</d2l-demo-snippet>
 
+		<d2l-demo-snippet>
+			<template>
+				<d2l-button id="toggle" disabled-tooltip="This tooltip shows if button is disabled, but only after refocusing">Toggle Disable</d2l-button>
+			</template>
+		</d2l-demo-snippet>
+
 		<script>
+			document.querySelector('#toggle').addEventListener('click', e => e.target.disabled = !e.target.disabled);
 			document.addEventListener('d2l-tooltip-show', e => console.log('d2l-tooltip-show', e.target));
 			document.addEventListener('d2l-tooltip-hide', e => console.log('d2l-tooltip-hide', e.target));
 		</script>

diff --git a/components/tooltip/test/tooltip.test.js b/components/tooltip/test/tooltip.test.js
@@ -223,6 +223,20 @@ describe('d2l-tooltip', () => {
 
 		});
 
+		it('should not show if added to a disabled target that already has focus', async() => {
+
+			const target = tooltipFixture.querySelector('#explicit-target');
+			target.disabled = true;
+			await focusElem(target);
+
+			const dynamicTooltip = document.createElement('d2l-tooltip');
+			dynamicTooltip.for = target.id;
+			tooltipFixture.appendChild(dynamicTooltip);
+
+			await aTimeout(100);
+			expect(dynamicTooltip.showing).to.be.false;
+		});
+
 	});
 
 	describe('delay', () => {

diff --git a/components/tooltip/tooltip.js b/components/tooltip/tooltip.js
@@ -544,8 +544,8 @@ class Tooltip extends RtlMixin(LitElement) {
 		;
 	}
 
-	updated(changedProperties) {
-		super.updated(changedProperties);
+	willUpdate(changedProperties) {
+		super.willUpdate(changedProperties);
 
 		changedProperties.forEach((_, prop) => {
 			if (prop === 'for') {
@@ -950,9 +950,17 @@ class Tooltip extends RtlMixin(LitElement) {
 	}
 
 	_updateTarget() {
+		const newTarget = this._findTarget();
+		if (this._target === newTarget) {
+			return;
+		}
+
 		this._removeListeners();
-		this._target = this._findTarget();
+		this._target = newTarget;
+
 		if (this._target) {
+			const targetDisabled = this._target.hasAttribute('disabled') || this._target.getAttribute('aria-disabled') === 'true';
+
 			const isInteractive = this._isInteractive(this._target);
 			this.id = this.id || getUniqueId();
 			this.setAttribute('role', 'tooltip');
@@ -971,7 +979,7 @@ class Tooltip extends RtlMixin(LitElement) {
 			}
 			if (this.showing) {
 				this.updatePosition();
-			} else if (isComposedAncestor(this._target, getComposedActiveElement())) {
+			} else if (!targetDisabled && isComposedAncestor(this._target, getComposedActiveElement())) {
 				this._onTargetFocus();
 			}
 		}