[Bug]: Dialog component in combination with http-fetch in a watcher breaks autofocus about radix-vue HOT 5 OPEN

Hello!

Thank you for looking into it!
I've seen the fix.

I've spend so much time today again on it and also saw some strange behavior when removing nextTick() completely. I often - not always - got an endless update loop.

I tried three other solutions today

1. Moving
   const previouslyFocusedElement = document.activeElement as HTMLElement | null
   between

  const container = currentElement.value
  //--> here
  await nextTick()

That way we can remember the activeElemement, which gets changed by the MutationObserver. But this might break something else, when during nextTick the focus gets changed.

2. Checking, if the container equals document.activeElement
   const hasFocusedCandidate = container.contains(previouslyFocusedElement)
   If yes, set hasFocusedCanditate to false
   const hasFocusedCandidate = container.contains(previouslyFocusedElement) && document.activeElement !== container

That way we would rely on the implementation of the handleMutation, that it changes the focus to container on failure. Don't like it.

3. Using a flag to disable the handleMutations.

initEffectsActive += 1
await nextTick().finally(() => initEffectsActive -= 1);

  function handleMutations(mutations: MutationRecord[]) {
    if(initEffectsAtive > 0)
      return;
    const isLastFocusedElementExist = container.contains(lastFocusedElementRef.value)
    if (!isLastFocusedElementExist)
      focus(container)
  }

This would be actually my preferred way, as it is clear, that we don't want the MutationObserver to fix the focus, while we haven't init ourself during the watchEffect. As the watchEffect actually has the task to set the focus initially.

And if some user-code sets the focus during nextTick somewhere else the init routine wouldn't mess with it, as hasFocusedCandidate would be true as a result.

It is a counter, as the async function gets called twice before the first is actually finished.

from radix-vue.

Sorry for spamming, I've optimized the 3rd option.
I've never done a pull request before. But I could try tomorrow. It is already late here.

I wrote this code here blind without IDE, might have still typos.

function useAsyncExecuting<T>(promise: () => Promise<T>)
{
  let counter = 0;

  async function execute()
  {
    try
    {
	counter += 1;
        return await promise();
    }
    finally
    {
        counter -= 1;
    }
  }

  function isExecuting()
  {
     return counter > 0;
  }

  return {
    execute,
    isExecuting
  }
}


const { execute: executeNextTick, isExecuting: isExecutingNextTick }  = useAsyncExecuting(nextTick);

  function handleMutations(mutations: MutationRecord[]) {
    // Do not handle mutations for the duration of nextTick during the initialization
    // The initialization will take care of finding and focusing the first element
    if(isExecutingNextTick())
      return;
    const isLastFocusedElementExist = container.contains(lastFocusedElementRef.value)
    if (!isLastFocusedElementExist)
      focus(container)
  }

During the async watchEffect:

await executeNextTick();

from radix-vue.

Thanks for digging into this @heladrion ! Instead of using a workaround for this.. I might need to might the root cause of why the fetch is causing the nextTick to behave differently, causing the previouslyFocusedElement to be different.

FIY: I've tried your suggest solution and it's breaking tests. So it might not be feasible.

from radix-vue.

After testing for quite a bit, I believe your proposed solution #1 previouslyFocusedElement should fix the issue. As it make sense to capture the previously focused element before we have the nextTick.

from radix-vue.

I digged today into Vue itself with the SFC Playground, but couldn't find any differences between the calls. But it is super hard to debug.

Yes #1 previouslyFocusedElement should do the trick. But the handleMutation is still happening and the handleMutation temporarily moves the focus to the body. Before the actual autofocus-onMount call. I'm scared, that this might just hide the problem, but I guess every solution has the problem in the end :/

I fixed version #3
https://github.com/heladrion/radix-vue/commits/fix(FocusScope)-autoFocus-not-working%2C-when-fetch-called-during-a-watcher/
All tests are ok, but I also had to change AlertDialogContent.vue

    @open-auto-focus="
      () => {
        nextTick(() => {
          cancelElement?.focus({
            preventScroll: true,
          });
        });
      }
    "

to the following

    @open-auto-focus.prevent="(event) => {
      cancelElement?.focus({
        preventScroll: true,
      })
    }"

which is actually more in line with radix-ui
https://github.com/radix-ui/primitives/blob/c31c97274ff357aea99afe6c01c1c8c58b6356e0/packages/react/alert-dialog/src/AlertDialog.tsx#L131

If someone wants to debug it deeper on Chrome, here is the Vue SFC Playground URL for localhost on port 5173. The files are encoded in the URL, so it should work.
localhost:5173 Playground

from radix-vue.