From 94051a7085bccad55c103ed10d1d2104520163dc Mon Sep 17 00:00:00 2001 From: Richard Knoll Date: Wed, 17 Nov 2021 14:13:06 -0800 Subject: [PATCH] [stable7.3] Cherry pick badge fixes (#8602) * Adding certificate preview to the rewards modal and skillmap tests (#8592) * Adding certificate preview to the rewards modal and skillmap tests * Fix lint and remove extra css * Add the ability to specify a name for the badge * Make loading start earlier and add locked badges (#8600) * Make loading start earlier and add locked badges * pr feedback * Fix the tilemap wall cursor (#8601) --- gulpfile.js | 8 +- localtypings/pxtarget.d.ts | 1 + react-common/components/profile/Badge.tsx | 8 +- react-common/components/profile/BadgeInfo.tsx | 2 +- react-common/styles/profile/profile.css | 6 +- skillmap/.eslintrc.js | 1 + skillmap/src/App.tsx | 22 ++- skillmap/src/components/AppModal.tsx | 9 +- skillmap/src/lib/skillMap.d.ts | 2 + skillmap/src/lib/skillMapParser.ts | 152 ++++++++++++++---- skillmap/src/lib/skillMapUtils.ts | 5 +- skillmap/src/styles/modal.css | 17 ++ skillmap/tests/skillmapParser.spec.ts | 121 ++++++++++++++ skillmap/tests/tsconfig.json | 28 ++++ theme/pxt.less | 1 - .../components/ImageEditor/ImageCanvas.tsx | 4 + 16 files changed, 332 insertions(+), 55 deletions(-) create mode 100644 skillmap/tests/skillmapParser.spec.ts create mode 100644 skillmap/tests/tsconfig.json diff --git a/gulpfile.js b/gulpfile.js index c6567d7fd02d..a5373e000e67 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -602,6 +602,10 @@ const copySkillmapHtml = () => rimraf("webapp/public/skillmap.html") const skillmap = gulp.series(cleanSkillmap, buildSkillmap, gulp.series(copySkillmapCss, copySkillmapJs, copySkillmapHtml)); +const buildSkillmapTests = () => compileTsProject("skillmap/tests", "built/tests"); +const runSkillmapTests = () => exec("./node_modules/.bin/mocha ./built/tests/tests/skillmapParser.spec.js", true) + +const testSkillmap = gulp.series(buildSkillmapTests, runSkillmapTests); /******************************************************** Tests and Linting @@ -652,7 +656,8 @@ const testAll = gulp.series( testpytraces, testtutorials, testlanguageservice, - karma + karma, + testSkillmap ) function testTask(testFolder, testFile) { @@ -723,6 +728,7 @@ exports.webapp = gulp.series( browserifyWebapp ) +exports.skillmapTest = testSkillmap; exports.updatestrings = updatestrings; exports.updateblockly = copyBlockly; exports.lint = lint diff --git a/localtypings/pxtarget.d.ts b/localtypings/pxtarget.d.ts index 83a3ada8e422..55c9a3a5b2dd 100644 --- a/localtypings/pxtarget.d.ts +++ b/localtypings/pxtarget.d.ts @@ -1162,6 +1162,7 @@ declare namespace pxt.auth { type: BadgeType; title: string; image: string; + lockedImage?: string; timestamp?: number; } diff --git a/react-common/components/profile/Badge.tsx b/react-common/components/profile/Badge.tsx index e12a62e3c460..3d5db7b3902c 100644 --- a/react-common/components/profile/Badge.tsx +++ b/react-common/components/profile/Badge.tsx @@ -15,16 +15,18 @@ export const Badge = (props: BadgeProps) => { onClick(badge); }) + const image = (disabled && badge.lockedImage) || badge.image; + const alt = disabled ? pxt.U.lf("Locked '{0}' badge", badge.title) : badge.title; + return ( -
{isNew &&
{pxt.U.lf("New!")}
} - {badge.title} - {disabled && } + {alt}
); } diff --git a/react-common/components/profile/BadgeInfo.tsx b/react-common/components/profile/BadgeInfo.tsx index 3ef9f84a6bc1..3d7d3a61e955 100644 --- a/react-common/components/profile/BadgeInfo.tsx +++ b/react-common/components/profile/BadgeInfo.tsx @@ -42,7 +42,7 @@ export const badgeDescription = (badge: pxt.auth.Badge) => { case "skillmap-completion": return {jsxLF( lf("Completing {0}"), - {pxt.U.rlf(badge.title)} + {pxt.U.rlf(badge.title)} )} } } diff --git a/react-common/styles/profile/profile.css b/react-common/styles/profile/profile.css index 6eba42633832..21cc30681dc3 100644 --- a/react-common/styles/profile/profile.css +++ b/react-common/styles/profile/profile.css @@ -178,11 +178,6 @@ } -.profile-badge.disabled img { - filter: grayscale(1); - opacity: 0.5; -} - .profile-badge.disabled i.ui.icon { line-height: 1; vertical-align: middle; @@ -280,6 +275,7 @@ margin-right: 0; text-align: center; color: var(--header-text-color); + overflow: hidden; } .profile-initials-portrait { diff --git a/skillmap/.eslintrc.js b/skillmap/.eslintrc.js index 552f2a5d4954..6dc8a36b17d5 100644 --- a/skillmap/.eslintrc.js +++ b/skillmap/.eslintrc.js @@ -2,4 +2,5 @@ module.exports = { "parserOptions": { "project": "skillmap/tsconfig.json", }, + "ignorePatterns": ["tests/**/*.spec.ts", "public/**/*", "build/**/*"] } \ No newline at end of file diff --git a/skillmap/src/App.tsx b/skillmap/src/App.tsx index d39ee6efc5bc..31e9c9da5e1e 100644 --- a/skillmap/src/App.tsx +++ b/skillmap/src/App.tsx @@ -70,7 +70,7 @@ interface AppState { error?: string; cloudSyncCheckHasFinished: boolean; badgeSyncLock: boolean; - syncingLocalState?: boolean; + showingSyncLoader?: boolean; } class AppImpl extends React.Component { @@ -235,9 +235,8 @@ class AppImpl extends React.Component { protected async cloudSyncCheckAsync() { const res = await this.ready(); if (!await authClient.loggedInAsync()) { - this.setState({cloudSyncCheckHasFinished: true, syncingLocalState: false}); + this.setState({cloudSyncCheckHasFinished: true}); } else { - this.setState({syncingLocalState: true}); const doCloudSyncCheckAsync = async () => { const state = store.getState(); const localUser = await getLocalUserStateAsync(); @@ -315,14 +314,14 @@ class AppImpl extends React.Component { action: "requestprojectcloudstatus", headerIds: getFlattenedHeaderIds(currentUser, state.pageSourceUrl) } as pxt.editor.EditorMessageRequestProjectCloudStatus); - this.setState({cloudSyncCheckHasFinished: true, syncingLocalState: false}); + this.setState({ cloudSyncCheckHasFinished: true, showingSyncLoader: false }); } // Timeout if cloud sync check doesn't complete in a reasonable timeframe. const TIMEOUT_MS = 10 * 1000; await Promise.race([ pxt.U.delay(TIMEOUT_MS).then(() => { if (!this.state.cloudSyncCheckHasFinished) - this.setState({cloudSyncCheckHasFinished: true, syncingLocalState: false}); + this.setState({ cloudSyncCheckHasFinished: true, showingSyncLoader: false }); }), doCloudSyncCheckAsync()]); } @@ -343,6 +342,10 @@ class AppImpl extends React.Component { await this.initLocalizationAsync(); await this.parseHashAsync(); this.readyPromise.setAppMounted(); + + if (await authClient.loggedInAsync() && !this.state.cloudSyncCheckHasFinished) { + this.setState({ showingSyncLoader: true }); + } } componentWillUnmount() { @@ -354,11 +357,11 @@ class AppImpl extends React.Component { render() { const { skillMaps, activityOpen, backgroundImageUrl, theme } = this.props; - const { error, syncingLocalState } = this.state; + const { error, showingSyncLoader } = this.state; const maps = Object.keys(skillMaps).map((id: string) => skillMaps[id]); return (
- {syncingLocalState &&
+ {showingSyncLoader &&
{lf("MakeCode
{lf("Saving to cloud...")}
} @@ -433,11 +436,6 @@ class AppImpl extends React.Component { } } - if ((!this.props.signedIn || (this.props.signedIn && this.state.cloudSyncCheckHasFinished)) - && this.state.syncingLocalState) { - this.setState({ syncingLocalState: false }); - } - await this.syncBadgesAsync(); } diff --git a/skillmap/src/components/AppModal.tsx b/skillmap/src/components/AppModal.tsx index 3096c2aa172b..f2bff59a5a40 100644 --- a/skillmap/src/components/AppModal.tsx +++ b/skillmap/src/components/AppModal.tsx @@ -498,6 +498,11 @@ export class AppModalImpl extends React.Component return {lf("Use the button below to get your completion certificate!")} + {reward.previewUrl && +
+ {lf("certificate +
+ } } @@ -523,7 +528,7 @@ export class AppModalImpl extends React.Component if (signedIn) { message = jsxLF( lf("You’ve received the {0} Badge! Find it in the badges section of your {1}."), - {pxt.U.rlf(skillMap!.displayName)}, + {pxt.U.rlf(badge!.title)}, {lf("User Profile")} ); buttons.push( @@ -570,10 +575,10 @@ export class AppModalImpl extends React.Component return + {message}
- {message} } } diff --git a/skillmap/src/lib/skillMap.d.ts b/skillmap/src/lib/skillMap.d.ts index e512d429c2ec..a7c2e4310a60 100644 --- a/skillmap/src/lib/skillMap.d.ts +++ b/skillmap/src/lib/skillMap.d.ts @@ -73,11 +73,13 @@ type MapReward = MapRewardCertificate | MapCompletionBadge; interface MapRewardCertificate { type: "certificate"; url: string; + previewUrl?: string; } interface MapCompletionBadge { type: "completion-badge"; imageUrl: string; + displayName?: string; } interface MapRewardNode extends BaseNode { diff --git a/skillmap/src/lib/skillMapParser.ts b/skillmap/src/lib/skillMapParser.ts index 6ad827dc75ae..c3934002d0f5 100644 --- a/skillmap/src/lib/skillMapParser.ts +++ b/skillmap/src/lib/skillMapParser.ts @@ -3,11 +3,16 @@ const testMap = `` -interface MarkdownSection { +export interface MarkdownSection { headerKind: "single" | "double" | "triple"; header: string; attributes: { [index: string]: string }; - listAttributes?: { [index: string]: string[] }; + listAttributes?: { [index: string]: MarkdownList }; +} + +export interface MarkdownList { + key: string; + items: (string | MarkdownList)[]; } export function test() { @@ -41,7 +46,7 @@ export function parseSkillMap(text: string): { maps: SkillMap[], metadata?: Page return { maps: parsed, metadata }; } -function getSectionsFromText(text: string) { +export function getSectionsFromText(text: string) { const lines = text.split("\n"); let sections: MarkdownSection[] = []; @@ -49,6 +54,9 @@ function getSectionsFromText(text: string) { let currentKey: string | null = null; let currentValue: string | null = null; + let listStack: MarkdownList[] = []; + + let currentIndent = 0; for (const line of lines) { if (!line.trim()) { @@ -77,26 +85,60 @@ function getSectionsFromText(text: string) { } if (currentSection) { - const keyMatch = /^[*-]\s+(?:([^:]+):)?(.*)$/.exec(line); - const subkeyMatch = /^ {4}([*-])\s+(.*)$/.exec(line); + const indent = countIndent(line); + const trimmedLine = line.trim(); + + const keyMatch = /^[*-]\s+(?:([^:]+):)?(.*)$/.exec(trimmedLine); + if (!keyMatch) continue; + + // We ignore indent changes of 1 space to make the list authoring a little + // bit friendlier. Likewise, indents can be any length greater than 1 space + if (Math.abs(indent - currentIndent) > 1 && currentKey) { + if (indent > currentIndent) { + const newList = { + key: currentKey, + items: [] + }; + + if (listStack.length) { + listStack[listStack.length - 1].items.push(newList); + } + else { + if (!currentSection.listAttributes) currentSection.listAttributes = {}; + currentSection.listAttributes[currentKey] = newList; + } + currentKey = null; + listStack.push(newList); + } + else { + const prev = listStack.pop(); + + if (currentKey && currentValue) { + prev?.items.push((currentKey + ":" + currentValue).trim()) + currentValue = null; + } + } + + currentIndent = indent; + } if (keyMatch) { if (keyMatch[1]) { if (currentKey && currentValue) { - currentSection.attributes[currentKey] = currentValue.trim(); + if (listStack.length) { + listStack[listStack.length - 1].items.push((currentKey + ":" + currentValue).trim()); + } + else { + currentSection.attributes[currentKey] = currentValue.trim(); + } } + currentKey = keyMatch[1].toLowerCase(); currentValue = keyMatch[2]; } else if (currentKey) { currentValue += keyMatch[2]; } - } else if (subkeyMatch && currentKey) { - if (!currentSection.listAttributes) currentSection.listAttributes = {}; - if (!currentSection.listAttributes[currentKey]) currentSection.listAttributes[currentKey] = []; - if (subkeyMatch[2]) { - currentSection.listAttributes[currentKey].push(subkeyMatch[2].trim()); - } } } } @@ -108,10 +150,17 @@ function getSectionsFromText(text: string) { function pushSection() { if (currentSection) { if (currentKey && currentValue) { - currentSection.attributes[currentKey] = currentValue.trim(); + if (listStack.length) { + listStack[listStack.length - 1].items.push((currentKey + ":" + currentValue).trim()); + } + else { + currentSection.attributes[currentKey] = currentValue.trim(); + } } sections.push(currentSection); } + + listStack = []; } } @@ -276,7 +325,7 @@ function inflateMapReward(section: MarkdownSection, base: Partial if (section.listAttributes?.["actions"]) { const parsedActions: MapCompletionAction[] = []; - const actions = section.listAttributes["actions"]; + const actions = section.listAttributes["actions"].items.filter(a => typeof a === "string") as string[]; for (const action of actions) { let [kind, ...rest] = action.split(":"); const valueMatch = /\s*\[\s*(.*)\s*\](?:\(([^\s]+)\))?/gi.exec(rest.join(":")); @@ -317,22 +366,58 @@ function inflateMapReward(section: MarkdownSection, base: Partial if (section.listAttributes?.["rewards"]) { const parsedRewards: MapReward[] = []; const rewards = section.listAttributes["rewards"]; - for (const reward of rewards) { - let [kind, ...value] = reward.split(":"); - - switch (kind) { - case "certificate": - parsedRewards.push({ + for (const reward of rewards.items) { + if (typeof reward === "string") { + let [kind, ...value] = reward.split(":"); + + switch (kind) { + case "certificate": + parsedRewards.push({ + type: "certificate", + url: value.join(":").trim() + }); + break; + case "completion-badge": + parsedRewards.push({ + type: "completion-badge", + imageUrl: value.join(":").trim() + }); + break; + } + } + else { + if (reward.key === "certificate") { + const props = reward.items.filter(i => typeof i === "string") as string[] + const cert: Partial = { type: "certificate", - url: value.join(":").trim() - }); - break; - case "completion-badge": - parsedRewards.push({ + }; + + for (const prop of props) { + let [kind, ...value] = prop.split(":"); + + if (kind === "url") cert.url = value.join(":").trim(); + if (kind === "previewurl" || kind === "preview") cert.previewUrl = value.join(":").trim(); + } + + if (!cert.url) error(`Certificate in activity ${section.header} is missing url attribute`); + parsedRewards.push(cert as MapRewardCertificate); + } + else if (reward.key === "completion-badge") { + const props = reward.items.filter(i => typeof i === "string") as string[] + const badge: Partial = { type: "completion-badge", - imageUrl: value.join(":").trim() - }); - break; + }; + + for (const prop of props) { + let [kind, ...value] = prop.split(":"); + + if (kind === "imageurl" || kind === "image") badge.imageUrl = value.join(":").trim(); + if (kind === "displayname" || kind === "name") badge.displayName = value.join(":").trim(); + } + + if (!badge.imageUrl) error(`completion-badge in activity ${section.header} is missing imageurl attribute`); + parsedRewards.push(badge as MapCompletionBadge); + } } } @@ -500,5 +585,16 @@ function error(message: string): never { throw(message); } +// Handles tabs and spaces, but a mix of them might end up with strange results. Not much +// we can do about that so just treat 1 tab as 4 spaces +function countIndent(line: string) { + let indent = 0; + for (let i = 0; i < line.length; i++) { + if (line.charAt(i) === " ") indent++; + else if (line.charAt(i) === "\t") indent += 4; + else return indent; + } + return 0; +} diff --git a/skillmap/src/lib/skillMapUtils.ts b/skillmap/src/lib/skillMapUtils.ts index 52b72753948e..1e05fb9b0a7a 100644 --- a/skillmap/src/lib/skillMapUtils.ts +++ b/skillmap/src/lib/skillMapUtils.ts @@ -194,12 +194,13 @@ export function getCompletedBadges(user: UserState, pageSource: string, map: Ski } export function getCompletionBadge(pageSource: string, map: SkillMap, node: MapRewardNode): pxt.auth.Badge { + const badge = node.rewards.filter(b => b.type === "completion-badge")[0] as MapCompletionBadge; return { id: `skillmap-completion-${map.mapId}`, - image: (node.rewards.filter(b => b.type === "completion-badge")[0] as MapCompletionBadge)?.imageUrl, + image: badge?.imageUrl, sourceURL: pageSource, type: "skillmap-completion", - title: map.displayName + title: badge?.displayName || map.displayName }; } diff --git a/skillmap/src/styles/modal.css b/skillmap/src/styles/modal.css index e24d4c1845e2..a1b956cd2c68 100644 --- a/skillmap/src/styles/modal.css +++ b/skillmap/src/styles/modal.css @@ -306,6 +306,23 @@ max-width: 100%; } +/* CERTIFICATE MODAL */ +.certificate-reward { + display: flex; + flex-direction: column; + align-items: center; + margin-top: 1rem +} + +.certificate-reward img { + max-width: 50%; +} + +/* BADGE MODAL */ +.badge-modal-image { + margin-top: 1rem; +} + /* COMPLETION MODAL */ .completion-reward { display: flex; diff --git a/skillmap/tests/skillmapParser.spec.ts b/skillmap/tests/skillmapParser.spec.ts new file mode 100644 index 000000000000..265831ad679a --- /dev/null +++ b/skillmap/tests/skillmapParser.spec.ts @@ -0,0 +1,121 @@ +import { getSectionsFromText, MarkdownList, MarkdownSection } from "../src/lib/skillMapParser"; +import chai = require("chai"); + + +describe("skillmap parser", () => { + it("should parse nested lists", () => { + const test = ` +### forest-cert +* name: Congrats! +* url: /static/skillmap/certificates/forest-cert.pdf +* rewards: + * certificate: + * url: /static/skillmap/certificates/forest-cert.pdf + * previewUrl: /static/skillmap/certificates/forest-cert.png + * completion-badge: /static/badges/badge-forest.png +* actions: + * map: [Try the Jungle Monkey Skillmap](/skillmap/jungle) + * map: [Try the Space Explorer Skillmap](/skillmap/space) + * editor: [Edit Your Project with a Full Toolbox] (/) + `; + + const expected: MarkdownSection = { + headerKind: "triple", + header: "forest-cert", + attributes: { + "name": "Congrats!", + "url": "/static/skillmap/certificates/forest-cert.pdf" + }, + listAttributes: { + "rewards": { + key: "rewards", + items: [ + { + key: "certificate", + items: [ + "url: /static/skillmap/certificates/forest-cert.pdf", + "previewurl: /static/skillmap/certificates/forest-cert.png" + ] + }, + "completion-badge: /static/badges/badge-forest.png" + ] + }, + "actions": { + key: "actions", + items: [ + "map: [Try the Jungle Monkey Skillmap](/skillmap/jungle)", + "map: [Try the Space Explorer Skillmap](/skillmap/space)", + "editor: [Edit Your Project with a Full Toolbox] (/)", + ] + } + } + } + + + const result = getSectionsFromText(test); + chai.assert(result.length === 1, "Wrong number of sections"); + chai.expect(result[0]).deep.equals(expected); + }); + + it("should handle multiple sections", () => { + const test = ` +### section-1 +### section-2 +* hello: goodbye +`; + + const expected: MarkdownSection[] = [ + { + headerKind: "triple", + header: "section-1", + attributes: { }, + }, + { + headerKind: "triple", + header: "section-2", + attributes: { + "hello": "goodbye", + }, + } + ] + + + const result = getSectionsFromText(test); + chai.expect(result).deep.equals(expected); + }); + + it("should ignore single space indent errors", () => { + const test = ` +### section-1 +* one: 1 + * two: 2 +* list: + * three: 3 + * four: 4 +`; + + const expected: MarkdownSection[] = [ + { + headerKind: "triple", + header: "section-1", + attributes: { + "one": "1", + "two": "2" + }, + listAttributes: { + "list": { + key: "list", + items: [ + "three: 3", + "four: 4" + ] + } + } + } + ] + + + const result = getSectionsFromText(test); + chai.expect(result).deep.equals(expected); + }); +}) \ No newline at end of file diff --git a/skillmap/tests/tsconfig.json b/skillmap/tests/tsconfig.json new file mode 100644 index 000000000000..90c234401346 --- /dev/null +++ b/skillmap/tests/tsconfig.json @@ -0,0 +1,28 @@ +{ + "compilerOptions": { + "target": "ES2017", + "lib": [ + "dom", + "dom.iterable", + "ES2017", + "ES2018.Promise" + ], + "allowJs": true, + "skipLibCheck": true, + "esModuleInterop": true, + "allowSyntheticDefaultImports": true, + "strict": true, + "forceConsistentCasingInFileNames": true, + "noFallthroughCasesInSwitch": true, + "moduleResolution": "node", + "resolveJsonModule": true, + "isolatedModules": true, + "noEmit": true, + "jsx": "react", + "module": "commonjs" + }, + "include": [ + ".", + "../src/lib/*" + ] + } diff --git a/theme/pxt.less b/theme/pxt.less index 4e082e226198..5c07edd6aad5 100644 --- a/theme/pxt.less +++ b/theme/pxt.less @@ -38,7 +38,6 @@ @import 'webusb'; @import 'image-editor/imageEditor'; -@import (css) '../react-common/styles/profile/profile.css'; /* Reference import */ @import (reference) "semantic.less"; diff --git a/webapp/src/components/ImageEditor/ImageCanvas.tsx b/webapp/src/components/ImageEditor/ImageCanvas.tsx index 802726f36d06..5184dabf7542 100644 --- a/webapp/src/components/ImageEditor/ImageCanvas.tsx +++ b/webapp/src/components/ImageEditor/ImageCanvas.tsx @@ -759,6 +759,10 @@ export class ImageCanvasImpl extends React.Component imple } } + else if (isDrawingWalls) { + context.fillStyle = this.props.colors[color] + context.fillRect(left, top, 1, 1); + } else { context.fillStyle = this.props.colors[color] context.fillRect(left * this.cellWidth, top * this.cellWidth, width * this.cellWidth, width * this.cellWidth);