Added the ability to open a bot from just a URL #1232
Conversation
8884635 to
9e0daa7
Compare
Pull Request Test Coverage Report for Build 1799
💛 - Coveralls |
packages/app/client/src/ui/editor/recentBotsList/recentBotsList.spec.tsx
Show resolved
Hide resolved
packages/app/client/src/ui/editor/recentBotsList/recentBotsListContainer.ts
Show resolved
Hide resolved
| @@ -1,11 +1,10 @@ | |||
| import { SharedConstants } from '@bfemulator/app-shared'; | |||
| private get startSection(): JSX.Element { | ||
| const { onNewBotClick, onOpenBotClick } = this.props; | ||
| const { onNewBotClick } = this.props; |
There was a problem hiding this comment.
is there a reason onOpenBotClick was removed from the destructuring assignment, but it's still used with this. in front of it on line 91? If so, why is onNewBotClick destructured?
In my opinion, you should either keep both onNewBotClick and onOpenBotClick, or neither.
There was a problem hiding this comment.
One function is on props while the other is on this. I'll remove the destructuring assignment.
packages/app/client/src/ui/shell/explorer/botNotOpenExplorer/botNotOpenExplorer.spec.tsx
Show resolved
Hide resolved
| return ''; // Allow empty endpoints | ||
| } | ||
| if (/(http)(s)?(:\/\/)/.test(endpoint)) { | ||
| return endpoint.endsWith('/api/messages') ? '' : `Please include route if necessary: "/api/messages"`; |
There was a problem hiding this comment.
Why is it a requirement for a bot to have an /api/messages endpoint?
| title="Open a Bot"> | ||
| <p> | ||
| { `You can open a bot using just a URL, choose from a list of recently opened bots or ` } | ||
| <a href="javascript:void(0);" onClick={ this.onCreateNewBotClick }>create a new bot configuration</a> |
There was a problem hiding this comment.
This should be a button.
| ); | ||
| } | ||
|
|
||
| private onBotClick = (event: React.MouseEvent<HTMLButtonElement>) => { |
There was a problem hiding this comment.
IMO, these handlers are better as closures.
onClick = (bot) => () => this.props.onBotSelected(bot)| @@ -1,11 +1,10 @@ | |||
| import { SharedConstants } from '@bfemulator/app-shared'; | |||
There was a problem hiding this comment.
This didn't get a license
| onDeleteBotClick?: (_e: any, path: string) => void; | ||
| showContextMenuForBot?: (bot: BotInfo) => void; | ||
| recentBots?: BotInfo[]; | ||
| showOpenBotDialog: () => Promise<any>; |
There was a problem hiding this comment.
Can we avoid any? It is not very helpful.
| signInWithAzure?: () => void; | ||
| signOutWithAzure?: () => void; | ||
| switchToBot?: (path: string) => Promise<any>; |
There was a problem hiding this comment.
Ditto about avoiding any
| const filename = await this.browseForBotFile(); | ||
| if (!filename) { | ||
| filename = await this.browseForBotFile(); | ||
| } |
There was a problem hiding this comment.
Re-assigning function parameters is an anti-pattern: https://eslint.org/docs/rules/no-param-reassign#disallow-reassignment-of-function-parameters-no-param-reassign
| @@ -36,34 +36,42 @@ import * as React from 'react'; | |||
| import * as styles from './botNotOpenExplorer.scss'; | |||
|
|
|||
| export interface BotNotOpenExplorerProps { | |||
| onOpenBotClick: () => any; | |||
| hasChat?: boolean; | |||
| showOpenBotDialog: () => Promise<any>; | |||
|
|
||
| export interface RecentBotsListProps { | ||
| onBotSelected?: (bot: BotInfo) => void; | ||
| onDeleteBotClick?: (path: string) => Promise<any>; |
This PR Fixes #1209 by adding the ability to connect to a bot by just a URL
To Test
** Observe ** A new UI dialog is presented with the ability to type in a URL, browse for a bot file or choose from a list of previously opened bots.
** Observe ** A new chat tab is is opened with a connection to the specified url.