phetsims / calculus-grapher

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

Possible for netSignedAreaAccordionBox to extend offscreen with stringTest=dynamic #306

Closed Nancy-Salpepi closed 1 year ago

Nancy-Salpepi commented 1 year ago

Test device MacBook Air (and Dell)

Operating System macOS 13.2.1

Browser Safari 16.3

Problem description For https://github.com/phetsims/qa/issues/921, on the Integral Screen with stringTest=dynamic it is possible for netSignedAreaAccordionBox to extend past the bottom of the screen when the title is very long. This is also seen in master.

Steps to reproduce

  1. add ?stringTest=dynamic to url
  2. Go to the Integral Screen and check the Area Under Curve Checkbox
  3. Press the right arrow 3 times.

Visuals

Screenshot 2023-03-22 at 11 26 24 AM

Troubleshooting information: !!!!! DO NOT EDIT !!!!! Name: lyrics URL: https://phet-dev.colorado.edu/html/calculus-grapher/1.0.0-dev.25/phet/calculus-grapher_all_phet.html?stringTest=dynamic Version: 1.0.0-dev.25 2023-03-20 03:37:03 UTC Features missing: applicationcache, applicationcache, touch Flags: pixelRatioScaling User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/16.3 Safari/605.1.15 Language: en-US Window: 1337x710 Pixel Ratio: 2/1 WebGL: WebGL 1.0 GLSL: WebGL GLSL ES 1.0 (1.0) Vendor: WebKit (WebKit WebGL) Vertex: attribs: 16 varying: 30 uniform: 1024 Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 32) Max viewport: 16384x16384 OES_texture_float: true Dependencies JSON: {}

veillette commented 1 year ago

Thanks @Nancy-Salpepi . I was discussing this very point with @pixelzoom this morning.

For context, we top-align the accordion box with the chart rectangle, therefore the top alignment is never an issue. For the bottom alignment of the accordion box, we have come up with some magic number that assumes the title is two line long (such an for the English strings). These numbers were chosen such that the bottom of the accordion box matches the bottom of the chart rectangle. In practice, due to i18n, the title of the accordion box might be on one line-long, or possibly three -line long , in which case it wouldn't align with the bottom of the chart rectangle.

We don't have an easy way to rescale the vertical content of the accordion box in a way that is simple but we could give another try.

veillette commented 1 year ago

I went down the road of trying to find the appropriate height top the inner accordion box by working backward and estimating the height of the remaining component but that turned out to be a bit messy. Perhaps @pixelzoom has better ideas? or maybe this is a wont-fix?

pixelzoom commented 1 year ago

I'll take this one. Multiline titles in accordion boxes need a maxHeight, similar to what I did in another issue for multiline checkbox labels.

I'll also look at setting a fixed height for the acccordion boxes. But my understanding is that's not a requirement for anyrhing other than the English default strings.

pixelzoom commented 1 year ago

Fixed in the above commits, for both the "Net Signed Area" and "Slope of Tangent" accordion boxes, see screenshots below with ?stringTest=dynamic. The titles both have maxWidth: 80 and maxHeight: 40.

Note that it's not required that these accordion boxes maintain a fixed height -- and that's also something that's not currently supported by AccordionBox common code.

Back to @Nancy-Salpepi for review, close if OK.

screenshot_2464 screenshot_2463
Nancy-Salpepi commented 1 year ago

Looks good. Closing 🙂.