CODAP wrapper: Layout issues

samreid commented 5 months ago

Namely, make sure no panels appear behind the simulation.

samreid commented 5 months ago

We are seeing that font-family: 'Montserrat-Regular', sans-serif !important; looks good in the CODAP table. @matthew-blackman changed to use that font in the checkbox panel, and it looks good on @matthew-blackman's screen. But on my screen, I see the correct font in the table but the checkbox labels fall back to Arial.

samreid commented 5 months ago

Documentation for the di query parameter: https://codap-server.concord.org/help/work-documents-create-open-save-and-share-documents/use-url-parameters-to-modify-codaps-behavior

Documentation for the API: https://github.com/concord-consortium/codap/wiki/CODAP-Data-Interactive-Plugin-API#example-update-interactive-frame

samreid commented 5 months ago

I'm getting NaN when it tries to position the webview:

samreid commented 5 months ago

I'm experimenting with a wrapper container that has the sim on the left and the selected attributes to the right:

```diff Subject: [PATCH] Simplify the background gradient, and make it non-scaled, see https://github.com/phetsims/projectile-data-lab/issues/178 --- Index: repos/projectile-data-lab/wrappers/codap/codap.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/repos/projectile-data-lab/wrappers/codap/codap.js b/repos/projectile-data-lab/wrappers/codap/codap.js --- a/repos/projectile-data-lab/wrappers/codap/codap.js (revision c32aff49b9b3d76770c3773d625943c160ee007c) +++ b/repos/projectile-data-lab/wrappers/codap/codap.js (date 1709999052912) @@ -139,7 +139,7 @@ fieldIdentifier.substring( 0, fieldIdentifier.length - 'field'.length ); }; -const initProjectileData = () => { +const initProjectileData = async () => { fieldPhetioIDs.forEach( fieldPhetioID => { phetioClient.invoke( `${fieldPhetioID}.projectileLandedEmitter`, 'addListener', [ async projectile => { @@ -184,13 +184,13 @@ codapDataObject.caseIndex = result.values + 1; // Adding 1 to make it match the new row index // Now, create the data item in CODAP with the updated codapDataObject - codapPhone.call( { + + const createResult = await codapPhoneAsync( { action: 'create', resource: 'dataContext[projectile-data-lab].item', values: [ codapDataObject ] - }, createResult => { - elements.push( { projectile: codapDataObject, result: createResult } ); - } ); + } ); + elements.push( { projectile: codapDataObject, result: createResult } ); } else { console.error( 'Failed to get caseCount from CODAP' ); @@ -200,7 +200,8 @@ const embeddedSimWidth = 0.5 * window.outerWidth; const embeddedSimHeight = 0.64 * embeddedSimWidth; - codapPhone.call( [ { + + const response = await codapPhoneAsync( [ { action: 'get', resource: 'componentList' }, { action: 'update', resource: 'interactiveFrame', values: { @@ -216,85 +217,81 @@ }, attrs: initialProjectileDataTableAttributes } ] } - } ], response => { - const codapComponents = response ? response[ 0 ].values : []; - assert && assert( Array.isArray( codapComponents ), 'Make sure that the first response element contains the component list.' ); - const codapComponentNames = codapComponents.map( component => component.name ); - const TABLE_NAME = 'projectileCaseTable'; - const GRAPH_NAME = 'projectileGraph'; - const codapArgs = []; + } ] ); + + const codapComponents = response ? response[ 0 ].values : []; + assert && assert( Array.isArray( codapComponents ), 'Make sure that the first response element contains the component list.' ); + const codapComponentNames = codapComponents.map( component => component.name ); + const TABLE_NAME = 'projectileCaseTable'; + const GRAPH_NAME = 'projectileGraph'; + const codapArgs = []; - if ( !codapComponentNames.includes( TABLE_NAME ) ) { - codapArgs.push( { - action: 'create', resource: 'component', values: { - type: 'caseTable', name: TABLE_NAME, title: 'Projectile Data', dataContext: 'projectile-data-lab' - } - } ); - } - if ( !codapComponentNames.includes( 'projectileDataWebView' ) ) { + if ( !codapComponentNames.includes( TABLE_NAME ) ) { + codapArgs.push( { + action: 'create', resource: 'component', values: { + type: 'caseTable', name: TABLE_NAME, title: 'Projectile Data', dataContext: 'projectile-data-lab' + } + } ); + } + if ( !codapComponentNames.includes( 'projectileDataWebView' ) ) { - // console.log( window.location.href ); - const href = window.location.href; + // console.log( window.location.href ); + const href = window.location.href; - // unbuilt is like - // https://127.0.0.1:8123/phet-io-sim-specific/repos/projectile-data-lab/wrappers/codap/?sim=projectile-data-lab&lang=en + // unbuilt is like + // https://127.0.0.1:8123/phet-io-sim-specific/repos/projectile-data-lab/wrappers/codap/?sim=projectile-data-lab&lang=en - // built is like: - // ORIGIN: {build}/phet-io/wrappers/codap/index.html + // built is like: + // ORIGIN: {build}/phet-io/wrappers/codap/index.html - // chop off the string after /wrappers/codap - const prefix = href.split( 'wrappers/codap' )[ 0 ]; + // chop off the string after /wrappers/codap + const prefix = href.split( 'wrappers/codap' )[ 0 ]; - const query = []; + const query = []; - const hasAnyDataQueryStrings = - queryStrings.launchSpeed || - queryStrings.launchAngle || - queryStrings.horizontalDistance || - queryStrings.timeAirborne || - queryStrings.launcherConfiguration || - queryStrings.projectileType || - queryStrings.launcherMechanism || - queryStrings.angleStabilizerWidth; + const hasAnyDataQueryStrings = + queryStrings.launchSpeed || + queryStrings.launchAngle || + queryStrings.horizontalDistance || + queryStrings.timeAirborne || + queryStrings.launcherConfiguration || + queryStrings.projectileType || + queryStrings.launcherMechanism || + queryStrings.angleStabilizerWidth; - // If none of the data query strings are set, add all possible quantities to the list - ( !hasAnyDataQueryStrings || queryStrings.launchSpeed ) && query.push( 'launchSpeed' ); - ( !hasAnyDataQueryStrings || queryStrings.launchAngle ) && query.push( 'launchAngle' ); - ( !hasAnyDataQueryStrings || queryStrings.horizontalDistance ) && query.push( 'horizontalDistance' ); - ( !hasAnyDataQueryStrings || queryStrings.timeAirborne ) && query.push( 'timeAirborne' ); - ( !hasAnyDataQueryStrings || queryStrings.launcherConfiguration ) && query.push( 'launcherConfiguration' ); - ( !hasAnyDataQueryStrings || queryStrings.projectileType ) && query.push( 'projectileType' ); - ( !hasAnyDataQueryStrings || queryStrings.launcherMechanism ) && query.push( 'launcherMechanism' ); - ( !hasAnyDataQueryStrings || queryStrings.angleStabilizerWidth ) && query.push( 'angleStabilizerWidth' ); + // If none of the data query strings are set, add all possible quantities to the list + ( !hasAnyDataQueryStrings || queryStrings.launchSpeed ) && query.push( 'launchSpeed' ); + ( !hasAnyDataQueryStrings || queryStrings.launchAngle ) && query.push( 'launchAngle' ); + ( !hasAnyDataQueryStrings || queryStrings.horizontalDistance ) && query.push( 'horizontalDistance' ); + ( !hasAnyDataQueryStrings || queryStrings.timeAirborne ) && query.push( 'timeAirborne' ); + ( !hasAnyDataQueryStrings || queryStrings.launcherConfiguration ) && query.push( 'launcherConfiguration' ); + ( !hasAnyDataQueryStrings || queryStrings.projectileType ) && query.push( 'projectileType' ); + ( !hasAnyDataQueryStrings || queryStrings.launcherMechanism ) && query.push( 'launcherMechanism' ); + ( !hasAnyDataQueryStrings || queryStrings.angleStabilizerWidth ) && query.push( 'angleStabilizerWidth' ); - let projectileDataPanelURL = prefix + 'wrappers/codap/projectileDataPanel.html'; - if ( query.length > 0 ) { - projectileDataPanelURL += '?' + query.join( '&' ); - } + let projectileDataPanelURL = prefix + 'wrappers/codap/projectileDataPanel.html'; + if ( query.length > 0 ) { + projectileDataPanelURL += '?' + query.join( '&' ); + } - codapArgs.push( { - action: 'create', resource: 'component', values: { - type: 'webView', name: 'projectileDataWebView', title: 'Data to Collect', cannotClose: true, URL: projectileDataPanelURL, dimensions: { - width: 250, height: 300 - } - } - } ); - } + // codapArgs.push( { + // action: 'create', resource: 'component', values: { + // type: 'webView', name: 'projectileDataWebView', title: 'Data to Collect', cannotClose: true, URL: projectileDataPanelURL, dimensions: { + // width: 250, height: 300 + // } + // } + // } ); + } - if ( !codapComponentNames.includes( GRAPH_NAME ) ) { - codapArgs.push( { - action: 'create', resource: 'component', values: { - type: 'graph', name: GRAPH_NAME, title: 'Projectile Graph', dataContext: 'projectile-data-lab', xAttributeName: '', yAttributeName: '' - } - } ); - } + if ( !codapComponentNames.includes( GRAPH_NAME ) ) { + codapArgs.push( { + action: 'create', resource: 'component', values: { + type: 'graph', name: GRAPH_NAME, title: 'Projectile Graph', dataContext: 'projectile-data-lab', xAttributeName: '', yAttributeName: '' + } + } ); + } - // this is to fix a bug with CODAP component layout - // we skip one frame to render after CODAP has correctly sized our dataInteractive - window.setTimeout( () => codapPhone.call( codapArgs, () => { // eslint-disable-line bad-sim-text - console.log( 'codap callback' ); - } ), 100 ); - } ); + await codapPhoneAsync( codapArgs ); window.addEventListener( 'storage', async event => { if ( event.key === 'phetioLocalStorageChannel' ) { @@ -307,54 +304,56 @@ projectileDataSelectedColumns[ column ] = true; // see if we already have the column - codapPhone.call( { + const result = await codapPhoneAsync( { action: 'get', resource: 'dataContext[projectile-data-lab].collection[projectile-data].attributeList' - }, result => { - if ( result.success ) { - const values = result.values; + } ); + + if ( result.success ) { + const values = result.values; - console.log( 'trying to add column: ' + column + ' to ' + values ); - if ( !values.map( value => value.name ).includes( column ) ) { + console.log( 'trying to add column: ' + column + ' to ' + values ); + if ( !values.map( value => value.name ).includes( column ) ) { - // Look up the corresponding data in projectileDataTableAttrs. - const attr = SELECTABLE_PROJECTILE_DATA_TABLE_ATTRIBUTES.find( attr => attr.name === column ); + // Look up the corresponding data in projectileDataTableAttrs. + const attr = SELECTABLE_PROJECTILE_DATA_TABLE_ATTRIBUTES.find( attr => attr.name === column ); - codapPhone.call( { - action: 'create', resource: 'dataContext[projectile-data-lab].collection[projectile-data].attribute', values: [ { - name: attr.name, description: attr.description, type: attr.type, unit: attr.unit - } ] - } ); - const codapArgs = []; + await codapPhoneAsync( { + action: 'create', resource: 'dataContext[projectile-data-lab].collection[projectile-data].attribute', values: [ { + name: attr.name, description: attr.description, type: attr.type, unit: attr.unit + } ] + } ); + const codapArgs = []; - for ( let i = 0; i < elements.length; i++ ) { - const element = elements[ i ]; - codapArgs.push( { - action: 'update', resource: `dataContext[projectile-data-lab].itemByID[${element.result.itemIDs[ 0 ]}]`, values: { - [ column ]: `${element.projectile[ column ]}` - } - } ); - } - codapPhone.call( codapArgs, result => { - console.log( 'done', result ); - } ); - } - } - else { - throw new Error( 'failed to get attributes' ); - } - } ); + for ( let i = 0; i < elements.length; i++ ) { + const element = elements[ i ]; + codapArgs.push( { + action: 'update', resource: `dataContext[projectile-data-lab].itemByID[${element.result.itemIDs[ 0 ]}]`, values: { + [ column ]: `${element.projectile[ column ]}` + } + } ); + } + + const res = await codapPhoneAsync( codapArgs ); + console.log( 'done', res ); + } + } + else { + throw new Error( 'failed to get attributes' ); + } + + } else if ( data.command === 'removeColumn' ) { const column = data.column; projectileDataSelectedColumns[ column ] = false; - codapPhone.call( { + const results = await codapPhoneAsync( { action: 'delete', resource: `dataContext[projectile-data-lab].collection[projectile-data].attribute[${column}]` - }, results => { - console.log( 'delete done' ); - console.log( results ); - } ); + } ); + + console.log( 'delete done' ); + console.log( results ); } } } ); @@ -409,12 +408,6 @@ action: 'create', resource: 'component', values: { type: 'graph', name: 'toolDataGraph', title: 'Tool Data Graph', dataContext: 'tool-data-context', xAttributeName: '', yAttributeName: '' } - }, { - action: 'create', resource: 'component', values: { - type: 'webView', name: 'toolDataWebView', title: 'Tool Data', cannotClose: true, URL: toolDataPanelURL, dimensions: { - width: 200, height: 100 - } - } } ] ); window.addEventListener( 'storage', async event => { @@ -478,16 +471,15 @@ console.log( toolReadings ); // Add that data to the table - codapPhone.call( { + await codapPhoneAsync( { action: 'create', resource: 'dataContext[tool-data-context].item', values: [ toolReadings ] - }, result => { - console.log( 'added tool data' ); - } ); + } ); + + console.log( 'added tool data' ); } } } ); }; - /** * Index: repos/projectile-data-lab/wrappers/codap/projectileDataPanel.html IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/repos/projectile-data-lab/wrappers/codap/projectileDataPanel.html b/repos/projectile-data-lab/wrappers/codap/projectileDataPanel.html --- a/repos/projectile-data-lab/wrappers/codap/projectileDataPanel.html (revision c32aff49b9b3d76770c3773d625943c160ee007c) +++ b/repos/projectile-data-lab/wrappers/codap/projectileDataPanel.html (date 1709998602350) @@ -10,8 +10,6 @@ --> - - Projectile Data + + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file ```

However, this does not let the user collapse or hide the selected attributes (as a separate window). I don't know if this is an acceptable design. It also traps a lot of empty space below the checkboxes.

catherinecarter commented 5 months ago

Pros to keeping the panel docked

Users can stack panels on top of each other, so the 'trapped' space could easily be covered by a graph, table, or whatever panel is active.
I like how the panel is very clearly connected to the sim, meaning the variables are now very obviously coming from the data in the sim.

Cons to docking the panel

You bring up good points about trapping a lot of space in an already-crowded screen (especially on an iPad).
Not being able to collapse the choices would be my biggest concern. If a user checks all the variables, they never need to see that panel again, so hiding it would be beneficial.
And thinking about it, I just remembered that each panel is considered a "tile" in the upper right of CODAP. If we 'dock' the variables panel, a user wouldn't be able to find it with the 'tiles' unless they remembered it was attached to the sim.

Remembering this makes me lean towards not docking it.

samreid commented 5 months ago

@matthew-blackman and I discussed this problem.

@matthew-blackman has discussed a related layout problem with the CODAP team, and they are aware that web views appearing behind is a known problem, and a known problem for other clients. That was worked around with a setTimeout which doesn't appear to help for the webView
There is no documentation in the API about how to get or set the location of CODAP windows

We wanted to test in a self-contained reproducible minimal example to try to help understand if the problem is in our configuration or in the platform, so we started with the exemplar in the documentation: https://github.com/concord-consortium/codap-data-interactives/blob/master/NewRandomNumbers/random-numbers.js

We added one webview:

```diff Subject: [PATCH] Simplify the background gradient, and make it non-scaled, see https://github.com/phetsims/projectile-data-lab/issues/178 --- Index: NewRandomNumbers/random-numbers.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/NewRandomNumbers/random-numbers.js b/NewRandomNumbers/random-numbers.js --- a/NewRandomNumbers/random-numbers.js (revision 8a3a465487d89969b0269e317a92cb18d0a46fc8) +++ b/NewRandomNumbers/random-numbers.js (date 1710256056733) @@ -224,3 +224,11 @@ // handle errors console.log(msg); }); + +setTimeout(()=>{ + codapInterface.sendRequest({ + action: 'create', resource: 'component', values: { + type: 'webView', name: 'projectileDataWebView', title: 'Data to Collect', URL: 'https://www.google.com' + } + }); +},5000) ```

However, this still shows behind the data interactive:

We feel there may be a bug because using CODAP to create a web view does position it correctly. Options => Display Web Page:

@matthew-blackman says: We have spent hours on this and have no leads. We are questioning whether this is worth additional cost when it may not have a way forward.

Therefore, @matthew-blackman and I agree we will have to go with plan B which is currently docking to the right side of the window.

matthew-blackman commented 5 months ago

Patch for 3/13 pairing work with @samreid, building out the 'docked' view of the data stream controls:

``` diff Subject: [PATCH] Update keyboard help string - see https://github.com/phetsims/projectile-data-lab/issues/239 --- Index: repos/projectile-data-lab/wrappers/codap/projectileDataPanel.html IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/repos/projectile-data-lab/wrappers/codap/projectileDataPanel.html b/repos/projectile-data-lab/wrappers/codap/projectileDataPanel.html --- a/repos/projectile-data-lab/wrappers/codap/projectileDataPanel.html (revision 3724b2d6b0dc59c17ed7a82f282b7e33ba3e45c1) +++ b/repos/projectile-data-lab/wrappers/codap/projectileDataPanel.html (date 1710354356414) @@ -10,8 +10,6 @@ --> - - Projectile Data + + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file

samreid commented 5 months ago

@matthew-blackman and I got to a point where the docked checkbox panel was working properly. We were also overjoyed to eliminate the hack localStorage protocol which would likely have failed QA on some platforms.

We want to apply this dock pattern to the tool wrapper as well.

samreid commented 5 months ago

I'll take the next steps here...

samreid commented 5 months ago

I removed a lot of the hacks and workarounds, and was also able to refactor the different wrapper logic into individual files. Things are looking more understandable and maintainable here. There is a little bit of repeated boilerplate, but it is worth it. Let's check in with @matthew-blackman next.

matthew-blackman commented 5 months ago

This is looking great - nice work! The docked design is much more straightforward IMO, and cleans up the workaround we were using with localStorage. The wrappers for streaming data and tool data both look good. Closing.

phetsims / projectile-data-lab

CODAP wrapper: Layout issues #233

Categorical Variables

Quantitative Variables

Categorical Variables

Quantitative Variables