fix: setFocus methods should wait for component to be ready.

[driskull]

Related Issue: #3580

Summary

fix: setFocus methods should wait for component to be ready..

See: https://github.com/ionic-team/stencil/blob/main/src/runtime/readme.md#lifecycle-process

async setFocus(): Promise<void> {
  const { el } = this;
  await componentReady(el);
  el.focus();
}

[driskull]

@benelan it seems like only await el.componentOnReady(); is needed and not requestAnimationframe. Can you confirm?

This leads me to think we should maybe be documenting componentOnReady or at least come up with our own method that wraps it so we can document how a user can wait for a component to be ready.

This following scenario requires a user use componentOnReady().

const modal = document.querySelector("calcite-modal");
const input = document.createElement("calcite-input");
content.appendChild(input);
await input.componentOnReady(); // Needed so that the input has been set up with all its internal elements
await modal.updateFocusTrapElements();
input.setFocus();

@eriklharper @benelan What are your thoughts on whether we should document the stencil method componentOnReady or document our own method that wraps it incase they change the method.

[benelan]

It depends on the component and the time it takes to load. Try it with inline-editable and I think it will only work with requestAnimationFrame. I had Kitty document componentOnReady based on the info we recieved from support:
https://developers.arcgis.com/calcite-design-system/web-concepts/#component-on-ready

But I'll clean it up this/next week with the correct info.

[driskull]

Try it with inline-editable and I think it will only work with requestAnimationFrame

Thats opposite of what I was seeing with that component LOL. When I had requestAnimationFrame inside setFocus it would break the disabled commonTest.

[benelan]

lol weird. Did you test it in the demos? When I tested in the demos with inline editable I'm pretty sure it needed requestAnimationFrame. Might need some tweaks to the disabled commonTest, idk.

Try using the demo on this branch: benelan/setFocus-refactor
And this codepen: https://codepen.io/benesri/pen/MWBgJBR

[eriklharper]

@eriklharper @benelan What are your thoughts on whether we should document the stencil method componentOnReady or document our own method that wraps it incase they change the method.

Wholeheartedly yes I think it would be beneficial to document this, not just for our users but for contributors as well. I've been bit by this so many times, I created my own snippet in VS Code for inserting this code into an HTML script tag for demo work:

"Component On Ready": {
    "prefix": "onready",
    "body": [
      "(async () => {",
      "	await customElements.whenDefined(\"$1\");",
      "	const element = document.querySelector(\"$1\");",
      "	await element.componentOnReady();",
      "	$0",
      "})();"
    ],
    "description": "HTML script snippet for waiting until a component is ready before interacting with it."
  }

[benelan]

Yeah I'll clean up the existing doc right now

[benelan]

Doc is updated on next: https://next.sites.afd.arcgis.com/calcite-design-system/web-concepts/#component-on-ready

[driskull]

@benelan every component seems to be good except for calcite-tile-select. Could you take a look?

[benelan]

code changes LGTM, I'll do some demo testing in the next couple days just to be sure

[benelan]

Did some testing and looks good!

[github-actions]

This PR has been automatically marked as stale because it has not had recent activity. Please close your PR if it is no longer relevant. Thank you for your contributions.

[jcfranco]

I think the componentOnReady approach won't work across the board as this API is only available in the lazy-loaded output target (www).

@benelan We should be able to proceed on this by reverting the switch to componentOnReady, resolving merge conflicts and updating the test helper, right?

WRT documentation, I think it's a good idea to add a snippet, but let's try to create an issue on Stencil's doc site. The burden of doc'ing componentOnReady shouldn't land on us.

[github-actions]

This PR has been automatically marked as stale because it has not had recent activity. Please close your PR if it is no longer relevant. Thank you for your contributions.