Skip to content

Commit

Permalink
Merge pull request #2151 from umbraco/v14/bugfix/focus-directive-late…
Browse files Browse the repository at this point in the history
…-focus

Bugfix: focus directive refactor to support late case
  • Loading branch information
nielslyngsoe authored Aug 5, 2024
2 parents 79323dd + 7f8b203 commit 8d6befc
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 11 deletions.
66 changes: 56 additions & 10 deletions src/packages/core/lit-element/directives/focus.lit-directive.ts
Original file line number Diff line number Diff line change
@@ -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() {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()}
Expand Down

0 comments on commit 8d6befc

Please sign in to comment.