fix(shared): ensure the "updateComplete" in Focusable is stable

packages/action-group/test/action-group.test.ts

@@ -770,6 +770,8 @@ describe('ActionGroup', () => {
 
         await elementUpdated(el);
 
+        await Promise.all(el.buttons.map((button) => elementUpdated(button)));
+
         const firstButton = el.querySelector('.first') as ActionButton;
         const secondButton = el.querySelector('.second') as ActionButton;
         const thirdButton = el.querySelector('.third') as ActionButton;
@@ -819,6 +821,8 @@ describe('ActionGroup', () => {
 
         await elementUpdated(el);
 
+        await Promise.all(el.buttons.map((button) => elementUpdated(button)));
+
         const firstButton = el.querySelector('.first') as ActionButton;
         const secondButton = el.querySelector('.second') as ActionButton;
         const thirdButton = el.querySelector('.third') as ActionButton;

packages/button/test/button.test.ts

@@ -97,6 +97,41 @@ describe('Button', () => {
         expect(el).to.not.be.undefined;
         await expect(el).to.be.accessible();
     });
+    it('has a stable/predictable `updateComplete`', async () => {
+        const test = await fixture<HTMLDivElement>(
+            html`
+                <div></div>
+            `
+        );
+
+        let keydownTime = -1;
+        let updateComplete1 = -1;
+        let updateComplete2 = -1;
+
+        const el = document.createElement('sp-button');
+        el.autofocus = true;
+        el.addEventListener('keydown', () => {
+            keydownTime = performance.now();
+        });
+        el.updateComplete.then(() => {
+            updateComplete1 = performance.now();
+        });
+        el.updateComplete.then(() => {
+            updateComplete2 = performance.now();
+        });
+        test.append(el);
+        // don't use elementUpdated(), as it is under test...
+        await nextFrame();
+        await nextFrame();
+        await nextFrame();
+        await nextFrame();
+
+        expect(keydownTime, 'keydown happened').to.not.eq(-1);
+        expect(updateComplete1, 'first update complete happened').to.not.eq(-1);
+        expect(updateComplete2, 'first update complete happened').to.not.eq(-1);
+        expect(updateComplete1).lte(updateComplete2);
+        expect(updateComplete2).lte(keydownTime);
+    });
     it('manages "role"', async () => {
         const el = await fixture<Button>(
             html`

packages/top-nav/test/top-nav.test.ts

@@ -47,11 +47,14 @@ describe('TopNav', () => {
 
         await expect(el).to.be.accessible();
     });
-    it('updates indicator size when Nav Item conten changes', async () => {
+    it('updates indicator size when Nav Item content changes', async () => {
         const el = await fixture<TopNav>(Selected());
 
         await elementUpdated(el);
 
+        const items = [...el.querySelectorAll('sp-top-nav-item')];
+        await Promise.all(items.map((item) => elementUpdated(item)));
+
         const indicator = el.shadowRoot.querySelector(
             '#selection-indicator'
         ) as HTMLDivElement;
@@ -64,10 +67,14 @@ describe('TopNav', () => {
 
         // Wait for slotchange time before continuing the test.
         await nextFrame();
+        await nextFrame();
 
         const { width: widthEnd } = indicator.getBoundingClientRect();
 
-        expect(widthStart).to.be.greaterThan(widthEnd);
+        expect(
+            widthStart,
+            `${widthStart} is not greater than ${widthEnd}`
+        ).to.be.greaterThan(widthEnd);
     });
     it('can have an item removed', async () => {
         const el = await fixture<TopNav>(Selected());

test/benchmark/helpers.ts

@@ -26,6 +26,10 @@ declare global {
 
 /**
  * Runs `callback` shortly after the next browser Frame is produced.
+ *
+ * Adopted from https://webperf.tips/tip/measuring-paint-time/ but possibly replaceable by
+ * a performance observer with additional work:
+ * https://developer.mozilla.org/en-US/docs/Web/API/PerformanceObserver/PerformanceObserver
  */
 function runAfterFramePaint(callback: () => void) {
     // Queue a "before Render Steps" callback via requestAnimationFrame.

tools/shared/src/focusable.ts

@@ -270,29 +270,31 @@ export class Focusable extends FocusVisiblePolyfillMixin(SpectrumElement) {
 
     protected override async getUpdateComplete(): Promise<boolean> {
         const complete = (await super.getUpdateComplete()) as boolean;
-        if (this._recentlyConnected) {
-            this._recentlyConnected = false;
-            // If at connect time the [autofocus] content is placed within
-            // content that needs to be "hidden" by default, it would need to wait
-            // two rAFs for animations to be triggered on that content in
-            // order for the [autofocus] to become "visisble" and have its
-            // focus() capabilities enabled.
-            //
-            // Await this with `getUpdateComplete` so that the element cannot
-            // become "ready" until `manageFocus` has occured.
-            await nextFrame();
-            await nextFrame();
-        }
+        await this.autofocusReady;
         return complete;
     }
 
-    private _recentlyConnected = false;
+    private autofocusReady = Promise.resolve();
 
     public override connectedCallback(): void {
         super.connectedCallback();
-        this._recentlyConnected = true;
-        this.updateComplete.then(() => {
-            this.manageAutoFocus();
-        });
+        if (this.autofocus) {
+            this.autofocusReady = new Promise(async (res) => {
+                // If at connect time the [autofocus] content is placed within
+                // content that needs to be "hidden" by default, it would need to wait
+                // two rAFs for animations to be triggered on that content in
+                // order for the [autofocus] to become "visisble" and have its
+                // focus() capabilities enabled.
+                //
+                // Await this with `getUpdateComplete` so that the element cannot
+                // become "ready" until `manageFocus` has occured.
+                await nextFrame();
+                await nextFrame();
+                res();
+            });
+            this.updateComplete.then(() => {
+                this.manageAutoFocus();
+            });
+        }
     }
 }