phetsims / circuit-construction-kit-common

"Circuit Construction Kit: Basics" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
10 stars 10 forks source link

Loading premade circuits into a wrapper sometimes results in an error #971

Closed Nancy-Salpepi closed 1 year ago

Nancy-Salpepi commented 1 year ago

Test device MacBook Air M1 chip

Operating System 13.1

Browser Safari and Chrome

Problem description For https://github.com/phetsims/qa/issues/900, following the directions in the Examples doc for loading a premade circuit into a saved Standard PhET-iO Wrapper results in an error on my first attempt. I am usually successful after reloading the wrapper and pasting again.

Steps to reproduce I followed the steps in the Examples doc:

  1. On the Intro screen, make a circuit
  2. Go to circuitConstructionKitDc.introScreen.model.circuit and press 'Get Value'
  3. Then press 'Copy Set Value Code' --code is copied to clipboard
  4. Open a saved Standard PhET-iO wrapper
  5. In the console paste the code and press enter --I usually get an error here
  6. Refresh the page
  7. Paste again --it works

Visuals This is the error I get:

Screenshot 2023-02-17 at 4 01 59 PM
samreid commented 1 year ago

In Chrome Dev Tools, you can select whether you want to run a command in the wrapper "top" or sim "iframe", you can select between them via the combobox:

image

phetioClient only exists in top and if sim-frame was selected, you may receive a phetioClient is not defined error. @Nancy-Salpepi could this explain the behavior you saw?

Nancy-Salpepi commented 1 year ago

@samreid when I change to top it works correctly. Thanks! I still have 2 questions:

  1. How do I make that change in Safari?
  2. @arouinfar Would instructional designers know what to do?
samreid commented 1 year ago

In safari, that control is in the bottom right of the console.

Develop => Show JavaScript Console

image

Then in the bottom right

image
Nancy-Salpepi commented 1 year ago

Thank you @samreid! It works in Safari as well.

matthew-blackman commented 1 year ago

@samreid, @zepumph and I feel that using an invoke() command to set state is a function that will be geared towards 3rd-party developers, so it is realistic to expect them to navigate to the correct element before doing this. We do not feel that this needs to be documented for PhET-iO clients.

@Nancy-Salpepi will you please document this in the QA documentation? After that, this issue can be closed.

Nancy-Salpepi commented 1 year ago

@matthew-blackman is this something that is specific to the examples for CCK? We don't usually add things to the QA book that are sim-specific. If so, I can just add this to my own notes.

matthew-blackman commented 1 year ago

I believe this will come up in any PhET-iO sim, any time a client tries to use an invoke() command in the console. @samreid or @zepumph can you confirm that this is the case?

matthew-blackman commented 1 year ago

@samreid has confirmed that this is the case for all PhET-iO sims with 'Copy Set Value' code.

Nancy-Salpepi commented 1 year ago

ok thanks. I will add something now.

arouinfar commented 1 year ago

@arouinfar Would instructional designers know what to do?

In this case, we expect instructional designers to create circuits for later use in a wrapper. The examples doc states:

Verify the circuit is correctly restored by testing it in Standard PhET-iO Wrapper by pasting the code into the browser console.

After speaking with @Nancy-Salpepi and @stemilymill, it's clear that there are potential stumbling blocks when it comes to using the console. Reopening to add some clarification on selecting the correct JavaScript context.

Reopening for https://github.com/phetsims/qa/issues/908

arouinfar commented 1 year ago

I'm not sure how to word this. Chrome and Safari call it "top". Safari calls it "Studio" in Studio and a preview of a Standard PhET-iO Wrapper and it uses "filename" when opening up a downloaded Standard PhET-iO Wrapper. We need a more general term, so I was thinking "topmost layer" but I honestly don't know. My goal is for non-developers to know what to google if they run into trouble.

Here's my proposal of what to add to the end of the Load premade circuits into a wrapper example:

Verify the circuit is correctly restored by testing it in Standard PhET-iO Wrapper by pasting the code into the browser console. Note that the JavaScript context must be set to the topmost layer.

@samreid or @matthew-blackman can you review and advise?

matthew-blackman commented 1 year ago

@arouinfar @samreid and I discussed this, and agree that this can be solved with additional documentation in the Examples.md file and possibly the PhET-iO Guide. We recommend adding some instructions about how to ensure the correct browser context, similar to the following:

"Make sure that the command is being applied to the top-level outermost element. Most browsers show a combobox that lets you choose which context the command will apply to. If you select/inspect an iframe, the command will apply to that iframe. Apply the commands to the outer element, which will be called 'top' or '{FILENAME}', rather than the inner one, which will be called 'sim' or 'sim-frame'."

arouinfar commented 1 year ago

I updated the guides for CCK: DC and CCK: DC - VL in the above commits using the language from https://github.com/phetsims/circuit-construction-kit-common/issues/971#issuecomment-1456527611.

https://github.com/phetsims/phet-io-sim-specific/commit/efbfbea32113677dfc6a28ee67d0134ed030fd3e should be cherry-picked into CCK: DC. (Virtual Lab hasn't yet been branched.)

samreid commented 1 year ago

For the QA Team, to verify this in RC.3, please test that the "examples" document has wording like: "You may need to make sure that the command is being applied to the outermost element." etc.

Nancy-Salpepi commented 1 year ago

Wording in Examples doc has been updated. Looks good in rc.3. Closing.