Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keyboard commands don't work in iframes #1027

Open
KatieWoe opened this issue Mar 10, 2025 · 2 comments
Open

Keyboard commands don't work in iframes #1027

KatieWoe opened this issue Mar 10, 2025 · 2 comments

Comments

@KatieWoe
Copy link
Contributor

Device
Lenovo Laptop
OS
Win 11
Browser
Chrome
Problem Description
For phetsims/qa#1224. Discovered and discussed in phetsims/circuit-construction-kit-dc#209
When the sim is in an iframe the delete/backspace button to delete an element doesn't work. @Nancy-Salpepi noted that alt+r to reset all also doesn't work in this case.

@zepumph
Copy link
Member

zepumph commented Mar 11, 2025

@samreid @jonathanolson and I discussed this issue, and we feel like the best course of action is having support for a focusable element inside a sim when there are no other focusable elements (no PDOM or interactive description). This solution attempts to apply a general solution, but in a way where we only turn it on in CCK right now. Basically there should be a focusable content in sims without keyboard interaction. Maybe it can say something like "this is an interactive sim...". It doesn't matter too much at this time, but it could be useful in other cases too (so tabbing a webpage doesn't just skip the sim, treating it like a black box).

Subject: [PATCH] update dependencies for CODAP wrapper, https://github.com/phetsims/phet-io-sim-specific/issues/50
---
Index: circuit-construction-kit-dc/js/circuit-construction-kit-dc-main.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/circuit-construction-kit-dc/js/circuit-construction-kit-dc-main.ts b/circuit-construction-kit-dc/js/circuit-construction-kit-dc-main.ts
--- a/circuit-construction-kit-dc/js/circuit-construction-kit-dc-main.ts	(revision d1c4df2ec984f36b3f137f5a9714c78b52114ea7)
+++ b/circuit-construction-kit-dc/js/circuit-construction-kit-dc-main.ts	(date 1741719187043)
@@ -11,6 +11,7 @@
 import PreferencesModel from '../../joist/js/preferences/PreferencesModel.js';
 import Sim from '../../joist/js/Sim.js';
 import simLauncher from '../../joist/js/simLauncher.js';
+import affirm from '../../perennial-alias/js/browser-and-node/affirm.js';
 import soundManager from '../../tambo/js/soundManager.js';
 import Tandem from '../../tandem/js/Tandem.js';
 import CircuitConstructionKitDcStrings from './CircuitConstructionKitDcStrings.js';
@@ -53,6 +54,16 @@
     phetioDesigned: true
   } );
   sim.start();
+  // TODO: Make a CCKCSim() that factors this out? https://github.com/phetsims/circuit-construction-kit-common/issues/1027
+  const element = sim.getNonAccessiblePlaceholder();
+  affirm( element );
+  sim.display.addInputListener( {
+    down: () => {
+      if ( document.activeElement === document.body ) {
+        element.focus();
+      }
+    }
+  } );
 
   // Disable sounds for joist/home screen/navigation bar/carousel, but leave sound for the dog bark
   soundManager.setOutputLevelForCategory( 'user-interface', 0 );
Index: joist/js/SimDisplay.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/joist/js/SimDisplay.ts b/joist/js/SimDisplay.ts
--- a/joist/js/SimDisplay.ts	(revision bba59bfab24b4fdae859bd4a99efa0b4e7b3fc91)
+++ b/joist/js/SimDisplay.ts	(date 1741715443275)
@@ -121,6 +121,20 @@
 
     super( new Node( { renderer: options.rootRenderer } ), options );
 
+    // add supportsInteractiveDescription
+    // add cck specific code
+    // do the below
+    // do the below, but only in CCK
+
+    // if ( !options.accessibility ) {
+    //   const  x = createFocusableDummy();
+    //   this.addInputListener( {
+    //     down: () => {
+// x.focus();
+//         }
+//       } );
+//     }
+
     this.simulationRoot = new Node();
     this.rootNode.addChild( this.simulationRoot );
 
Index: joist/js/Sim.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/joist/js/Sim.ts b/joist/js/Sim.ts
--- a/joist/js/Sim.ts	(revision bba59bfab24b4fdae859bd4a99efa0b4e7b3fc91)
+++ b/joist/js/Sim.ts	(date 1741716415640)
@@ -1176,6 +1176,18 @@
     }
     return info;
   }
+
+  public getNonAccessiblePlaceholder(): HTMLElement | null {
+    if ( !this.display.isAccessible() ) {
+      const tempFocusable = document.createElement( 'div' );
+      tempFocusable.tabIndex = 0;
+      tempFocusable.innerText = `${this.displayedSimNameProperty.value} is an interactive sim. It changes as you play with it.`;
+      document.body.appendChild( tempFocusable );
+      return tempFocusable;
+    }
+
+    return null;
+  }
 }
 
 type LayoutNode = Node & {

zepumph added a commit to phetsims/circuit-construction-kit-dc-virtual-lab that referenced this issue Mar 11, 2025
zepumph added a commit to phetsims/circuit-construction-kit-ac-virtual-lab that referenced this issue Mar 11, 2025
zepumph added a commit to phetsims/circuit-construction-kit-black-box-study that referenced this issue Mar 11, 2025
@zepumph
Copy link
Member

zepumph commented Mar 11, 2025

Ok. I got to a commit point here. I created a CCKCSim to house this new CCK-specific code, and a general function in SimDisplay. @samreid can you please review?

@zepumph zepumph removed their assignment Mar 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants