Conversation
src/dialogs/RootDialog.ts
Outdated
| import { HelloDialog } from "./examples/basic/HelloDialog"; | ||
| import { HelpDialog } from "./examples/basic/HelpDialog"; | ||
| import { HeroCardDialog } from "./examples/basic/HeroCardDialog"; | ||
| import { PopupSignInDailog } from "./examples/basic/PopupSignInDailog"; |
|
|
||
| export class PopupSignInDailog extends TriggerActionDialog { | ||
|
|
||
| private static async step1(session: builder.Session, args?: any | builder.IDialogResult<any>, next?: (args?: builder.IDialogResult<any>) => void): Promise<void> { |
There was a problem hiding this comment.
there IS only one step, right? so maybe a more descriptive name than "step1"?
public/botViews/popUpSignin.html
Outdated
| <head> | ||
| </head> | ||
| <!--Purpose of this login html page is to show fake authentication when user clicks on SignIn Action --> | ||
| <!--This login page will do nothing, except when user will click on Login page it will post back to Main controller --> |
There was a problem hiding this comment.
"This simulates a login page to show what happens when the user clicks on a button with the signin action.
When the user clicks on the Login button, it calls notifySuccess() to fake a successful login and trigger a new message to the bot."
| if (session) { | ||
| session.clearDialogStack(); | ||
| session.send("Authentication Successful!!"); | ||
|
|
src/Bot.ts
Outdated
| } | ||
|
|
||
| private SigninHandler(bot: builder.UniversalBot): (event: builder.IEvent, query: teams.ISigninStateVerificationQuery, callback: (err: Error, body: any, status?: number) => void) => void { | ||
| return async function ( |
There was a problem hiding this comment.
We should change this getXXXHandler() pattern as it's not clear how to access private members in the handler (this confused Shoeb who was working on the Trello CE).
Better to do something like this:
this._connector.onSigninStateVerification((event, query, callback) => { this.verifySigninState(event, query, callback); });
and put your logic in the private member function verifySigninState().
| * main purpose of this class is to Display the PopUp SignIn Card | ||
| */ | ||
|
|
||
| // let input = ""; |
There was a problem hiding this comment.
Scrub for random commented out code like this, and delete.
| let buttons = new Array<builder.CardAction>(); | ||
| /** | ||
| * This is PopUp SignIn Dialog Class. | ||
| * main purpose of this class is to Display the PopUp SignIn Card |
There was a problem hiding this comment.
- Should this be here, or up on line 6, as a comment to the class itself?
- I'd say something like this instead "Demonstrates using a signin action to show a login page in a popup"
|
|
||
| buttons.push(new builder.CardAction(session) | ||
| .type("signin") | ||
| .title("Sign In") |
There was a problem hiding this comment.
@jotrick is there a principle we follow in this project for when a string goes in index.json vs. inline in the ts file?
There was a problem hiding this comment.
@jotrick has used index.json for string constant, I have used the same now.
| cards.push(newCard); | ||
|
|
||
| session.send(new builder.Message(session) | ||
| .attachments(cards)); |
There was a problem hiding this comment.
for a single attachment you can use addAttachment, and avoid constructing the "cards" array.
|
|
||
| export class PopupSignInDialog extends TriggerActionDialog { | ||
|
|
||
| private static async PopUpSignIn(session: builder.Session, args?: any | builder.IDialogResult<any>, next?: (args?: builder.IDialogResult<any>) => void): Promise<void> { |
There was a problem hiding this comment.
Nit: TS convention is lowercase method names.
| .value(popUpUrl), | ||
| ); | ||
|
|
||
| let newCard = new builder.HeroCard(session) |
There was a problem hiding this comment.
"newCard" feels too generic, "signinCard" might be better.
|
|
||
| let newCard = new builder.HeroCard(session) | ||
| .title("Please click below for Popup Sign-In experience") | ||
| .buttons(buttons); |
There was a problem hiding this comment.
You can inline this array, like
.buttons([
new builder.CardAction(session)...
]);
unless you intentionally did it this way to be extra-clear?
Not sure if this project has conventions one way or another @jotrick.
| // Demonstrates using a signin action to show a login page in a popup | ||
| export class PopupSignInDialog extends TriggerActionDialog { | ||
|
|
||
| private static async popupsignin(session: builder.Session, args?: any | builder.IDialogResult<any>, next?: (args?: builder.IDialogResult<any>) => void): Promise<void> { |
There was a problem hiding this comment.
NIT: Change name of function to sendPopupSigninCard
public/botViews/popUpSignin.html
Outdated
| microsoftTeams.initialize(); | ||
| function submit() | ||
| { | ||
| microsoftTeams.authentication.notifySuccess('PopUpSignInAuthenticationSuccess'); |
There was a problem hiding this comment.
For this example, let's pass the user's input username through the notifySuccess call so that the verifyState handler in the bot can present that field in the outgoing message.
src/Bot.ts
Outdated
| if (session) | ||
| { | ||
| session.clearDialogStack(); | ||
| session.send(Strings.popupsignin_successful); |
There was a problem hiding this comment.
For this example, let's pass the user's input username through the notifySuccess call so that the verifyState handler in the bot can present that field in the outgoing message - so here, the handler should grab the passed in value and send that out in the message.
| "o365connectorcard_action_response": "<h2>Thanks, %s!</h2><br/><h3>Your input action ID:</h3><br/><pre>%s</pre><br/><h3>Your input body:</h3><br/><pre>%s</pre>", | ||
| "end_of_example_string_responses": "******************************* EVERYTHING ABOVE HERE IS FOR A TEMPLATE EXAMPLE DIALOG *******************************" | ||
| "end_of_example_string_responses": "******************************* EVERYTHING ABOVE HERE IS FOR A TEMPLATE EXAMPLE DIALOG *******************************", | ||
| "popupsignin_card_title":"Please click below for Popup Sign-In experience", |
There was a problem hiding this comment.
Move these three strings above the "****************" line so that the user knows they are strings used by examples.
src/utils/DialogMatches.ts
Outdated
| HelloDialogMatch2: /hi/i, | ||
| HelpDialogMatch: /help/i, | ||
| HeroCardDialogMatch: /hero card/i, | ||
| PopUpSignInDialogMatch: /sign in/i, |
There was a problem hiding this comment.
Let's make the space in the regex optional so if I typed "sign in" or "signin", it would trigger the dialog.
| { | ||
| elem.value = Math.floor((Math.random() * 10000) + 1); | ||
| } | ||
|
|
| { | ||
| magicNumber = document.getElementById('txtMagicNumber').value; | ||
| } | ||
|
|
5 participants
@msftdherron @jotrick @goelashish7 @jialic @aosolis @MattSFT
Added Sample for PopUpSignIn in NodeJs App.