Return button can be mostly offscreen

[Nancy-Salpepi]

Test device
MacBook Air M1 chip

Operating System
13.2.1

Browser
Safari

Problem description
For phetsims/qa#917, if I set up a ten frame on the left side of the screen and add an object, the return button is mostly offscreen.

Visuals

Troubleshooting information: !!!!! DO NOT EDIT !!!!! Name: ‪Number Compare‬ URL: https://phet-dev.colorado.edu/html/number-compare/1.0.0-dev.6/phet/number-compare_all_phet.html Version: 1.0.0-dev.6 2023-03-15 01:30:22 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: 1203x654 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: {}

[amanda-phet]

I think this is a weird situation because the button isn't always there, and if we change the drag bounds so that the user can't drag this button off-screen, then ten frames would never be able to exist close to the left edge. But if you drag an empty ten frame, you can get closer. This seems strange to me as well, and if we start coming up with logic that involves bouncing something back in the play area or other edge cases, this becomes a huge thing.

Given the nature of this sim's timeline and resources, I would prefer to defer this until we have a client who wants a specific behavior.

@chrisklus will spend 30 minutes investigating the options and report back.

[chrisklus]

@marlitas @amanda-phet and I worked on this issue. This is tricky because we are constraining the position of the ten frame by its model bounds, which does not include the return button, and there is only return button in the view. We have an idea to use a constant from the size of the return button to change the constraining function in the ten frame's model. BUT now I'm realizing that would be importing a view thing into a model file which isn't great. Patch below to show where this constant would be factored into the model constraint.

Index: js/lab/model/TenFrame.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/lab/model/TenFrame.ts b/js/lab/model/TenFrame.ts
--- a/js/lab/model/TenFrame.ts	(revision ca4f7ef4a637f299101e2edce92b3cf6db6eb7ad)
+++ b/js/lab/model/TenFrame.ts	(date 1679515340485)
@@ -132,7 +132,7 @@
    * Determine how this ten frame's origin can be placed in the provided bounds.
    */
   public getOriginBounds( viewBounds: Bounds2 ): Bounds2 {
-    this.originBounds.minX = viewBounds.left - this.localBounds.left;
+    this.originBounds.minX = viewBounds.left - this.localBounds.left + 36;
     this.originBounds.minY = viewBounds.top - this.localBounds.top;
     this.originBounds.maxX = viewBounds.right - this.localBounds.right;
     this.originBounds.maxY = viewBounds.bottom - this.localBounds.bottom;

[chrisklus]

@zepumph and I handled this in the above commit. We made space for the return button when dragging whether it is visible or not. We were also able to do this in the view so we didn't need to import any constants to get the width of the return button.

Back over to @Nancy-Salpepi to test on phettest, thanks! Feel free to close if looking good.

[Nancy-Salpepi]

This looks good initially, but if I make my window size smaller, it moves out of view again.

arrowlost.mp4

[zepumph]

My guess is that this is correct, since this is the kind of sim that expands its controls to the edge of the screen instead of keeping to the dev bounds.

This is the same for a sim like projectile-motion:

Over to @chrisklus to close if all is well.

[chrisklus]

I think @zepumph may have missed what @Nancy-Salpepi was demonstrating in #29 (comment). That still looks incorrect to me and inconsistent with the drag behavior that was fixed in earlier commits. Leaving self-assigned to fix.

[zepumph]

Ahh yes, thanks. Sorry @Nancy-Salpepi, I was not paying close enough attention. So rude. I'll take a look now.

[zepumph]

Ok. @chrisklus and I weren't handling the general change when the play area drag bounds changed, and instead only the dragging logic. I set things up so all dragBounds changes go through the Node so that the return button bounds can be offset. @Nancy-Salpepi this should be fixed now. Please review.

[Nancy-Salpepi]

OK! Check this one off the list too!