Infinite loop when calling setFocus on new calcite-input

[nwhittaker]

Actual Behavior

Programmatically inserting a <calcite-input> into a <calcite-modal> and calling setFocus() on the input results in an infinite loop that locks up the window.

Expected Behavior

Programmatically inserting a <calcite-input> into a <calcite-modal> and calling setFocus() on it shifts focus to the input.

Reproduction Sample

https://codepen.io/nwhittaker-esri/pen/VwdEody

Reproduction Steps

1. Visit the sample.
2. Wait for the modal to open and click the Add input button.
3. Notice the preview area locks up after a few seconds, and the browser presents some sort of Page Unresponsive dialog after awhile.

Reproduction Version

next.653

Relevant Info

The regression appears to have been introduced over a couple releases:

1. next.632: refactor(modal): Update modal to use focus-trap module. #5676
   • Browser does not lock up, but new input elements do not gain immediate focus.
2. next.636: fix: setFocus methods should wait for the component to be loaded #5749
   • Browser locks up when setting focus on the new input element.

The likely root of the problem is the new focus trap library isn't aware of the new input element yet, so it thinks an element outside of the modal is being focused. The loop that emerges is:

1. setFocus() called on the new <calcite-input>.
2. focus() called on the shadow <input>.
3. The focus traps's focusin handler reverts focus to the previously focused element.
4. The <calcite-input> element's focus handler runs and calls setFocus() on itself which leads back to step 1.

Regression?

next.631

Impact

The Field Maps web app uses a modal for editing coded value domains. When a user wants to add a new coded value, a new row of inputs is inserted and focus should shift to the row's first input. This is blocking us from upgrading Calcite beyond next.631.

Esri team

ArcGIS Field Apps

[benelan]

Yeah, now this one isn't a doc cleanup issue 😬

Triaging to the current sprint, cc @driskull @geospatialem

[driskull]

I think this might be because when the modal is opened it is trapping focus. We might need to provide an option to disable focus trap for modal here.

[driskull]

@eriklharper can you take this one? It seems like an issue with calcite-input. It works fine if a native input is used.

I narrowed it down to a infinite loop in setFocus/input

When setFocus() is called it focuses on the child element.

https://github.com/Esri/calcite-components/blob/0a658c3e4fca54a0b3275b85943dd519a8af6572/src/components/input/input.tsx#L535

The problem is when the input is focused, it calls setFocus() again if the target doesn't match the slottedActionEl. So then it gets into a never ending loop.

https://github.com/Esri/calcite-components/blob/0a658c3e4fca54a0b3275b85943dd519a8af6572/src/components/input/input.tsx#L623-L629

[driskull]

It actually has nothing to do with the modal. If you append the input's to the body it freezes as well.

I'm guessing input-number and input-text are also affected

[eriklharper]

It actually has nothing to do with the modal. If you append the input's to the body it freezes as well.

Could you share a codepen that reproduces this? I can't repro that with code like this (based on your description, assuming a button is wired up to fire this callback):

function addInput() {
      const input = document.createElement("calcite-input");
      document.body.appendChild(input);
      input.setFocus();
    }

[driskull]

I was wrong, it does have to do with the focus trap but not modal specifically. It seems to fight the focus trap because of that infinite loop it gets into.

here's a reproduction:

https://codepen.io/driskull/pen/JjZzdBN?editors=1010

[driskull]

If you change the JS to just use a native input it works fine.

[eriklharper]

I was wrong, it does have to do with the focus trap but not modal specifically.

This is my finding as well after investigating.

[github-actions]

Installed and assigned for verification.

[geospatialem]

Verified in beta.668 with the following Codepen using the disableFocusTrap prop.

[nwhittaker]

@driskull, I'm having trouble with the solution because I'd like to continue trapping focus within the modal. Temporarily disabling the focus trap shifts focus back to the modal's close button when I re-enable the trap: https://codepen.io/nwhittaker-esri/pen/QWBwNxy. Would that be considered a bug?

What is the recommend pattern for using the new disableFocusTrap property to support the reported use-case of setting focus on a new element in the modal? Am I expected to keep it disabled and implement my own focus trap logic instead?

FWIW, I was thinking you'd just call updateContainerElements() on the focus trap whenever the content slot's subtree was mutated which should rebuild the list of tabbable nodes. Any problems with this type of approach?

[driskull]

FWIW, I was thinking you'd just call updateContainerElements() on the focus trap whenever the content slot's subtree was mutated which should rebuild the list of tabbable nodes. Any problems with this type of approach

Yes, I tried this but it didn't seem to work. I think the focus-trap module has a bug with custom elements. I need to create a repro and open a bug for them.

[driskull]

Hey Adelheid, for the focusing issue, have you tried using requestAnimationFrame instead of some timeout with a number?

Like:

requestAnimationFrame(() => el.setFocus());

Example: https://codepen.io/benesri/pen/MWBgJBR

[driskull]

@nwhittaker created issue here: #6140