Scale Tutorial Text Alongside Workspace #9729
Conversation
| // Increase font size to match the editor's % zoom. | ||
| const maxEditorScale = this.editor.getMaxScale(); | ||
| const zoomAmt = (newScale - this.initialEditorScale) / (maxEditorScale - this.initialEditorScale); | ||
| const newTutorialFontSize = this.tutorialInitialFontSize + (this.tutorialMaxFontSize - this.tutorialInitialFontSize) * zoomAmt; |
There was a problem hiding this comment.
Can you explain this math a bit? I'm having a hard time understanding why we want to multiply zoom amount with the difference of tutorialMaxFontSize and tutorialInitialFontSize.
There was a problem hiding this comment.
This may be tricky without a whiteboard, but I'll give it a try :)
So the question we're trying to answer is how far between the minimum (initial) font and the maximum font we want the new font size to be. To do this, we're basically mapping how far between the editor's min and max scale we are (as a fraction between 0 and 1) and we want to be that same fraction between the font's min and max.
We start with the editor's current_scale, but the min_scale isn't 0, so to get a value between 0 and 1, the first thing we do is kind of "rebase" to a different number line where new_minimum_scale is 0 (min_scale - min_scale) and the new_max_scale is max_scale - min_scale. Then we see where the current scale falls along that same line (new_current_scale = current_scale - min_scale) and we get the fraction by saying new_current_scale / new_max_scale = (current_scale - min_scale) / (max_scale - min_scale), which will be some value between 0 and 1 since current_scale <= max_scale. So for the sake of argument, let's say that's 0.75.
Then to get the font size from that, we kind of do the opposite. We need to figure out where, between the min_font and max_font we should be. So we do the same thing where we rebase to a number line where new_min_font is 0 and the new_max_font is (max_font - min_font), and we know we want to be 0.75 of the way along that number line, so 0.75 * (max_font - min_font). Then to get the final, actual new font value, we just have to add the min_font back onto it.
Here's a diagram of questionable value:
Also happy to hop on a call and talk through it, if that'd be helpful.
There was a problem hiding this comment.
Yeah, this explanation was extremely helpful! Thank you for that clarification!!
webapp/src/blocks.tsx
Outdated
| else if (ev.type === Blockly.Events.VIEWPORT_CHANGE) { | ||
| const viewportChangeEvent = ev as Blockly.Events.ViewportChange; | ||
| if (this.initialScaleSet && viewportChangeEvent.oldScale !== viewportChangeEvent.scale) { | ||
| if (this.onScaleChanged) { |
There was a problem hiding this comment.
Is it possible that the function doesn't get initialized?
There was a problem hiding this comment.
Yeah, it's optional. For example, we only set it at the moment if we're in a tutorial, otherwise it remains undefined.
I should probably combine this with the above if statement though. No real reason to break it out into a separate one :)
| @@ -1108,6 +1116,8 @@ export class ProjectView | |||
| // subscribe to user preference changes (for simulator or non-render subscriptions) | |||
| data.subscribe(this.highContrastSubscriber, auth.HIGHCONTRAST); | |||
| data.subscribe(this.cloudStatusSubscriber, `${cloud.HEADER_CLOUDSTATE}:*`); | |||
|
|
|||
| document.getElementById("root").style.setProperty("--tutorialFontSize", `${this.tutorialInitialFontSize}rem`); | |||
There was a problem hiding this comment.
i would define this in css instead of adding it here and just override it when the scale changes. that way it'll always have at least some value... anything set on the root directly will override whatever is in the css styles.
There was a problem hiding this comment.
Yeah I went back and forth on this. Decided to put it here to keep it all in once place, but happy to move it back to css.
| @@ -1532,6 +1542,26 @@ export class ProjectView | |||
| } | |||
| } | |||
|
|
|||
| onScaleChanged(oldScale: number, newScale: number) { | |||
There was a problem hiding this comment.
passing in the oldScale here to set the initial scale seems kinda fragile. is the initial scale not always the same value? and failing that, can't we just read it from blockly when we load the workspace?
There was a problem hiding this comment.
It's different between Monaco and Blockly. I'll see about reading it when we load the workspace. Should be do-able.
|
not sure if this should be behind a target flag or not. are we sure we'll want this in all targets? |
|
Putting this on hold. Sounds like we probably don't want this - or at the very least, we may only want to scope it to HoC. But even then, browser zoom would be preferable. |
Fixes https://github.com/microsoft/pxt-minecraft/issues/2050 by making tutorial text size increase when the workspace is zoomed in.
I went through a few different approaches to this. Initially, I hooked into the zoom button events directly, but you could get out of sync if you used "ctrl + mouse wheel" to zoom the workspace, so I decided to track change-in-scale events instead.
I also opted to scale the text relative to zoom % of the workspace (as opposed to increasing/decreasing by a fixed amount on every zoom in/out). I did this because different ways of zooming (mouse wheel vs workspace buttons vs "ctrl +/-" in monaco) would zoom in different amounts, and it looked strange to always use a fixed change to the tutorial-text size regardless. It also led to some unexpected behavior if you zoomed in with one technique but out with another.
Lastly, I prevent the tutorial text from ever shrinking beyond its initial size, just because it gets very hard to read and looks a little silly when that happens. I'm not sure there's a realistic use case for it.
Upload target: https://minecraft.makecode.com/app/bfd4e1e83bcc22c2b26d7062d853a80e4555fb59-9dbaf7a159