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

teacher tool: misc tidy stuff #9870

Merged
merged 1 commit into from
Feb 16, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
42 changes: 42 additions & 0 deletions .vscode/launch.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,48 @@
{
"version": "0.2.0",
"configurations": [
{
"name": "pxt ci (pxt-core)",
"type": "node",
"request": "launch",
"program": "${workspaceRoot}/built/pxt.js",
"stopOnEntry": false,
"args": [
"ci",
],
"cwd": "${workspaceRoot}",
"runtimeExecutable": null,
"runtimeArgs": [
"--nolazy"
],
"env": {
"NODE_ENV": "development"
},
"console": "integratedTerminal",
"sourceMaps": false,
"outFiles": []
},
{
"name": "pxt ci (microbit)",
"type": "node",
"request": "launch",
"program": "${workspaceRoot}/built/pxt.js",
"stopOnEntry": false,
"args": [
"ci",
],
"cwd": "${workspaceRoot}/../pxt-microbit",
"runtimeExecutable": null,
"runtimeArgs": [
"--nolazy"
],
"env": {
"NODE_ENV": "development"
},
"console": "integratedTerminal",
"sourceMaps": false,
"outFiles": []
},
{
"name": "pxt serve (microbit)",
"type": "node",
Expand Down
4 changes: 1 addition & 3 deletions cli/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2047,7 +2047,6 @@ async function buildSemanticUIAsync(parsed?: commandParser.ParsedCommand) {
generateReactCommonCss("skillmap"),
generateReactCommonCss("authcode"),
generateReactCommonCss("multiplayer"),
generateReactCommonCss("teachertool")
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm surprised. Did something change, or has this always been unnecessary?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's always been unnecessary, and was causing the react-common css to be loaded twice. Duplicates of every class.

]);

// Run postcss with autoprefixer and rtlcss
Expand Down Expand Up @@ -2076,8 +2075,7 @@ async function buildSemanticUIAsync(parsed?: commandParser.ParsedCommand) {
"blockly.css",
"react-common-skillmap.css",
"react-common-authcode.css",
"react-common-multiplayer.css",
"react-common-teachertool.css"
"react-common-multiplayer.css"
];

for (const cssFile of files) {
Expand Down
25 changes: 22 additions & 3 deletions cli/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1037,21 +1037,40 @@
};

const serveWebappFile = (webappName: string, webappPath: string) => {
const webappUri = url.parse(`http://localhost:3000/${webappPath}${uri.search || ""}`);
http.get(webappUri, r => {
const webappUri = url.parse(`http://127.0.0.1:3000/${webappPath}${uri.search || ""}`);
const request = http.get(webappUri, r => {
let body = "";
r.on("data", (chunk) => {
body += chunk;
});
r.on("end", () => {
if (body.includes("<title>Error</title>")) { // CRA development server returns this for missing files
res.writeHead(404, {
'Content-Type': 'text/html; charset=utf8',
});
res.write(body);
return res.end();
}
if (!webappPath || webappPath === "index.html") {
body = expandWebappHtml(webappName, body);
}
res.writeHead(200);
if (webappPath) {
res.writeHead(200, {
'Content-Type': U.getMime(webappPath),
});
} else {
res.writeHead(200, {
'Content-Type': 'text/html; charset=utf8',
});
}
res.write(body);
res.end();
});
});

Check failure

Code scanning / CodeQL

Server-side request forgery Critical

The
URL
of this request depends on a
user-provided value
.
The
URL
of this request depends on a user-provided value.
request.on("error", (e) => {
console.error(`Error fetching ${webappUri.href} .. ${e.message}`);
error(500, e.message);
});
};

const webappIdx = webappNames.findIndex(s => new RegExp(`^-{0,3}${s}$`).test(elts[0] || ''));
Expand Down
10 changes: 0 additions & 10 deletions react-common/styles/react-common-teachertool-core.less

This file was deleted.

12 changes: 0 additions & 12 deletions react-common/styles/react-common-teachertool.less

This file was deleted.

1 change: 0 additions & 1 deletion teachertool/.gitignore
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
.vscode
build
!package-lock.json
src/Fonts.css
91 changes: 62 additions & 29 deletions teachertool/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions teachertool/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
]
},
"devDependencies": {
"@babel/plugin-proposal-private-property-in-object": "^7.21.11",
"@types/react": "file:../node_modules/@types/react",
"@types/react-dom": "file:../node_modules/@types/react-dom",
"html-webpack-plugin": "^5.5.0",
Expand Down
5 changes: 2 additions & 3 deletions teachertool/public/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,16 @@
<meta name="theme-color" content="#000000" />
<meta
name="description"
content="MakeCode Teacher Tool. Automatically analyze and evaluate projects."
content="MakeCode Project Insights. Designed to help teachers evaluate student projects using a rubric."
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI, don't need to block on this, but in our sync yesterday, there was some disagreement around the name Project Insights. We probably need to have a meeting with branding folks to hash this out.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure. We can keep iterating on it.

/>
<link rel="stylesheet" data-rtl="/blb/rtlsemantic.css" href="/blb/semantic.css" type="text/css">
<link rel="stylesheet" href="/blb/icons.css" type="text/css">
<link rel="stylesheet" href="/blb/react-common-teachertool.css" type="text/css">
<!-- <link rel="apple-touch-icon" href="/teachertool-data/logo192.png" /> -->
<!--
manifest.json provides metadata used when your web app is installed on a
user's mobile device or desktop. See https://developers.google.com/web/fundamentals/web-app-manifest/
-->
<link rel="manifest" href="/teachertool-data/manifest.json" />
<!-- <link rel="manifest" href="/teachertool-data/manifest.json" /> -->
<title>MakeCode Teacher Tool</title>
<script>
// This line gets patched up by the cloud
Expand Down
5 changes: 0 additions & 5 deletions teachertool/src/index.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
// TODO: pxtcompiler type is only needed for a few compiler service types,
// we should get rid of this somehow.
/// <reference path="../../built/pxtcompiler.d.ts" />
/// <reference path="../../built/pxtsim.d.ts" />
/// <reference path="../../built/pxtlib.d.ts" />

import React from "react";
import ReactDOM from "react-dom";
// eslint-disable-next-line import/no-unassigned-import
import "./global.scss";
import { App } from "./App";
import { AppStateProvider } from "./state/appStateContext";
Expand Down Expand Up @@ -47,9 +45,6 @@ window.addEventListener("DOMContentLoaded", () => {

enableAnalytics();

// prefetch worker on load
pxt.worker.getWorker(pxt.webConfig.workerjs);

ReactDOM.render(
<React.StrictMode>
<AppStateProvider>
Expand Down