-
Notifications
You must be signed in to change notification settings - Fork 66
Add task module samples #65
base: master
Are you sure you want to change the base?
Conversation
|
||
switch (invokeType) { | ||
case "task/fetch": | ||
let taskModule = payload.data.taskModule.toLowerCase(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add comments here (in each "case" block") explaining conceptually what these invokes mean and how we respond to them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also mention which properties are standard from the framework, and which ones you added. For example, "taskModule" is something that you added in the "data" field of the adaptive card action. I'd like this to be clear because it's hard, when reading a sample, to separate what comes from the framework and what the developer added herself.
@@ -0,0 +1,111 @@ | |||
import * as builder from "botbuilder"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Name this file TaskModuleAdaptiveCardDialog.ts, and move it to the teams folder, not basic.
horizontalAlignment: "left", | ||
wrap: false, | ||
maxLines: 0, | ||
speak: "<s>Adaptive card!</s>", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's remove the speak elements, as it might imply that Teams supports speech.
actions: [ | ||
{ | ||
type: "Action.Submit", | ||
title: "Invoke Task Module(Adaptive Card) -single step", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Be consistent with capitalization, here we have "single step", below we capitalize the first character.
@@ -72,6 +73,11 @@ export class DefaultTab { | |||
document.getElementById('contextOutput').innerHTML = JSON.stringify(context); | |||
}); | |||
} | |||
|
|||
function openTaskModule(){ | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: remove blank line
taskInfo.height = 510; | ||
taskInfo.width = 510; | ||
let submitHandler = (err, result) => { | ||
document.getElementById('taskModuleResult').innerHTML="you enterd the command: " + result.commandToBot; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: enterd
taskInfo.height = 510; | ||
taskInfo.width = 510; | ||
let submitHandler = (err, result) => { | ||
document.getElementById('taskModuleResult').innerHTML="you enterd the command: " + result.commandToBot; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, don't use the innerHTML property, as that can make the page susceptible to XSS. Use JQuery's text().
}; | ||
} | ||
|
||
document.addEventListener("DOMContentLoaded", function(){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All this can be in the ready() handler (line 60).
public static getRequestHandler(): express.RequestHandler { | ||
return async function (req: any, res: any, next: any): Promise<void> { | ||
try{ | ||
let htmlPage = `<!DOCTYPE html> <head> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you change this to use a handlebars template instead. It's easier to read.
No description provided.