theKashey / react-focus-lock

It is a trap! A lock for a Focus. 🔓
MIT License
1.27k stars 67 forks source link

Autofocus keeps firing inside shadow dom #188

Closed niairobi closed 1 year ago

niairobi commented 2 years ago

A problem occurs when a focus lock component with autoFocus is rendered inside a shadow root. Component successfully focuses first interactive element, but then repeatedly resets focus to it, when a user tries to focus anything else.

Here is the code of the demo reproducing the issue:

import "./styles.css";
import { StrictMode } from "react";
import ReactDOM from "react-dom";
import ReactFocusLock from "react-focus-lock";

export default function App() {
  return (
    <ReactFocusLock autoFocus={true}>
      <div className="App">
        <h1>Hello CodeSandbox</h1>
        <input></input>
        <input></input>
        <h2>Start editing to see some magic happen!</h2>
      </div>
    </ReactFocusLock>
  );
}
const template = document.createElement("template");
template.innerHTML = `

<div>
  <p part="title">React attached below</p>
  <div id="root"></div>
</div>
`;

export class WebComp extends HTMLElement {
  constructor() {
    super();
    // attach to the Shadow DOM
    const root = this.attachShadow({ mode: "closed" });
    root.appendChild(template.content.cloneNode(true));
    ReactDOM.render(
      <StrictMode>
        <App />
      </StrictMode>,
      root
    );
  }
}

window.customElements.define("web-comp", WebComp);
theKashey commented 2 years ago

well, I know what's happening and even why.

The only real problem - in the past there were some complications around creating a comprehensive unit test for shadow dom.

n1313 commented 2 years ago

@theKashey this issue affects me as well, and kinda in a big way. Is there anything I can do to help you here?

theKashey commented 2 years ago

The best one can do - create a failing test. Then it will be easier to "understand" the problem and "know" when it's fixed.

pavelsherm commented 2 years ago

@theKashey It's not related to autofocus attribute. Focus-lock is not working with shadow-dom, because document.activeElement returns root element of shadow dom. In order to get actual active element you need to check if an element have shadowRoot attribute: document.activeElement.shadowRoot ? document.activeElement.shadowRoot.activeElement : document.activeElement

theKashey commented 2 years ago

Thanks @pavelsherm - now I am one step closer to understanding the problem (as I am not any shadow-dom/web-components expert or even user)

Jaodi commented 2 years ago

Here is a test that reliably fails on the current version. If you remove the use of FocusLock component, the test passes.

I had to update the version of jsdom this project is using, because currently installed version of the library does not support shadow DOM yet

diff --git a/_tests/FocusLock.spec.js b/_tests/FocusLock.spec.js
index 0b07d7e..19f8928 100644
--- a/_tests/FocusLock.spec.js
+++ b/_tests/FocusLock.spec.js
@@ -905,6 +905,54 @@ text
       });
     });

+    describe('child creates a shadow tree', () => {
+      it('does not stop focus from moving inside the shadow DOM', () => {
+        function App() {
+          return (
+            <FocusLock>
+              <div className="App">
+                <h1>Hello CodeSandbox</h1>
+                <input id="first-input" />
+                <input id="second-input" />
+                <h2>Start editing to see some magic happen!</h2>
+              </div>
+            </FocusLock>
+          );
+        }
+        const template = document.createElement('template');
+        template.innerHTML = `
+          <div>
+            <p part="title">React attached below</p>
+            <div id="root"></div>
+          </div>
+        `;
+        class WebComp extends HTMLElement {
+          constructor() {
+            super();
+            // attach to the Shadow DOM
+            const root = this.attachShadow({ mode: 'closed' });
+            root.appendChild(template.content.cloneNode(true));
+            this.ref = {
+              focused: () => root.activeElement,
+              focusSecond: () => root.querySelector('#second-input').focus(),
+            };
+            ReactDOM.render(
+              <App />,
+              root,
+            );
+          }
+        }
+        window.customElements.define('web-comp', WebComp);
+        const webComp = document.createElement('web-comp');
+        document.body.appendChild(webComp);
+
+        webComp.focus();
+        const { focused, focusSecond } = webComp.ref;
+        focusSecond();
+        expect(focused().id).to.be.equal('second-input');
+      });
+    });
+
     describe('groups', () => {
       it('false test', (done) => {
         const wrapper = mount(<div>
diff --git a/package.json b/package.json
index c4adba0..d85297e 100644
--- a/package.json
+++ b/package.json
@@ -79,7 +79,7 @@
     "eslint-plugin-jsx-a11y": "^6.2.1",
     "eslint-plugin-mocha": "^5.3.0",
     "eslint-plugin-react": "^7.13.0",
-    "jsdom": "15.1.1",
+    "jsdom": "^19.0.0",
     "jsdom-global": "^3.0.2",
     "material-ui": "^0.20.0",
     "mocha": "^8.3.2",
theKashey commented 2 years ago

The test provided by @Jaodi has been added as a base specification for shadow-dom and react-focus-lock is currently passing it with the updated focus-lock functionality provided by @Shermayster

Necessary updates were released as a part of v 2.8.1

I cannot verify the correctness of this implementation as I don't have a real usecase. Please try how the new version matching your expectations.

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.