phetsims / build-a-nucleus

"Build a Nucleus" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
0 stars 5 forks source link

Support InteractiveHighlights? #161

Open zepumph opened 11 months ago

zepumph commented 11 months ago

Looks like that option is auto-added to preferences when I support Interactive Description (in #96). That makes sense, but I only found it just now. I think we only need to add support to the Particles and their creator node, so it would be a small lift (I think).

zepumph commented 11 months ago

This commit get's us pretty much good with the particles, but when dragging out a particle, the highlight stays on the creator.

@jessegreenberg also mentioned on slack that perhaps I shouldn't even need the commit in the first place. Can you please help me out?

jessegreenberg commented 11 months ago

This is happening because of the behavior of "locking" focus to the pointer during input. Commenting this out fixes the problem.

https://github.com/phetsims/scenery/blob/470ef91c2d916d1b2f018f8853ab66ac0b07b99b/js/accessibility/voicing/InteractiveHighlighting.ts#L359-L361

I think we discovered it is working in geometric-optics because the 'creator' Node becomes invisible/non-interactive.

I don't think I can work on this during this iteration, sorry! Ill open a scenery issue to schedule for work sometime soon.

zepumph commented 11 months ago

Also noting that on CT last night, this started showing up:

build-a-nucleus : fuzz : unbuilt : query-parameters-wrong
http://128.138.93.172//continuous-testing/ct-snapshots/1692666027301/build-a-nucleus/build-a-nucleus_en.html?continuousTest=%7B%22test%22%3A%5B%22build-a-nucleus%22%2C%22fuzz%22%2C%22unbuilt%22%2C%22query-parameters-wrong%22%5D%2C%22snapshotName%22%3A%22snapshot-1692666027301%22%2C%22timestamp%22%3A1692670706686%7D&brand=phet&ea&fuzz&decayScreenProtons=200&decayScreenNeutrons=200&chartIntoScreenProtons=200&chartIntoScreenNeutrons=200
Query: brand=phet&ea&fuzz&decayScreenProtons=200&decayScreenNeutrons=200&chartIntoScreenProtons=200&chartIntoScreenNeutrons=200
Uncaught Error: Assertion failed: tried to removeListener on something that wasn't a listener
Error: Assertion failed: tried to removeListener on something that wasn't a listener
at window.assertions.assertFunction (http://128.138.93.172//continuous-testing/ct-snapshots/1692666027301/assert/js/assert.js:28:13)
at assert (TinyEmitter.ts:173:6)
at removeListener (TinyProperty.ts:150:9)
at unlink (ReadOnlyProperty.ts:429:22)
at unlink (InteractiveHighlighting.ts:251:62)
at dispose (ParticleView.ts:113:10)
at call (Disposable.ts:50:21)
at dispose (BANScreenView.ts:396:21)
at listener (TinyEmitter.ts:122:8)
at emit (Disposable.ts:85:25)
[URL] http://128.138.93.172//continuous-testing/aqua/html/sim-test.html?url=..%2F..%2Fct-snapshots%2F1692666027301%2Fbuild-a-nucleus%2Fbuild-a-nucleus_en.html&simQueryParameters=brand%3Dphet%26ea%26fuzz%26decayScreenProtons%3D200%26decayScreenNeutrons%3D200%26chartIntoScreenProtons%3D200%26chartIntoScreenNeutrons%3D200&testInfo=%7B%22test%22%3A%5B%22build-a-nucleus%22%2C%22fuzz%22%2C%22unbuilt%22%2C%22query-parameters-wrong%22%5D%2C%22snapshotName%22%3A%22snapshot-1692666027301%22%2C%22timestamp%22%3A1692670706686%7D
[NAVIGATED] http://128.138.93.172//continuous-testing/aqua/html/sim-test.html?url=..%2F..%2Fct-snapshots%2F1692666027301%2Fbuild-a-nucleus%2Fbuild-a-nucleus_en.html&simQueryParameters=brand%3Dphet%26ea%26fuzz%26decayScreenProtons%3D200%26decayScreenNeutrons%3D200%26chartIntoScreenProtons%3D200%26chartIntoScreenNeutrons%3D200&testInfo=%7B%22test%22%3A%5B%22build-a-nucleus%22%2C%22fuzz%22%2C%22unbuilt%22%2C%22query-parameters-wrong%22%5D%2C%22snapshotName%22%3A%22snapshot-1692666027301%22%2C%22timestamp%22%3A1692670706686%7D
[NAVIGATED] about:blank
[NAVIGATED] http://128.138.93.172//continuous-testing/ct-snapshots/1692666027301/build-a-nucleus/build-a-nucleus_en.html?continuousTest=%7B%22test%22%3A%5B%22build-a-nucleus%22%2C%22fuzz%22%2C%22unbuilt%22%2C%22query-parameters-wrong%22%5D%2C%22snapshotName%22%3A%22snapshot-1692666027301%22%2C%22timestamp%22%3A1692670706686%7D&brand=phet&ea&fuzz&decayScreenProtons=200&decayScreenNeutrons=200&chartIntoScreenProtons=200&chartIntoScreenNeutrons=200
[CONSOLE] enabling assert
[CONSOLE] Invalid value supplied for key "decayScreenNeutrons": 200
[CONSOLE] Invalid value supplied for key "decayScreenProtons": 200
[CONSOLE] continuous-test-load
[CONSOLE] Assertion failed: tried to removeListener on something that wasn't a listener
[PAGE ERROR] Error: Error: Assertion failed: tried to removeListener on something that wasn't a listener
at window.assertions.assertFunction (http://128.138.93.172//continuous-testing/ct-snapshots/1692666027301/assert/js/assert.js:28:13)
at TinyProperty.removeListener (http://128.138.93.172//continuous-testing/ct-snapshots/1692666027301/chipper/dist/js/axon/js/TinyEmitter.js:142:7)
at TinyProperty.unlink (http://128.138.93.172//continuous-testing/ct-snapshots/1692666027301/chipper/dist/js/axon/js/TinyProperty.js:133:10)
at DerivedProperty.unlink (http://128.138.93.172//continuous-testing/ct-snapshots/1692666027301/chipper/dist/js/axon/js/ReadOnlyProperty.js:346:23)
at ParticleView.dispose (http://128.138.93.172//continuous-testing/ct-snapshots/1692666027301/chipper/dist/js/scenery/js/accessibility/voicing/InteractiveHighlighting.js:205:63)
at ParticleView.dispose (http://128.138.93.172//continuous-testing/ct-snapshots/1692666027301/chipper/dist/js/shred/js/view/ParticleView.js:78:11)
at Disposable.dispose (http://128.138.93.172//continuous-testing/ct-snapshots/1692666027301/chipper/dist/js/axon/js/Disposable.js:42:22)
at http://128.138.93.172//continuous-testing/ct-snapshots/1692666027301/chipper/dist/js/build-a-nucleus/js/common/view/BANScreenView.js:309:22
at TinyEmitter.emit (http://128.138.93.172//continuous-testing/ct-snapshots/1692666027301/chipper/dist/js/axon/js/TinyEmitter.js:94:9)
at BANParticle.dispose (http://128.138.93.172//continuous-testing/ct-snapshots/1692666027301/chipper/dist/js/axon/js/Disposable.js:70:26)
[CONSOLE] continuous-test-error

https://github.com/phetsims/scenery/blob/470ef91c2d916d1b2f018f8853ab66ac0b07b99b/js/accessibility/voicing/InteractiveHighlighting.ts#L251

I'm pretty sure I'd like to punt on this on disable interactive highlights until we have more bandwidth to investigate these.

zepumph commented 11 months ago

Ok Interactive Highlights is out for now. I'll confirm in slack with designers.