theKashey / focus-lock

Gotcha! A11y util for scoping a focus.
MIT License
172 stars 17 forks source link

Not able to enter input field in shadow dom #48

Closed ChibiOokami86 closed 6 months ago

ChibiOokami86 commented 1 year ago

Description: When a user attempts to click into an element on the shadow dom (input or button), focus is being set to the first selectable element outside of the shadow dom. A user is not able to tab into the shadow dom.

Investigation: Looking at src/utils/DOMutils.ts => contains function.

In v0.11.2 I see the following code which can be read as "return (A or B) || backup". A or B must resolve to truthy or the backup is utilized.

https://github.com/theKashey/focus-lock/blob/v0.11.2/src/utils/DOMutils.ts#L48-L55

In v0.11.3+, it has been changed to the following which can be read as "return A or (B || backup)". Here, A no longer has to resolve to truthy.

https://github.com/theKashey/focus-lock/blob/v0.11.3/src/utils/DOMutils.ts#L48-L61


Summary: This is causing issues when trying to click into an input field on the shadow dom.


Local Fix:

I do not have enough understanding to the specifics but this solved our issue. I see that in v0.11.6 the toArray portion has been changed further. I am hoping you might be able to deduce what was this solves in v0.11.3 and take that learning into v0.11.6

export const contains = (scope: Element | ShadowRoot, element: Element): boolean => {
  if ((scope as HTMLElement).shadowRoot) {
    return contains((scope as HTMLElement).shadowRoot as ShadowRoot, element) ||
       // FIX: Add backup option if the line above is not truthy
        toArray(scope.children).some((child) => contains(child, element));
  } else {
    if (
      Object.getPrototypeOf(scope).contains !== undefined &&
      Object.getPrototypeOf(scope).contains.call(scope, element)
    ) {
      return true;
    }

    return toArray(scope.children).some((child) => contains(child, element));
  }
};
theKashey commented 1 year ago

the code above is totally equal to the one before as contains will use toArray(scope.children)... as a fallback.

Let me create a test for this case to better understand the situation and prevent future regressions.

stale[bot] commented 1 year ago

This issue has been marked as "stale" because there has been no activity for 2 months. If you have any new information or would like to continue the discussion, please feel free to do so. If this issue got buried among other tasks, maybe this message will reignite the conversation. Otherwise, this issue will be closed in 7 days. Thank you for your contributions so far.

stale[bot] commented 1 year ago

This issue has been marked as "stale" because there has been no activity for 2 months. If you have any new information or would like to continue the discussion, please feel free to do so. If this issue got buried among other tasks, maybe this message will reignite the conversation. Otherwise, this issue will be closed in 7 days. Thank you for your contributions so far.

stale[bot] commented 1 year ago

This issue has been marked as "stale" because there has been no activity for 2 months. If you have any new information or would like to continue the discussion, please feel free to do so. If this issue got buried among other tasks, maybe this message will reignite the conversation. Otherwise, this issue will be closed in 7 days. Thank you for your contributions so far.

stale[bot] commented 11 months ago

This issue has been marked as "stale" because there has been no activity for 2 months. If you have any new information or would like to continue the discussion, please feel free to do so. If this issue got buried among other tasks, maybe this message will reignite the conversation. Otherwise, this issue will be closed in 7 days. Thank you for your contributions so far.

stale[bot] commented 9 months ago

This issue has been marked as "stale" because there has been no activity for 2 months. If you have any new information or would like to continue the discussion, please feel free to do so. If this issue got buried among other tasks, maybe this message will reignite the conversation. Otherwise, this issue will be closed in 7 days. Thank you for your contributions so far.

stale[bot] commented 7 months ago

This issue has been marked as "stale" because there has been no activity for 2 months. If you have any new information or would like to continue the discussion, please feel free to do so. If this issue got buried among other tasks, maybe this message will reignite the conversation. Otherwise, this issue will be closed in 7 days. Thank you for your contributions so far.