Skip to content

Commit

Permalink
Teacher tool: Show placeholder text when no project is loaded (#10303)
Browse files Browse the repository at this point in the history
This was some low hanging fruit we discussed after the customer interviews which I didn't get around to fixing until just now. Since I don't want to spend too long on it, I've just done a "bare minimum" implementation. Haven't put together any nice graphics or anything like that, but I think it's still an improvement.

I've also increased the contrast for placeholder text in the input field (a11y issue and also came up multiple times in the interviews) and fixed a null ref exception that came up in testing.
  • Loading branch information
thsparks authored Dec 5, 2024
1 parent 352be97 commit 405808c
Show file tree
Hide file tree
Showing 7 changed files with 55 additions and 17 deletions.
20 changes: 12 additions & 8 deletions teachertool/src/components/MakecodeFrame.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { setEditorRef } from "../services/makecodeEditorService";
import { AppStateContext } from "../state/appStateContext";
import { getEditorUrl } from "../utils";
import { classList } from "react-common/components/util";
import { logDebug } from "../services/loggingService";
import { MakeCodeFramePlaceholder } from "./MakecodeFramePlaceholder";

interface IProps {}

Expand Down Expand Up @@ -57,14 +57,18 @@ export const MakeCodeFrame: React.FC<IProps> = () => {
}

/* eslint-disable @microsoft/sdl/react-iframe-missing-sandbox */
const frameVisible = !!teacherTool.projectMetadata?.id;
return (
<iframe
id="code-eval-project-view-frame"
className={classList(css["makecode-frame"], teacherTool.projectMetadata?.id ? undefined : css["invisible"])}
src={frameUrl}
title={"title"}
ref={handleIFrameRef}
/>
<>
<iframe
id="code-eval-project-view-frame"
className={classList(css["makecode-frame"], frameVisible ? undefined : css["invisible"])}
src={frameUrl}
title={"title"}
ref={handleIFrameRef}
/>
{!frameVisible && <MakeCodeFramePlaceholder />}
</>
);
/* eslint-enable @microsoft/sdl/react-iframe-missing-sandbox */
};
14 changes: 14 additions & 0 deletions teachertool/src/components/MakecodeFramePlaceholder.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/// <reference path="../../../localtypings/pxteditor.d.ts" />

import css from "./styling/MakecodeFramePlaceholder.module.scss";

interface IProps {}
export const MakeCodeFramePlaceholder: React.FC<IProps> = () => {
return (
<div className={css["makecode-frame-placeholder"]}>
<i className="far fa-hand-point-up" />
<span>{lf("No project loaded.")}</span>
<span>{lf("Enter a share link above to evaluate!")}</span>
</div>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,6 @@
border-radius: 0.5rem;
border: solid 2px var(--pxt-headerbar-accent-smoke);
}

input::placeholder {
color: var(--pxt-content-accent);
}
}

.segment-container {
Expand Down
2 changes: 2 additions & 0 deletions teachertool/src/components/styling/MakeCodeFrame.module.scss
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,7 @@
// We should not use display: none here because it messes up blocks rendering when we
// request block images from the iframe.
visibility: hidden;
width: 0;
height: 0;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
.makecode-frame-placeholder {
width: 100%;
height: 100%;
margin: 0;
padding: 0;

align-items: center;
justify-content: center;
display: flex;
flex-direction: column;
gap: 1rem;

color: var(--pxt-page-foreground);
opacity: 0.65;
font-size: 1.5rem;
text-align: center;
user-select: none;

i {
font-size: 1.5rem;
}
}
2 changes: 1 addition & 1 deletion teachertool/src/style.overrides/Input.scss
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
padding-left: 0;

&::placeholder {
color: var(--pxt-content-accent);
color: var(--pxt-content-secondary-foreground);
font-style: italic;
}
}
Expand Down
8 changes: 4 additions & 4 deletions teachertool/src/transforms/setEvalResult.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,21 @@ function reportChanges(criteriaId: string, result: CriteriaResult) {
const previousResult = teacherTool.evalResults[criteriaId];
const criteriaInstance = getCriteriaInstanceWithId(teacherTool, criteriaId);

if (previousResult.result != result.result) {
if (previousResult?.result != result?.result) {
pxt.tickEvent(Ticks.SetEvalResultOutcome, {
catalogCriteriaId: criteriaInstance?.catalogCriteriaId ?? "",
newValue: EvaluationStatus[result.result],
oldValue: previousResult?.result ? EvaluationStatus[previousResult.result] : "",
newValueIsManual: result.resultIsManual + "",
newValueIsManual: result?.resultIsManual + "",
oldValueIsManual: previousResult?.resultIsManual + "",
});
}

if (previousResult.notes != result.notes) {
if (previousResult?.notes != result?.notes) {
// Setting notes is debounced so this isn't too noisy.
pxt.tickEvent(Ticks.SetEvalResultNotes, {
catalogCriteriaId: criteriaInstance?.catalogCriteriaId ?? "",
newLength: result.notes?.length ?? 0,
newLength: result?.notes?.length ?? 0,
oldLength: previousResult?.notes?.length ?? 0,
});
}
Expand Down

0 comments on commit 405808c

Please sign in to comment.