search

phetsims / ratio-and-proportion

"Ratio and Proportion" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
1 stars 4 forks source link

Avoid decorator pattern for the ratio lock toggle #555

Closed jessegreenberg closed 2 months ago

jessegreenberg commented 2 months ago

In https://github.com/phetsims/sun/issues/860, we are making it so you cannot decorate UI componenents that set content with additional Nodes. The ratio lock toggle button was identified as a button that gets a child added to it. Changed in the following commit, @zepumph can you please review or change how you would like?

zepumph commented 2 months ago

I think CT is breaking because of this issue:


ratio-and-proportion : xss-fuzz
URL: http://128.138.93.172/continuous-testing/ct-snapshots/1724341777073/ratio-and-proportion/ratio-and-proportion_en.html?continuousTest=%7B%22test%22%3A%5B%22ratio-and-proportion%22%2C%22xss-fuzz%22%5D%2C%22snapshotName%22%3A%22snapshot-1724341777073%22%2C%22timestamp%22%3A1724342504601%7D&brand=phet&ea&fuzz&stringTest=xss
ERROR: QUERY: brand=phet&ea&fuzz&stringTest=xss
Uncaught Error: Assertion failed: availableWidth should always be greater than width of component
STACK: Error: Assertion failed: availableWidth should always be greater than width of component
    at window.assertions.assertFunction (http://128.138.93.172/continuous-testing/ct-snapshots/1724341777073/assert/js/assert.js:45:13)
    at assert (RAPScreenView.ts:545:14)
    at scaleControls (RAPScreenView.ts:482:11)
    at listener (TinyEmitter.ts:213:6)
    at notifyLoop (TinyEmitter.ts:185:17)
    at emit (TinyStaticProperty.ts:56:9)
    at notifyListeners (Node.ts:1566:30)
    at onAccessAttempt (TinyStaticProperty.ts:31:9)
    at get (TinyStaticProperty.ts:56:20)
    at notifyListeners (Node.ts:1465:35)
    at onAccessAttempt (TinyStaticProperty.ts:31:9)
    at get (TinyStaticProperty.ts:56:20)
    at notifyListeners (Node.ts:1919:33)
    at setLocalBounds (Node.ts:1874:9)
    at  (LayoutNode.ts:46:22)
    at listener (TinyEmitter.ts:213:6)
    at notifyLoop (TinyEmitter.ts:185:17)
    at emit (ReadOnlyProperty.ts:343:22)
    at _notifyListeners (ReadOnlyProperty.ts:287:13)
    at unguardedSet (ReadOnlyProperty.ts:267:11)
====================
FULL LOG:
[URL] http://128.138.93.172/continuous-testing/aqua/html/sim-test.html?url=..%2F..%2Fct-snapshots%2F1724341777073%2Fratio-and-proportion%2Fratio-and-proportion_en.html&simQueryParameters=brand%3Dphet%26ea%26fuzz%26stringTest%3Dxss&duration=10000&testInfo=%7B%22test%22%3A%5B%22ratio-and-proportion%22%2C%22xss-fuzz%22%5D%2C%22snapshotName%22%3A%22snapshot-1724341777073%22%2C%22timestamp%22%3A1724342504601%7D
[NAVIGATED] http://128.138.93.172/continuous-testing/aqua/html/sim-test.html?url=..%2F..%2Fct-snapshots%2F1724341777073%2Fratio-and-proportion%2Fratio-and-proportion_en.html&simQueryParameters=brand%3Dphet%26ea%26fuzz%26stringTest%3Dxss&duration=10000&testInfo=%7B%22test%22%3A%5B%22ratio-and-proportion%22%2C%22xss-fuzz%22%5D%2C%22snapshotName%22%3A%22snapshot-1724341777073%22%2C%22timestamp%22%3A1724342504601%7D
[ATTACHED] 
[NAVIGATED] about:blank
[NAVIGATED] http://128.138.93.172/continuous-testing/ct-snapshots/1724341777073/ratio-and-proportion/ratio-and-proportion_en.html?continuousTest=%7B%22test%22%3A%5B%22ratio-and-proportion%22%2C%22xss-fuzz%22%5D%2C%22snapshotName%22%3A%22snapshot-1724341777073%22%2C%22timestamp%22%3A1724342504601%7D&brand=phet&ea&fuzz&stringTest=xss
[CONSOLE] enabling assert
[CONSOLE] Assertion failed:  availableWidth should always be greater than width of component
[CONSOLE] Debug info: {
  "seed": 0.5596015520387647,
  "currentScreenName": "HomeScreen"
}
[PAGE ERROR] Error: Error: Assertion failed: availableWidth should always be greater than width of component
    at window.assertions.assertFunction (http://128.138.93.172/continuous-testing/ct-snapshots/1724341777073/assert/js/assert.js:45:13)
    at CreateScreenView.scaleControls (http://128.138.93.172/continuous-testing/ct-snapshots/1724341777073/chipper/dist/js/ratio-and-proportion/js/common/view/RAPScreenView.js:407:15)
    at http://128.138.93.172/continuous-testing/ct-snapshots/1724341777073/chipper/dist/js/ratio-and-proportion/js/common/view/RAPScreenView.js:364:12
    at TinyStaticProperty.notifyLoop (http://128.138.93.172/continuous-testing/ct-snapshots/1724341777073/chipper/dist/js/axon/js/TinyEmitter.js:176:7)
    at TinyStaticProperty.emit (http://128.138.93.172/continuous-testing/ct-snapshots/1724341777073/chipper/dist/js/axon/js/TinyEmitter.js:154:18)
    at TinyStaticProperty.notifyListeners (http://128.138.93.172/continuous-testing/ct-snapshots/1724341777073/chipper/dist/js/axon/js/TinyStaticProperty.js:49:10)
    at VBox.validateBounds (http://128.138.93.172/continuous-testing/ct-snapshots/1724341777073/chipper/dist/js/scenery/js/nodes/Node.js:1364:31)
    at TinyStaticProperty.get (http://128.138.93.172/continuous-testing/ct-snapshots/1724341777073/chipper/dist/js/axon/js/TinyStaticProperty.js:26:10)
    at TinyStaticProperty.notifyListeners (http://128.138.93.172/continuous-testing/ct-snapshots/1724341777073/chipper/dist/js/axon/js/TinyStaticProperty.js:49:20)
    at VBox.validateBounds (http://128.138.93.172/continuous-testing/ct-snapshots/1724341777073/chipper/dist/js/scenery/js/nodes/Node.js:1274:36)
    at TinyStaticProperty.get (http://128.138.93.172/continuous-testing/ct-snapshots/1724341777073/chipper/dist/js/axon/js/TinyStaticProperty.js:26:10)
    at TinyStaticProperty.notifyListeners (http://128.138.93.172/continuous-testing/ct-snapshots/1724341777073/chipper/dist/js/axon/js/TinyStaticProperty.js:49:20)
    at VBox.setLocalBounds (http://128.138.93.172/continuous-testing/ct-snapshots/1724341777073/chipper/dist/js/scenery/js/nodes/Node.js:1687:34)
    at set localBounds [as localBounds] (http://128.138.93.172/continuous-testing/ct-snapshots/1724341777073/chipper/dist/js/scenery/js/nodes/Node.js:1650:10)
    at http://128.138.93.172/continuous-testing/ct-snapshots/1724341777073/chipper/dist/js/scenery/js/layout/nodes/LayoutNode.js:23:24
    at TinyProperty.notifyLoop (http://128.138.93.172/continuous-testing/ct-snapshots/1724341777073/chipper/dist/js/axon/js/TinyEmitter.js:176:7)
    at TinyProperty.emit (http://128.138.93.172/continuous-testing/ct-snapshots/1724341777073/chipper/dist/js/axon/js/TinyEmitter.js:154:18)
    at Property._notifyListeners (http://128.138.93.172/continuous-testing/ct-snapshots/1724341777073/chipper/dist/js/axon/js/ReadOnlyProperty.js:254:23)
    at Property.unguardedSet (http://128.138.93.172/continuous-testing/ct-snapshots/1724341777073/chipper/dist/js/axon/js/ReadOnlyProperty.js:201:14)
    at Property.set (http://128.138.93.172/continuous-testing/ct-snapshots/1724341777073/chipper/dist/js/axon/js/ReadOnlyProperty.js:183:12)
[CONSOLE] continuous-test-error
samreid commented 2 months ago

CT is very happy about this