From ad7d3a56cc868cb71cda00716cce9bd5be57e0c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niels=20Lyngs=C3=B8?= Date: Tue, 30 Jul 2024 14:03:01 +0200 Subject: [PATCH 1/5] refactor directive to support scenarios where things are not ready --- .../directives/focus.lit-directive.ts | 52 ++++++++++++++++--- ...ace-split-view-variant-selector.element.ts | 3 +- 2 files changed, 47 insertions(+), 8 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/lit-element/directives/focus.lit-directive.ts b/src/Umbraco.Web.UI.Client/src/packages/core/lit-element/directives/focus.lit-directive.ts index 5d237844ea..c8fb9156fa 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/core/lit-element/directives/focus.lit-directive.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/core/lit-element/directives/focus.lit-directive.ts @@ -1,9 +1,31 @@ import { AsyncDirective, directive, nothing, type ElementPart } from '@umbraco-cms/backoffice/external/lit'; +function isDescendant(parent: any, child: any) { + let node = child.parentNode; + while (node != null) { + if (node == parent) { + return true; + } + node = node.host ? node.host : node.parentNode; + } + return false; +} + +function hasFocus(current: any) { + if (current.shadowRoot) { + const node = current.shadowRoot.activeElement; + if (node) { + return hasFocus(node); + } + } + return current; +} + /** * The `focus` directive sets focus on the given element once its connected to the DOM. */ class UmbFocusDirective extends AsyncDirective { + static _latestElement?: HTMLElement; private _el?: HTMLElement; override render() { @@ -12,18 +34,34 @@ class UmbFocusDirective extends AsyncDirective { 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(); - }); + UmbFocusDirective._latestElement = 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 = () => { + if (this._el && this._el === UmbFocusDirective._latestElement) { + this._el.focus(); + if (hasFocus(document.activeElement)) { + setTimeout(this.#setFocus, 100); + } + } + }; + override disconnected() { + if (this._el === UmbFocusDirective._latestElement) { + UmbFocusDirective._latestElement = undefined; + } this._el = undefined; } diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/workspace/components/workspace-split-view/workspace-split-view-variant-selector.element.ts b/src/Umbraco.Web.UI.Client/src/packages/core/workspace/components/workspace-split-view/workspace-split-view-variant-selector.element.ts index 2fd9078243..e8a4f0ee96 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/core/workspace/components/workspace-split-view/workspace-split-view-variant-selector.element.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/core/workspace/components/workspace-split-view/workspace-split-view-variant-selector.element.ts @@ -209,10 +209,11 @@ export class UmbWorkspaceSplitViewVariantSelectorElement extends UmbLitElement { } override render() { + // TODO: Localize [NL] return html` Date: Tue, 30 Jul 2024 14:05:37 +0200 Subject: [PATCH 2/5] stop retry if success --- .../directives/focus.lit-directive.ts | 33 +++++++------------ 1 file changed, 12 insertions(+), 21 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/lit-element/directives/focus.lit-directive.ts b/src/Umbraco.Web.UI.Client/src/packages/core/lit-element/directives/focus.lit-directive.ts index c8fb9156fa..51337f192f 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/core/lit-element/directives/focus.lit-directive.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/core/lit-element/directives/focus.lit-directive.ts @@ -1,16 +1,5 @@ import { AsyncDirective, directive, nothing, type ElementPart } from '@umbraco-cms/backoffice/external/lit'; -function isDescendant(parent: any, child: any) { - let node = child.parentNode; - while (node != null) { - if (node == parent) { - return true; - } - node = node.host ? node.host : node.parentNode; - } - return false; -} - function hasFocus(current: any) { if (current.shadowRoot) { const node = current.shadowRoot.activeElement; @@ -25,16 +14,16 @@ function hasFocus(current: any) { * The `focus` directive sets focus on the given element once its connected to the DOM. */ class UmbFocusDirective extends AsyncDirective { - static _latestElement?: HTMLElement; - private _el?: HTMLElement; + static #next?: HTMLElement; + #el?: HTMLElement; override render() { return nothing; } override update(part: ElementPart) { - if (this._el !== part.element) { - UmbFocusDirective._latestElement = this._el = part.element as HTMLElement; + if (this.#el !== part.element) { + UmbFocusDirective.#next = this.#el = part.element as HTMLElement; this.#setFocus(); } return nothing; @@ -50,19 +39,21 @@ class UmbFocusDirective extends AsyncDirective { * cause Lit does not re-render the element but also notice reconnect callback on the directive is not triggered either. [NL] */ #setFocus = () => { - if (this._el && this._el === UmbFocusDirective._latestElement) { - this._el.focus(); - if (hasFocus(document.activeElement)) { + if (this.#el && this.#el === UmbFocusDirective.#next) { + this.#el.focus(); + if (hasFocus(document.activeElement) !== this.#el) { setTimeout(this.#setFocus, 100); + } else { + UmbFocusDirective.#next = undefined; } } }; override disconnected() { - if (this._el === UmbFocusDirective._latestElement) { - UmbFocusDirective._latestElement = undefined; + if (this.#el === UmbFocusDirective.#next) { + UmbFocusDirective.#next = undefined; } - this._el = undefined; + this.#el = undefined; } //override reconnected() {} From a3237e53e7d3d2ca3ac24e6dbe791384f83d3d78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niels=20Lyngs=C3=B8?= Date: Tue, 30 Jul 2024 14:07:38 +0200 Subject: [PATCH 3/5] comments and clearing --- .../core/lit-element/directives/focus.lit-directive.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/lit-element/directives/focus.lit-directive.ts b/src/Umbraco.Web.UI.Client/src/packages/core/lit-element/directives/focus.lit-directive.ts index 51337f192f..e806e4d11b 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/core/lit-element/directives/focus.lit-directive.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/core/lit-element/directives/focus.lit-directive.ts @@ -16,6 +16,7 @@ function hasFocus(current: any) { class UmbFocusDirective extends AsyncDirective { static #next?: HTMLElement; #el?: HTMLElement; + #timeout?: number; override render() { return nothing; @@ -39,10 +40,16 @@ class UmbFocusDirective extends AsyncDirective { * 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) { - setTimeout(this.#setFocus, 100); + this.#timeout = setTimeout(this.#setFocus, 100) as unknown as number; } else { UmbFocusDirective.#next = undefined; } From eda9b1041a2db07815e02ba8375567cc61b0bf46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niels=20Lyngs=C3=B8?= Date: Tue, 30 Jul 2024 14:54:50 +0200 Subject: [PATCH 4/5] fix inner focused elements --- .../directives/focus.lit-directive.ts | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/lit-element/directives/focus.lit-directive.ts b/src/Umbraco.Web.UI.Client/src/packages/core/lit-element/directives/focus.lit-directive.ts index e806e4d11b..f4e7032fd8 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/core/lit-element/directives/focus.lit-directive.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/core/lit-element/directives/focus.lit-directive.ts @@ -1,13 +1,23 @@ import { AsyncDirective, directive, nothing, type ElementPart } from '@umbraco-cms/backoffice/external/lit'; - -function hasFocus(current: any) { +/** + * + * 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); + return hasFocus(node, target); } } - return current; + return false; } /** @@ -48,7 +58,7 @@ class UmbFocusDirective extends AsyncDirective { // 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) { + if (hasFocus(document.activeElement, this.#el) === false) { this.#timeout = setTimeout(this.#setFocus, 100) as unknown as number; } else { UmbFocusDirective.#next = undefined; From ced2cdf781bfd0ad55d2a04c83405d0e56193be2 Mon Sep 17 00:00:00 2001 From: leekelleher Date: Mon, 5 Aug 2024 08:50:20 +0100 Subject: [PATCH 5/5] Localized workspace split view name input label --- .../workspace-split-view-variant-selector.element.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/src/packages/core/workspace/components/workspace-split-view/workspace-split-view-variant-selector.element.ts b/src/Umbraco.Web.UI.Client/src/packages/core/workspace/components/workspace-split-view/workspace-split-view-variant-selector.element.ts index e8a4f0ee96..c4bfbf0cab 100644 --- a/src/Umbraco.Web.UI.Client/src/packages/core/workspace/components/workspace-split-view/workspace-split-view-variant-selector.element.ts +++ b/src/Umbraco.Web.UI.Client/src/packages/core/workspace/components/workspace-split-view/workspace-split-view-variant-selector.element.ts @@ -209,11 +209,10 @@ export class UmbWorkspaceSplitViewVariantSelectorElement extends UmbLitElement { } override render() { - // TODO: Localize [NL] return html`