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

Scale Tutorial Text Alongside Workspace #9729

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions pxteditor/editor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ namespace pxt.editor {
zoomIn(): void;
zoomOut(): void;
resize(): void;
getMaxScale(): number;
setScale(scale: number): void;
onScaleChanged: (oldScale: number, newScale: number) => void;
}

export interface IFile {
Expand Down Expand Up @@ -102,6 +104,7 @@ namespace pxt.editor {
extensionsVisible?: boolean;
isMultiplayerGame?: boolean; // Arcade: Does the current project contain multiplayer blocks?
onboarding?: pxt.tour.BubbleStep[];
tutorialFontSize?: number;
}

export interface EditorState {
Expand Down
5 changes: 2 additions & 3 deletions theme/tutorial-sidebar.less
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
@defaultTutorialHeight: 18.625rem;
@tutorialFontSize: 1.125rem;
@tutorialTitleFontSize: 1.5rem;

@tutorialPrimaryColor: @sidebarPrimaryColor;
Expand Down Expand Up @@ -55,7 +54,7 @@
overflow-x: hidden;
overflow-y: auto;
font-family: @segoeUIFont;
font-size: @tutorialFontSize;
font-size: var(--tutorialFontSize);

span.docs.inlineblock {
white-space: break-spaces;
Expand Down Expand Up @@ -248,7 +247,7 @@
margin: 0 1rem;
color: @white;
background: @tutorialPrimaryColor;
font-size: @tutorialFontSize;
font-size: var(--tutorialFontSize);
white-space: nowrap;
overflow: hidden;
text-overflow: ellipsis;
Expand Down
30 changes: 30 additions & 0 deletions webapp/src/app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,10 @@ export class ProjectView
private rootClasses: string[];
private pendingImport: pxt.Util.DeferredPromise<void>;

private initialEditorScale: number;
private tutorialInitialFontSize = 1.125; // rem
private tutorialMaxFontSize = 2; // rem

private highContrastSubscriber: data.DataSubscriber = {
subscriptions: [],
onDataChanged: () => {
Expand Down Expand Up @@ -211,6 +215,7 @@ export class ProjectView
this.exitTutorial = this.exitTutorial.bind(this);
this.setEditorOffset = this.setEditorOffset.bind(this);
this.resetTutorialTemplateCode = this.resetTutorialTemplateCode.bind(this);
this.onScaleChanged = this.onScaleChanged.bind(this);
this.initSimulatorMessageHandlers();

// add user hint IDs and callback to hint manager
Expand Down Expand Up @@ -1029,7 +1034,10 @@ export class ProjectView
}
this.allEditors = [this.pxtJsonEditor, this.gitjsonEditor, this.blocksEditor, this.serialEditor, this.assetEditor, this.textEditor]
this.allEditors.forEach(e => e.changeCallback = changeHandler)
this.allEditors.forEach(e => e.onScaleChanged = this.onScaleChanged)

this.editor = this.allEditors[this.allEditors.length - 1]
this.initialEditorScale = undefined;
}

public UNSAFE_componentWillMount() {
Expand Down Expand Up @@ -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`);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}

public componentWillUnmount() {
Expand Down Expand Up @@ -1532,6 +1542,26 @@ export class ProjectView
}
}

onScaleChanged(oldScale: number, newScale: number) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's different between Monaco and Blockly. I'll see about reading it when we load the workspace. Should be do-able.

if (this.isTutorial && oldScale !== newScale) {
if (this.initialEditorScale === undefined) {
this.initialEditorScale = oldScale;
}

const root = document.getElementById("root");
if (root && newScale <= this.initialEditorScale) {
// Do not shrink the text beyond its initial size.
root.style.setProperty("--tutorialFontSize", `${this.tutorialInitialFontSize}rem`);
} else if (root) {
// 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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:
image

Also happy to hop on a call and talk through it, if that'd be helpful.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this explanation was extremely helpful! Thank you for that clarification!!

root.style.setProperty("--tutorialFontSize", `${newTutorialFontSize}rem`);
}
}
}

///////////////////////////////////////////////////////////
//////////// Load header /////////////
///////////////////////////////////////////////////////////
Expand Down
20 changes: 19 additions & 1 deletion webapp/src/blocks.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export class Editor extends toolboxeditor.ToolboxEditor {
compilationResult: pxt.blocks.BlockCompilationResult;
isFirstBlocklyLoad = true;
functionsDialog: CreateFunctionDialog = null;
onScaleChanged: (oldScale: number, newScale: number) => void;

showCategories: boolean = true;
breakpointsByBlock: pxt.Map<number>; // Map block id --> breakpoint ID
Expand All @@ -48,6 +49,10 @@ export class Editor extends toolboxeditor.ToolboxEditor {

public nsMap: pxt.Map<toolbox.BlockDefinition[]>;

// Blockly fires a scale event when it loads, but the user hasn't actually changed the scale.
// We use this flag to ignore that initial event.
private initialScaleSet: boolean = false;

constructor(parent: pxt.editor.IProjectView) {
super(parent);

Expand Down Expand Up @@ -581,7 +586,6 @@ export class Editor extends toolboxeditor.ToolboxEditor {
Blockly.Events.UI,
Blockly.Events.SELECTED,
Blockly.Events.CLICK,
Blockly.Events.VIEWPORT_CHANGE,
Blockly.Events.BUBBLE_OPEN
];

Expand Down Expand Up @@ -636,6 +640,15 @@ export class Editor extends toolboxeditor.ToolboxEditor {
}
}
}
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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible that the function doesn't get initialized?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 :)

this.onScaleChanged(viewportChangeEvent.oldScale, viewportChangeEvent.scale);
}
}
this.initialScaleSet = true;
}

// reset tutorial hint animation on any blockly event
if (this.parent.state.tutorialOptions != undefined) {
Expand Down Expand Up @@ -753,6 +766,11 @@ export class Editor extends toolboxeditor.ToolboxEditor {
this.editor.zoomCenter(-0.8);
}

getMaxScale() {
if (!this.editor) return undefined;
return this.editor.options.zoomOptions.maxScale;
}

setScale(scale: number) {
if (!this.editor) return;
if (scale != this.editor.scale) {
Expand Down
20 changes: 19 additions & 1 deletion webapp/src/monaco.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,7 @@ export class Editor extends toolboxeditor.ToolboxEditor {
extraLibs: pxt.Map<monaco.IDisposable>;
nsMap: pxt.Map<toolbox.BlockDefinition[]>;
giveFocusOnLoading: boolean = true;
onScaleChanged: (oldScale: number, newScale: number) => void;

protected fieldEditors: FieldEditorManager;
protected feWidget: ViewZoneEditorHost | ModalEditorHost;
Expand Down Expand Up @@ -968,9 +969,13 @@ export class Editor extends toolboxeditor.ToolboxEditor {
this.editor.onDidLayoutChange((e: monaco.editor.EditorLayoutInfo) => {
// Update editor font size in settings after a ctrl+scroll zoom
let currentFont = this.getEditorFontSize();
if (this.parent.settings.editorFontSize != currentFont) {
let prevFont = this.parent.settings.editorFontSize;
if (prevFont != currentFont) {
this.parent.settings.editorFontSize = currentFont;
this.forceDiagnosticsUpdate();
if(this.onScaleChanged) {
this.onScaleChanged(prevFont, currentFont);
}
}
// Update widgets
const toolbox = document.getElementById('monacoToolboxDiv');
Expand Down Expand Up @@ -1184,6 +1189,10 @@ export class Editor extends toolboxeditor.ToolboxEditor {
this.parent.settings.editorFontSize = currentFont + 1;
this.editor.updateOptions({ fontSize: this.parent.settings.editorFontSize });
this.forceDiagnosticsUpdate();

if (this.onScaleChanged) {
this.onScaleChanged(currentFont, this.parent.settings.editorFontSize);
}
}

zoomOut() {
Expand All @@ -1193,6 +1202,15 @@ export class Editor extends toolboxeditor.ToolboxEditor {
this.parent.settings.editorFontSize = currentFont - 1;
this.editor.updateOptions({ fontSize: this.parent.settings.editorFontSize });
this.forceDiagnosticsUpdate();

if (this.onScaleChanged) {
this.onScaleChanged(currentFont, this.parent.settings.editorFontSize);
}
}

getMaxScale() {
if (!this.editor) return undefined;
return MAX_EDITOR_FONT_SIZE;
}

private loadReference() {
Expand Down
2 changes: 2 additions & 0 deletions webapp/src/srceditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export class Editor implements pxt.editor.IEditor {
isVisible = false;
constructor(public parent: ProjectView) {
}
onScaleChanged: (oldScale: number, newScale: number) => void;
changeCallback = () => { };
setVisible(v: boolean) {
this.isVisible = v;
Expand Down Expand Up @@ -86,6 +87,7 @@ export class Editor implements pxt.editor.IEditor {

zoomIn() { }
zoomOut() { }
getMaxScale() { return 1; }
setScale(scale: number) { }

closeFlyout() { }
Expand Down
Loading