Merge pull request #2151 from umbraco/v14/bugfix/focus-directive-late-focus

Bugfix: focus directive refactor to support late case

Author: nielslyngsoe

src/packages/core/lit-element/directives/focus.lit-directive.ts

@@ -1,30 +1,76 @@
 import { AsyncDirective, directive, nothing, type ElementPart } from '@umbraco-cms/backoffice/external/lit';
+/**
+ *
+ * test if a element has focus
+ * this also returns true if the focused element is a child of the target.
+ * @param current
+ * @param target
+ * @returns bool
+ */
+function hasFocus(current: any, target: HTMLElement): boolean {
+	if (current === target) {
+		return true;
+	}
+	if (current.shadowRoot) {
+		const node = current.shadowRoot.activeElement;
+		if (node) {
+			return hasFocus(node, target);
+		}
+	}
+	return false;
+}
 
 /**
  * The `focus` directive sets focus on the given element once its connected to the DOM.
  */
 class UmbFocusDirective extends AsyncDirective {
-	private _el?: HTMLElement;
+	static #next?: HTMLElement;
+	#el?: HTMLElement;
+	#timeout?: number;
 
 	override render() {
 		return nothing;
 	}
 
 	override update(part: ElementPart) {
-		if (this._el !== part.element) {
-			// This does feel wrong that we need to wait one render. [NL]
-			// Because even if our elements focus method is implemented so it can be called initially, my research shows that calling the focus method at this point is too early, thought the element is connected to the DOM and the focus method is available. [NL]
-			// This smells a bit like the DOMPart of which the directive is in is not connected to the main DOM yet, and therefor cant receive focus. [NL]
-			// Which is why we need to await one render: [NL]
-			requestAnimationFrame(() => {
-				(this._el = part.element as HTMLElement).focus();
-			});
+		if (this.#el !== part.element) {
+			UmbFocusDirective.#next = this.#el = part.element as HTMLElement;
+			this.#setFocus();
 		}
 		return nothing;
 	}
 
+	/**
+	 * This method tries to set focus, if it did not succeed, it will try again.
+	 * It always tests against the latest element, because the directive can be used multiple times in the same render.
+	 * This is NOT needed because the elements focus method isn't ready to be called, but due to something with rendering of the DOM.
+	 * But I'm not completely sure at this movement why the browser does not accept the focus call.
+	 * But I have tested that everything is in place for it to be good, so something else must have an effect,
+	 * setting the focus somewhere else, maybe a re-appending of some sort?
+	 * cause Lit does not re-render the element but also notice reconnect callback on the directive is not triggered either. [NL]
+	 */
+	#setFocus = () => {
+		// Make sure we clear the timeout, so we don't get multiple timeouts running.
+		if (this.#timeout) {
+			clearTimeout(this.#timeout);
+			this.#timeout = undefined;
+		}
+		// If this is the next element to focus, then try to focus it.
+		if (this.#el && this.#el === UmbFocusDirective.#next) {
+			this.#el.focus();
+			if (hasFocus(document.activeElement, this.#el) === false) {
+				this.#timeout = setTimeout(this.#setFocus, 100) as unknown as number;
+			} else {
+				UmbFocusDirective.#next = undefined;
+			}
+		}
+	};
+
 	override disconnected() {
-		this._el = undefined;
+		if (this.#el === UmbFocusDirective.#next) {
+			UmbFocusDirective.#next = undefined;
+		}
+		this.#el = undefined;
 	}
 
 	//override reconnected() {}

src/packages/core/workspace/components/workspace-split-view/workspace-split-view-variant-selector.element.ts

@@ -212,7 +212,7 @@ export class UmbWorkspaceSplitViewVariantSelectorElement extends UmbLitElement {
 		return html`
 			<uui-input
 				id="name-input"
-				label="Document name (TODO: Localize)"
+				label=${this.localize.term('placeholders_entername')}
 				.value=${this._name ?? ''}
 				@input=${this.#handleInput}
 				${umbFocus()}