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

CT: Corner overlap on left edge #171

Closed
pixelzoom opened this issue Jan 16, 2019 · 7 comments
Closed

CT: Corner overlap on left edge #171

pixelzoom opened this issue Jan 16, 2019 · 7 comments

Comments

@pixelzoom
Copy link
Contributor

pixelzoom commented Jan 16, 2019

@jbphet put this in #170, and it's unrelated to that issue. So I'm moving it here.

This is occurring in griddle, projectile-motion, scenery-phet, wave-interference.

Error: Assertion failed: Corner overlap on left edge
    at window.assertions.assertFunction (https://bayes.colorado.edu/continuous-testing/snapshot-1547658484416/assert/js/assert.js:22:13)
    at Function.Shape.roundedRectangleWithRadii (https://bayes.colorado.edu/continuous-testing/snapshot-1547658484416/kite/js/Shape.js?bust=1547660315914:1952:15)
    at createButtonShape (https://bayes.colorado.edu/continuous-testing/snapshot-1547658484416/sun/js/buttons/RectangularButtonView.js?bust=1547660315914:38:18)
    at ComboBoxButton.RectangularButtonView [as constructor] (https://bayes.colorado.edu/continuous-testing/snapshot-1547658484416/sun/js/buttons/RectangularButtonView.js?bust=1547660315914:115:28)
    at new RectangularPushButton (https://bayes.colorado.edu/continuous-testing/snapshot-1547658484416/sun/js/buttons/RectangularPushButton.js?bust=1547660315914:47:27)
    at new ComboBoxButton (https://bayes.colorado.edu/continuous-testing/snapshot-1547658484416/sun/js/ComboBoxButton.js?bust=1547660315914:111:7)
    at new ComboBox (https://bayes.colorado.edu/continuous-testing/snapshot-1547658484416/sun/js/ComboBox.js?bust=1547660315914:262:19)
    at new IntroProjectilePanel (https://bayes.colorado.edu/continuous-testing/snapshot-1547658484416/projectile-motion/js/intro/view/IntroProjectilePanel.js?bust=1547660315914:101:36)
    at new IntroScreenView (https://bayes.colorado.edu/continuous-testing/snapshot-1547658484416/projectile-motion/js/intro/view/IntroScreenView.js?bust=1547660315914:33:33)
    at IntroScreen.createView (https://bayes.colorado.edu/continuous-testing/snapshot-1547658484416/projectile-motion/js/intro/IntroScreen.js?bust=1547660315914:37:34)
@pixelzoom
Copy link
Contributor Author

pixelzoom commented Jan 16, 2019

The failing assertion is at KITE/Shape line 1952, in function roundedRectangleWithRadii:

assert && assert( topLeftRadius + bottomLeftRadius <= height, 'Corner overlap on left edge' );

This creates the rounded rectangle for the ComboBoxButton (a subclass of RectangularPushButton).

The default in ComboBox and ComboBoxButton is cornerRadius: 8, which is propagated to RectangularPushButton. projectile-motion is not overriding the default, see IntroProjectilePanel line 101:

    var projectileChoiceComboBox = new ComboBox(
      comboBoxItems,
      selectedProjectileObjectTypeProperty,
      comboBoxListParent, {
        xMargin: 12,
        yMargin: 7,
        buttonLineWidth: comboBoxLineWidth,
        listLineWidth: comboBoxLineWidth
      }
    );

My guess that the default radius is too large for projectile-motion's unusually tiny combo box.

pixelzoom added a commit to phetsims/circuit-construction-kit-black-box-study that referenced this issue Jan 16, 2019
pixelzoom added a commit to phetsims/models-of-the-hydrogen-atom that referenced this issue Jan 16, 2019
pixelzoom added a commit to phetsims/sun that referenced this issue Jan 16, 2019
@pixelzoom
Copy link
Contributor Author

Indeed, griddle, projectile-motion, and wave-interference all had items ("content" for the combo box button) whose height was too small to work with ComboBoxButton's default cornerRadius and margin. So I made the following adjustments in the above commits:

(1) Revised the defaults in ComboBox and ComboBoxButton for cornerRadius, xMargin, and yMargin to work with more typical size of items.
(2) Adjusted ComboBox cornerRadius value in griddle, projectile-motion, and wave-interference, since their items are atypically small.
(3) Specially set ComboBox options in sims that were relying on the old default values.

I also created issue phetsims/sun#448, since this exposed a general problem with RectangularPushButton.

@jbphet please review.

@pixelzoom
Copy link
Contributor Author

I'm still seeing this issue in CT for FPAF, MOTHA, and Under Pressure. But it's intermittent, and I'm not sure why, because this is an issue when the ComboBox is constructed. Perhaps there is some ComboBox in these sims that is only constructed when a specific "scene" is selected.

@pixelzoom pixelzoom assigned pixelzoom and unassigned jbphet Jan 17, 2019
@pixelzoom
Copy link
Contributor Author

pixelzoom commented Jan 17, 2019

Ah... This is occurring with test "xss-fuzz". I assume that must mean "stringTest=xss" query parameter. And in fact, that's the only way that I can reproduce this.

With "stringTest=xss" the combo box's content is a very long string, and it gets scaled down so that it's height is very small. That will cause the corners of the button to overlap.

From the code-review check list:

  • Does the sim stay on the sim page (doesn't redirect to an external page) when running with the query parameter stringTest=xss? This test passes if sim does not redirect, OK if sim crashes or fails to fully start. Only test on one desktop platform.

So (a) there's no requirement to fix this, (b) why is CT running a fuzz test with stringTest=xss, and (c) why is CT reporting anything other than a redirect for stringTest=xss?

I'm also wondering why this is intermittent in CT. Is "xss-fuzz" not being run sometimes?

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Jan 17, 2019

@jbphet when you include CT output in an issue, please include the complete output. In #171 (comment), you omitted a very important line, which contained a crucial clue about why the ComboBox content's height was atypically small:

projectile-motion : xss-fuzz : run

@pixelzoom
Copy link
Contributor Author

1/17/19 dev meeting conclusions:

  • For now, detect this problem in RectangularPushButton and set cornerRadius:0. Log a warning to console.

@pixelzoom
Copy link
Contributor Author

This will be addressed in phetsims/sun#448.

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