Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature: implement new ProjectTree design with popout menus #4361

Merged
merged 31 commits into from
Oct 20, 2020
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
7595021
Update en-US.json
beyackle Oct 5, 2020
78c3f7a
Merge branch 'main' of https://github.com/microsoft/BotFramework-Comp…
beyackle Oct 9, 2020
e0576b5
Merge branch 'main' of https://github.com/microsoft/BotFramework-Comp…
beyackle Oct 12, 2020
e6c088d
bring in stuff from the draft branch
beyackle Oct 13, 2020
49d7ce0
make deletion work
beyackle Oct 13, 2020
73b657e
Merge branch 'main' into beyackle/botProjectTreeOnly
beyackle Oct 13, 2020
9fa39e9
add error/warning icons
beyackle Oct 13, 2020
28c4baf
Merge branch 'main' into beyackle/botProjectTreeOnly
beyackle Oct 14, 2020
e49078c
Merge branch 'beyackle/botProjectTreeOnly' of https://github.com/micr…
beyackle Oct 14, 2020
bdb21db
Merge branch 'main' into beyackle/botProjectTreeOnly
beyackle Oct 14, 2020
28fcadf
read notification map for state
beyackle Oct 14, 2020
ca31c22
Merge branch 'beyackle/botProjectTreeOnly' of https://github.com/micr…
beyackle Oct 14, 2020
34d6415
fix type-checking and start on unit tests
beyackle Oct 14, 2020
9cee86c
add sampleDialog and fix more tests
beyackle Oct 14, 2020
ea7ced6
add showAll
beyackle Oct 15, 2020
7a4fa20
rename to onAllSelected because it's a callback
beyackle Oct 15, 2020
69ddaa3
update unit tests
beyackle Oct 15, 2020
fa557ef
Merge branch 'main' into beyackle/botProjectTreeOnly
beyackle Oct 15, 2020
ebf8450
fix onSelect handling in ProjectTree
beyackle Oct 15, 2020
ad66423
Update qna.test.tsx
beyackle Oct 15, 2020
48ab9bf
Merge branch 'main' into beyackle/botProjectTreeOnly
beyackle Oct 15, 2020
76bee68
Update design.test.tsx
beyackle Oct 15, 2020
b877aca
add unit test
beyackle Oct 15, 2020
762b2b5
Merge branch 'main' into beyackle/botProjectTreeOnly
beyackle Oct 16, 2020
463870a
Merge branch 'main' into beyackle/botProjectTreeOnly
beyackle Oct 16, 2020
014a094
Merge branch 'main' of https://github.com/microsoft/BotFramework-Comp…
beyackle Oct 16, 2020
89caba3
Merge branch 'main' into beyackle/botProjectTreeOnly
beyackle Oct 19, 2020
a02da9b
fixes from PR comments
beyackle Oct 19, 2020
0a20716
Merge branch 'main' of https://github.com/microsoft/BotFramework-Comp…
beyackle Oct 19, 2020
d00c42d
Merge branch 'main' into beyackle/botProjectTreeOnly
beyackle Oct 19, 2020
90c92d0
Merge branch 'main' into beyackle/botProjectTreeOnly
cwhitten Oct 20, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 16 additions & 16 deletions Composer/packages/client/__tests__/components/design.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,39 +3,39 @@

import * as React from 'react';
import { fireEvent } from '@botframework-composer/test-utils';
import { DialogInfo } from '@bfc/shared';

import { renderWithRecoil } from '../testUtils';
import { dialogs } from '../constants.json';
import { renderWithRecoil, SAMPLE_DIALOG } from '../testUtils';
import { ProjectTree } from '../../src/components/ProjectTree/ProjectTree';
import { TriggerCreationModal } from '../../src/components/ProjectTree/TriggerCreationModal';
import { CreateDialogModal } from '../../src/pages/design/createDialogModal';
import { dialogsState, currentProjectIdState, botProjectIdsState, schemasState } from '../../src/recoilModel';

jest.mock('@bfc/code-editor', () => {
return {
LuEditor: () => <div></div>,
};
});
const projectId = '1234a-324234';
const projectId = '12345.6789';
const dialogs = [SAMPLE_DIALOG];

const initRecoilState = ({ set }) => {
set(currentProjectIdState, projectId);
set(botProjectIdsState, [projectId]);
set(dialogsState(projectId), dialogs);
set(schemasState(projectId), { sdk: { content: {} } });
};

describe('<ProjectTree/>', () => {
it('should render the ProjectTree', async () => {
const dialogId = 'todobot';
const selected = 'triggers[0]';
const handleSelect = jest.fn(() => {});
const handleDeleteDialog = jest.fn(() => {});
const handleDeleteTrigger = jest.fn(() => {});
const { findByText } = renderWithRecoil(
<ProjectTree
dialogId={dialogId}
dialogs={(dialogs as unknown) as DialogInfo[]}
selected={selected}
onDeleteDialog={handleDeleteDialog}
onDeleteTrigger={handleDeleteTrigger}
onSelect={handleSelect}
/>

const { findByTestId } = renderWithRecoil(
<ProjectTree onDeleteDialog={handleDeleteDialog} onDeleteTrigger={handleDeleteTrigger} onSelect={handleSelect} />,
initRecoilState
);
const node = await findByText('addtodo');
const node = await findByTestId('EchoBot-1_Greeting');
fireEvent.click(node);
expect(handleSelect).toHaveBeenCalledTimes(1);
});
Expand Down
51 changes: 33 additions & 18 deletions Composer/packages/client/__tests__/components/projecttree.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,41 +4,56 @@
import * as React from 'react';
import { fireEvent } from '@botframework-composer/test-utils';

import { dialogs } from '../constants.json';
import { ProjectTree } from '../../src/components/ProjectTree/ProjectTree';
import { renderWithRecoil } from '../testUtils';
import { renderWithRecoil, SAMPLE_DIALOG } from '../testUtils';
import { dialogsState, currentProjectIdState, botProjectIdsState, schemasState } from '../../src/recoilModel';

const projectId = '12345.6789';
const dialogs = [SAMPLE_DIALOG];

const initRecoilState = ({ set }) => {
set(currentProjectIdState, projectId);
set(botProjectIdsState, [projectId]);
set(dialogsState(projectId), dialogs);
set(schemasState(projectId), { sdk: { content: {} } });
};

describe('<ProjectTree/>', () => {
it('should render the projecttree', async () => {
const { findByText } = renderWithRecoil(
<ProjectTree
dialogId="ToDoBot"
dialogs={dialogs as any}
selected=""
onDeleteDialog={() => {}}
onDeleteTrigger={() => {}}
onSelect={() => {}}
/>
<ProjectTree onDeleteDialog={() => {}} onDeleteTrigger={() => {}} onSelect={() => {}} />,
initRecoilState
);

await findByText('ToDoBot');
await findByText('EchoBot-1');
});

it('should handle project tree item click', async () => {
const mockFileSelect = jest.fn(() => null);
const component = renderWithRecoil(
<ProjectTree onDeleteDialog={() => {}} onDeleteTrigger={() => {}} onSelect={mockFileSelect} />,
initRecoilState
);

const node = await component.findByTestId('EchoBot-1_Greeting');
fireEvent.click(node);
expect(mockFileSelect).toHaveBeenCalledTimes(1);
});

it('fires the onSelectAll event', async () => {
const mockOnSelected = jest.fn();
const { findByText } = renderWithRecoil(
<ProjectTree
dialogId="ToDoBot"
dialogs={dialogs as any}
selected=""
onAllSelected={mockOnSelected}
onDeleteDialog={() => {}}
onDeleteTrigger={() => {}}
onSelect={mockFileSelect}
/>
onSelect={() => {}}
/>,
initRecoilState
);

const node = await findByText('addtodo');
const node = await findByText('All');
fireEvent.click(node);
expect(mockFileSelect).toHaveBeenCalledTimes(1);
expect(mockOnSelected).toHaveBeenCalledTimes(1);
});
});
1 change: 1 addition & 0 deletions Composer/packages/client/__tests__/testUtils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@
export * from './renderWithRecoilAndContext';
export * from './renderWithRecoil';
export * from './react-recoil-hooks-testing-library';
export * from './sampleDialog';
93 changes: 93 additions & 0 deletions Composer/packages/client/__tests__/testUtils/sampleDialog.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
// Copyright (c) Microsoft Corporation.
Copy link
Contributor

@srinaath srinaath Oct 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Maybe move this file inside __mocks folder rather than directly inside testUtils

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good idea.

// Licensed under the MIT License.

// This is a copy of the JSON that defines the EchoBot sample plus some
// additional dialogs and triggers, including a trigger with a syntax
// error for use with testing error messages.

export const SAMPLE_DIALOG = {
isRoot: true,
displayName: 'EchoBot-1',
id: 'echobot-1',
content: {
$kind: 'Microsoft.AdaptiveDialog',
$designer: { id: '433224', description: '', name: 'EchoBot-1' },
autoEndDialog: true,
defaultResultProperty: 'dialog.result',
triggers: [
{
$kind: 'Microsoft.OnUnknownIntent',
$designer: { id: '821845' },
actions: [
{ $kind: 'Microsoft.SendActivity', $designer: { id: '003038' }, activity: '${SendActivity_003038()}' },
],
},
{
$kind: 'Microsoft.OnConversationUpdateActivity',
$designer: { id: '376720' },
actions: [
{
$kind: 'Microsoft.Foreach',
$designer: { id: '518944', name: 'Loop: for each item' },
itemsProperty: 'turn.Activity.membersAdded',
actions: [
{
$kind: 'Microsoft.IfCondition',
$designer: { id: '641773', name: 'Branch: if/else' },
condition: 'string(dialog.foreach.value.id) != string(turn.Activity.Recipient.id)',
actions: [
{
$kind: 'Microsoft.SendActivity',
$designer: { id: '859266', name: 'Send a response' },
activity: '${SendActivity_Welcome()}',
},
],
},
],
},
],
},
{ $kind: 'Microsoft.OnError', $designer: { id: 'XVSGCI' } },
{
$kind: 'Microsoft.OnIntent',
$designer: { id: 'QIgTMy', name: 'more errors' },
intent: 'test',
actions: [{ $kind: 'Microsoft.SetProperty', $designer: { id: 'VyWC7G' }, value: '=[' }],
},
],
generator: 'echobot-1.lg',
$schema:
'https://raw.githubusercontent.com/microsoft/BotFramework-Composer/stable/Composer/packages/server/schemas/sdk.schema',
id: 'EchoBot-1',
recognizer: 'echobot-1.lu.qna',
},
diagnostics: [
{
message:
"must be an expression: syntax error at line 1:1 mismatched input '<EOF>' expecting {STRING_INTERPOLATION_START, '+', '-', '!', '(', '[', ']', '{', NUMBER, IDENTIFIER, STRING}",
source: 'echobot-1',
severity: 0,
path: 'echobot-1.triggers[3].actions[0]#Microsoft.SetProperty#value',
},
],
referredDialogs: [],
lgTemplates: [
{ name: 'SendActivity_003038', path: 'echobot-1.triggers[0].actions[0]' },
{ name: 'SendActivity_Welcome', path: 'echobot-1.triggers[1].actions[0].actions[0].actions[0]' },
],
referredLuIntents: [{ name: 'test', path: 'echobot-1.triggers[3]#Microsoft.OnIntent' }],
luFile: 'echobot-1',
qnaFile: 'echobot-1',
lgFile: 'echobot-1',
triggers: [
{ id: 'triggers[0]', displayName: '', type: 'Microsoft.OnUnknownIntent', isIntent: false },
{ id: 'triggers[1]', displayName: '', type: 'Microsoft.OnConversationUpdateActivity', isIntent: false },
{ id: 'triggers[2]', displayName: '', type: 'Microsoft.OnError', isIntent: false },
{ id: 'triggers[3]', displayName: 'more errors', type: 'Microsoft.OnIntent', isIntent: true },
],
intentTriggers: [
{ intent: 'test', dialogs: [] },
{ intent: 'test', dialogs: [] },
],
skills: [],
};
22 changes: 16 additions & 6 deletions Composer/packages/client/__tests__/utils/navigation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
} from './../../src/utils/navigation';

const projectId = '123a-sdf123';
const skillId = '98765.4321';

describe('getFocusPath', () => {
it('return focus path', () => {
Expand Down Expand Up @@ -94,13 +95,22 @@ describe('composer url util', () => {
});

it('convert path to url', () => {
const result1 = convertPathToUrl(projectId, 'main');
expect(result1).toEqual(`/bot/${projectId}/dialogs/main`);
const result2 = convertPathToUrl(projectId, 'main', 'main.triggers[0].actions[0]');
expect(result2).toEqual(`/bot/${projectId}/dialogs/main?selected=triggers[0]&focused=triggers[0].actions[0]`);
const result3 = convertPathToUrl(projectId, 'main', 'main.triggers[0].actions[0]#Microsoft.TextInput#prompt');
const result1 = convertPathToUrl(projectId, skillId, 'main');
expect(result1).toEqual(`/bot/${projectId}/skill/${skillId}/dialogs/main`);
const result2 = convertPathToUrl(projectId, skillId, 'main', 'main.triggers[0].actions[0]');
expect(result2).toEqual(
`/bot/${projectId}/skill/${skillId}/dialogs/main?selected=triggers[0]&focused=triggers[0].actions[0]`
);
const result3 = convertPathToUrl(
projectId,
skillId,
'main',
'main.triggers[0].actions[0]#Microsoft.TextInput#prompt'
);
expect(result3).toEqual(
`/bot/${projectId}/dialogs/main?selected=triggers[0]&focused=triggers[0].actions[0]#botAsks`
`/bot/${projectId}/skill/${skillId}/dialogs/main?selected=triggers[0]&focused=triggers[0].actions[0]#botAsks`
);
const result4 = convertPathToUrl(projectId, null, 'main');
expect(result4).toEqual(`/bot/${projectId}/dialogs/main`);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,21 @@
import { jsx, css } from '@emotion/core';
import { useRecoilValue } from 'recoil';
import { forwardRef } from 'react';
// import formatMessage from 'format-message';

import { RequireAuth } from '../RequireAuth';
import { ErrorBoundary } from '../ErrorBoundary';
import { Conversation } from '../Conversation';
//import { ProjectTree } from '../ProjectTree/ProjectTree';
//import { LeftRightSplit } from '../Split/LeftRightSplit';
beyackle marked this conversation as resolved.
Show resolved Hide resolved

import Routes from './../../router';
import { applicationErrorState, dispatcherState, currentProjectIdState } from './../../recoilModel';
import {
applicationErrorState,
dispatcherState,
currentProjectIdState,
// currentModeState,
} from './../../recoilModel';

// -------------------- Styles -------------------- //

Expand All @@ -33,10 +42,20 @@ const content = css`

const Content = forwardRef<HTMLDivElement>((props, ref) => <div css={content} {...props} ref={ref} />);

// const SHOW_TREE = ['design'];

export const RightPanel = () => {
const applicationError = useRecoilValue(applicationErrorState);
const { setApplicationLevelError, fetchProjectById } = useRecoilValue(dispatcherState);
const projectId = useRecoilValue(currentProjectIdState);
//const currentMode = useRecoilValue(currentModeState);

const conversation = (
<Conversation>
<Routes component={Content} />
</Conversation>
);

return (
<div css={rightPanel}>
<ErrorBoundary
Expand All @@ -45,7 +64,17 @@ export const RightPanel = () => {
setApplicationLevelError={setApplicationLevelError}
>
<RequireAuth>
<Routes component={Content} />
<div css={{ display: 'flex', flexDirection: 'row', label: 'MainPage' }}>
{/*
{SHOW_TREE.includes(currentMode) ? (
<LeftRightSplit initialLeftGridWidth="200px" minLeftPixels={200} minRightPixels={800}>
<ProjectTree regionName={formatMessage('Project tree')} showTriggers={currentMode === 'design'} />
{conversation}
</LeftRightSplit>
) : ( */}
{conversation}
{/* })} */}
</div>
</RequireAuth>
</ErrorBoundary>
</div>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

/** @jsx jsx */
import { jsx, css } from '@emotion/core';
import { useState, MouseEvent, KeyboardEvent } from 'react';

type Props = {
children: JSX.Element;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: React.ReactNode instead of JSX

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

summary: JSX.Element;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: React.ReactNode instead of JSX

depth?: number;
detailsRef?: (el: HTMLElement | null) => void;
};

const summaryStyle = css`
label: summary;
display: flex;
padding-left: 12px;
padding-top: 6px;
`;

const nodeStyle = (depth: number) => css`
margin-left: ${depth * 16}px;
`;

export const ExpandableNode = ({ children, summary, detailsRef, depth = 0 }: Props) => {
const [isOpen, setOpen] = useState(true);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: isExpanded

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see that it makes that much difference, but okay.


function handleClick(ev: MouseEvent) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't the event type: React.MouseEvent<HTMLElement, MouseEvent>

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not according to TypeScript. That's a standard HTML

tag, and following the chain of type declarations that VSCode gives me leads to this line in the TS lib:
onclick: ((this: GlobalEventHandlers, ev: MouseEvent) => any) | null;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screen Shot 2020-10-19 at 2 41 53 PM

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the context of react, these are SyntheticEvent. However, as long as the type check is happy I'm fine.

if ((ev.target as Element)?.tagName.toLowerCase() === 'summary') {
setOpen(!isOpen);
}
ev.preventDefault();
}

function handleKey(ev: KeyboardEvent) {
if (ev.key === 'Enter' || ev.key === 'Space') setOpen(!isOpen);
beyackle marked this conversation as resolved.
Show resolved Hide resolved
}

return (
<div css={nodeStyle(depth)} data-testid="dialog">
<details ref={detailsRef} open={isOpen}>
{/* eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions, jsx-a11y/no-noninteractive-tabindex */}
<summary css={summaryStyle} role="button" tabIndex={0} onClick={handleClick} onKeyUp={handleKey}>
{summary}
</summary>
{children}
</details>
</div>
);
};
Loading