Gamelab: showMobileControls() API #22609
Conversation
| @@ -1746,7 +1768,7 @@ a.WireframeButtons_active, div.WireframeButtons_active { | |||
|
|
|||
| #gameButtons { | |||
| width: auto; | |||
| padding: 1% 5%; | |||
| padding: 4px 5%; | |||
There was a problem hiding this comment.
The vertical padding was changed to be explicit, which is helpful for other height calculations in this area. Since gameButtons is rendered at 400px wide, 1% is equal to 4px
| @@ -939,7 +938,7 @@ GameLab.prototype.onDPadMouseMove = function (e) { | |||
| clientY = e.touches[0].clientY; | |||
| } | |||
|
|
|||
| if (experiments.isEnabled('fourWayDPad')) { | |||
| if (this.mobileControlsConfig.dpadFourWay) { | |||
There was a problem hiding this comment.
Is this PR also removing this experiment / enabling it for everyone? Or was this leftover from a past PR?
There was a problem hiding this comment.
yes, it removes the experiment. @poorvasingal wants the 4-way to be the default now, and the new API allows you to change it to 8-way.
There was a problem hiding this comment.
Can you provide a screenshot of the new appearance, since it looks like you've moved the controls?
I see a test for the new reducer function but can we get any coverage of the new droplet block, the React component, and/or the actual show/hide behavior being implemented here?
Is this block a complete no-op on desktop, or should it somehow indicate to the student that it's been called / will take effect on mobile? Maybe something as simple as logging to the console in desktop mode would prevent possible confusion.
apps/src/gamelab/GameLabP5.js
Outdated
| new window.p5(function (p5obj) { | ||
| this.p5 = p5obj; | ||
| this.p5.useQuadTree(false); | ||
| this.setP5FrameRate(); | ||
| this.gameLabWorld = new GameLabWorld(p5obj); | ||
|
|
||
| p5obj.showMobileControls = function ( |
There was a problem hiding this comment.
Do we get anything out of putting this on our p5 object? It seems like this could live someplace else.
There was a problem hiding this comment.
This is the most convenient place for it to be marshaled in the global space. But I can look at wedging it in specifically in other spot.
| @@ -32,7 +32,6 @@ | |||
| "hide_view_data_button": "false", | |||
| "debugger_disabled": "true", | |||
| "hide_animation_mode": "false", | |||
| "show_d_pad": "false", | |||
There was a problem hiding this comment.
A number of these levels have show_d_pad set to false, but you're removing that parameter and making the default true. Do we need to also update these levels to use the new block?
There was a problem hiding this comment.
this param hasn't really been used. when it was, it was primarily hiding it on the desktop. For about a year, it did nothing at all.
The new default is "not visible on desktop, visible on the mobile share page"
I don't believe that the level builders ever set this flag considering the mobile share experience.
|
I asked @poorvasingal about the idea of console logging in response to calling the function (based on your comment), given that it may not be obvious that it had any effect, but she felt like we are fine without logging. I'll take a look at other tests that could be created here. And I'll be sure to update the PR with a screenshot. |
|
Screenshot attached |
showMobileControls(spaceButtonVisible, dpadVisible, dpadFourWay, mobileOnly)API which controls the visibility of the space button and the d-pad, the behavior of the d-pad (four way vs eight way), and whether or not the controls appear only on mobile devices for this project. All values aretrueby default (if the student never calls the API)gameButtonsarea (instead of the within and to the right of the Run button), so that they can use the full 400px of width when visible on the/editpageresponsivescaling of the d-pad and space button and manual resizing using the vertical visualization resize barmobileControlsConfigas a property onGameLabwhich is routed into the redux storeshow_d_padproperty from the dashboard levels and level builder. It had not been used in a while (except for in the past few weeks as an override to force the d-pad off on mobile). This property conflicted with the new API.