From 068bdf8af06ab44c078e882c715480479ff4bcea Mon Sep 17 00:00:00 2001 From: Richard Knoll Date: Wed, 10 Nov 2021 16:08:07 -0800 Subject: [PATCH 1/3] Adding certificate preview to the rewards modal and skillmap tests --- gulpfile.js | 8 +- react-common/components/profile/BadgeInfo.tsx | 2 +- skillmap/src/components/AppModal.tsx | 7 +- skillmap/src/lib/skillMap.d.ts | 1 + skillmap/src/lib/skillMapParser.ts | 139 ++++++++++++++---- skillmap/src/styles/modal.css | 17 +++ skillmap/tests/skillmapParser.spec.ts | 121 +++++++++++++++ skillmap/tests/tsconfig.json | 28 ++++ 8 files changed, 291 insertions(+), 32 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/react-common/components/profile/BadgeInfo.tsx b/react-common/components/profile/BadgeInfo.tsx index b2419c5ffa7b..a41315acb094 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 the {0}"), - {pxt.U.rlf(badge.title)} + {pxt.U.rlf(badge.title)} )} } } diff --git a/skillmap/src/components/AppModal.tsx b/skillmap/src/components/AppModal.tsx index 3096c2aa172b..708cca01a712 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 +
+ } } @@ -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..91f219c46c65 100644 --- a/skillmap/src/lib/skillMap.d.ts +++ b/skillmap/src/lib/skillMap.d.ts @@ -73,6 +73,7 @@ type MapReward = MapRewardCertificate | MapCompletionBadge; interface MapRewardCertificate { type: "certificate"; url: string; + previewUrl?: string; } interface MapCompletionBadge { diff --git a/skillmap/src/lib/skillMapParser.ts b/skillmap/src/lib/skillMapParser.ts index 6ad827dc75ae..2fe6d14cd993 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,43 @@ 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: MapRewardCertificate = { type: "certificate", - url: value.join(":").trim() - }); - break; - case "completion-badge": - parsedRewards.push({ - type: "completion-badge", - imageUrl: value.join(":").trim() - }); - break; + url: "" + }; + + 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); + } } } @@ -500,5 +570,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/styles/modal.css b/skillmap/src/styles/modal.css index a70913480d36..f9ef68a90a05 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/*" + ] + } From b746652c9a8b38f9e0df5863af5c91eafa04c7a0 Mon Sep 17 00:00:00 2001 From: Richard Knoll Date: Wed, 10 Nov 2021 17:13:49 -0800 Subject: [PATCH 2/3] Fix lint and remove extra css --- skillmap/.eslintrc.js | 1 + theme/pxt.less | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) 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/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"; From 9b5be1f5b84432e7d645ac12c8ad660aa6696cad Mon Sep 17 00:00:00 2001 From: Richard Knoll Date: Thu, 11 Nov 2021 10:27:30 -0800 Subject: [PATCH 3/3] Add the ability to specify a name for the badge --- react-common/styles/profile/profile.css | 1 + skillmap/src/components/AppModal.tsx | 2 +- skillmap/src/lib/skillMap.d.ts | 1 + skillmap/src/lib/skillMapParser.ts | 21 ++++++++++++++++++--- skillmap/src/lib/skillMapUtils.ts | 5 +++-- 5 files changed, 24 insertions(+), 6 deletions(-) diff --git a/react-common/styles/profile/profile.css b/react-common/styles/profile/profile.css index 892d9d54d19b..553632ab3a84 100644 --- a/react-common/styles/profile/profile.css +++ b/react-common/styles/profile/profile.css @@ -274,6 +274,7 @@ margin-right: 0; text-align: center; color: var(--header-text-color); + overflow: hidden; } .profile-initials-portrait { diff --git a/skillmap/src/components/AppModal.tsx b/skillmap/src/components/AppModal.tsx index 708cca01a712..f2bff59a5a40 100644 --- a/skillmap/src/components/AppModal.tsx +++ b/skillmap/src/components/AppModal.tsx @@ -528,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( diff --git a/skillmap/src/lib/skillMap.d.ts b/skillmap/src/lib/skillMap.d.ts index 91f219c46c65..a7c2e4310a60 100644 --- a/skillmap/src/lib/skillMap.d.ts +++ b/skillmap/src/lib/skillMap.d.ts @@ -79,6 +79,7 @@ interface MapRewardCertificate { 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 2fe6d14cd993..c3934002d0f5 100644 --- a/skillmap/src/lib/skillMapParser.ts +++ b/skillmap/src/lib/skillMapParser.ts @@ -388,9 +388,8 @@ function inflateMapReward(section: MarkdownSection, base: Partial else { if (reward.key === "certificate") { const props = reward.items.filter(i => typeof i === "string") as string[] - const cert: MapRewardCertificate = { + const cert: Partial = { type: "certificate", - url: "" }; for (const prop of props) { @@ -401,7 +400,23 @@ function inflateMapReward(section: MarkdownSection, base: Partial } if (!cert.url) error(`Certificate in activity ${section.header} is missing url attribute`); - parsedRewards.push(cert); + 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", + }; + + 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); } } } diff --git a/skillmap/src/lib/skillMapUtils.ts b/skillmap/src/lib/skillMapUtils.ts index bc09e5249367..4fddb3530d00 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: pxt.U.lf("{0} Skillmap", map.displayName) + title: badge?.displayName || map.displayName }; }