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: add new survey notification to design page #8155

Merged
merged 32 commits into from
Jun 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
a23eebb
add custom style
beyackle Jun 14, 2021
95d2677
Update Publish.tsx
beyackle Jun 14, 2021
de094b9
display card on Home page
beyackle Jun 14, 2021
611ff7f
Update constants.tsx
beyackle Jun 14, 2021
1bef946
Update shell.ts
beyackle Jun 14, 2021
21aaba2
Merge branch 'feature/hats-survey' into beyackle/hatsUsage
beyackle Jun 15, 2021
5f51dab
Update Home.tsx
beyackle Jun 15, 2021
397a746
move survey into separate component
beyackle Jun 15, 2021
0fe5ebd
get more info automatically
beyackle Jun 15, 2021
518ed0e
move URL into constant
beyackle Jun 15, 2021
150ec25
move more props around
beyackle Jun 15, 2021
6a160b2
Update constants.tsx
beyackle Jun 15, 2021
43beda7
turn non-rendering component into hook
beyackle Jun 15, 2021
ecf2e2d
move card to design page
beyackle Jun 16, 2021
dadb925
start adding ipcRenderer stuff
beyackle Jun 17, 2021
a9babdd
update card design
beyackle Jun 17, 2021
7ea55db
get machineID and stash in Recoil
beyackle Jun 18, 2021
7b4d460
read OS and place in URL
beyackle Jun 18, 2021
27037dc
add spacer to notification card
beyackle Jun 18, 2021
854cffc
better notification styling
beyackle Jun 18, 2021
7d8f8ae
fix constants and typercheck error
beyackle Jun 18, 2021
f7c1459
Merge branch 'feature/hats-survey' into beyackle/hatsUsage
beyackle Jun 21, 2021
24d9f06
fixes from PR
beyackle Jun 21, 2021
80f7464
fix card alignment and make aka.ms URL
beyackle Jun 21, 2021
e727353
remove redundant prop
beyackle Jun 21, 2021
fbeff10
use query-string package to generate link
beyackle Jun 22, 2021
c6416d5
move MAC truncation to electron server
beyackle Jun 22, 2021
ac6853a
add fixes from CR and machineID truncation
beyackle Jun 23, 2021
d0cd394
fix typechecking
beyackle Jun 23, 2021
7121967
Update ToolbarButtonMenu.test.tsx
beyackle Jun 23, 2021
0dd4982
Update App.tsx
beyackle Jun 24, 2021
f062b3f
Merge branch 'feature/hats-survey' into beyackle/hatsUsage
beyackle Jun 24, 2021
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
7 changes: 6 additions & 1 deletion Composer/packages/client/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export const App: React.FC = () => {
checkNodeVersion,
performAppCleanupOnQuit,
setSurveyEligibility,
setMachineInfo,
} = useRecoilValue(dispatcherState);

useEffect(() => {
Expand All @@ -42,7 +43,11 @@ export const App: React.FC = () => {
ipcRenderer?.on('cleanup', (_event) => {
performAppCleanupOnQuit();
});
setSurveyEligibility();

ipcRenderer?.on('machine-info', (_event, info) => {
setMachineInfo(info);
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like you are using the machine info only inside the useSurveyNotification hook. If so, why do we need to create a recoil state for it. You can just use the ipcRenderer.on inside the hook. Ex useInitializeLoger.ts

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 was trying to make this work last week, but it runs into annoying race conditions where the message handler isn't initialized before the message gets sent and so nothing gets through. Using the Recoil store lets us put the machine info down somewhere whenever we receive it and then count on it being available when we need it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense to usethe recoil state unless the hook can be registered in app.tsx

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you explain the race condition a bit more. I'm also confused why we can't just initialize the survey here when the event is received? If the message handler doesn't get initialized before the message is sent, we would never get the message, right?

setSurveyEligibility();
});
}, []);

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { useRef } from 'react';
import { FontSizes, SharedColors } from '@uifabric/fluent-theme';
import { Shimmer, ShimmerElementType } from 'office-ui-fabric-react/lib/Shimmer';
import { Icon } from 'office-ui-fabric-react/lib/Icon';
import { Stack, IStackProps } from 'office-ui-fabric-react/lib/Stack';
import formatMessage from 'format-message';
import { Notification, NotificationLink } from '@botframework-composer/types';

Expand Down Expand Up @@ -36,7 +37,7 @@ const cardContainer = (show: boolean, ref?: HTMLDivElement | null) => () => {
border-left: 4px solid #0078d4;
background: white;
box-shadow: 0 6.4px 14.4px 0 rgba(0, 0, 0, 0.132), 0 1.2px 3.6px 0 rgba(0, 0, 0, 0.108);
min-width: 340px;
width: 340px;
beyackle marked this conversation as resolved.
Show resolved Hide resolved
border-radius: 2px;
display: flex;
flex-direction: column;
Expand Down Expand Up @@ -112,13 +113,20 @@ const cardDescription = css`
word-break: break-word;
`;

const linkButton = css`
color: #0078d4;
float: right;
font-size: 12px;
height: auto;
margin: 4px 0 4px 8px;
`;
const linkButton = {
root: {
padding: '0',
beyackle marked this conversation as resolved.
Show resolved Hide resolved
border: '0',
},
label: {
fontSize: '12px',
color: SharedColors.cyanBlue10,
margin: '0',
},
textContainer: {
height: '16px',
},
};

const getShimmerStyles = {
root: {
Expand Down Expand Up @@ -148,27 +156,58 @@ export type NotificationProps = {
};

const makeLinkLabel = (link: NotificationLink) => (
<ActionButton css={linkButton} onClick={link.onClick}>
<ActionButton styles={linkButton} onClick={link.onClick}>
{link.label}
</ActionButton>
);

const defaultCardContentRenderer = (props: CardProps) => {
const { title, description, type, link, links } = props;
const { title, description, type, link, links, leftLinks, rightLinks } = props;

const rightLinkList = rightLinks ?? links ?? [link];
const leftLinkList = leftLinks ?? [];

const stackProps: IStackProps = {
horizontal: true,
horizontalAlign: 'space-between',
tokens: {
childrenGap: '20px',
padding: '0 16px 0 0',
maxHeight: '24px',
},
};

return (
<div css={cardContent}>
{type === 'error' && <Icon css={errorType} iconName="ErrorBadge" />}
{type === 'success' && <Icon css={successType} iconName="Completed" />}
{type === 'warning' && <Icon css={warningType} iconName="Warning" />}
{type === 'question' && <Icon css={questionType} iconName="UnknownSolid" />}
{type === 'congratulation' && <Icon css={congratulationType} iconName="Trophy2Solid" />}
{type === 'custom' && (
<Icon
css={css`
margin-top: ${iconMargin};
color: ${props.color ?? SharedColors.gray10};
`}
iconName={props.icon ?? 'UnknownSolid'}
/>
)}
<div css={cardDetail}>
<div css={cardTitle}>{title}</div>
{description && <div css={cardDescription}>{description}</div>}
{link && makeLinkLabel(link)}
{links?.map((link) => (
<div key={link.label}>{makeLinkLabel(link)}</div>
))}
<Stack horizontal horizontalAlign="space-between">
<Stack {...stackProps}>
{leftLinkList.map(
(link) => link != null && <Stack.Item key={link.label}>{makeLinkLabel(link)}</Stack.Item>
)}
</Stack>
<Stack {...stackProps}>
{rightLinkList.map(
(link) => link != null && <Stack.Item key={link.label}>{makeLinkLabel(link)}</Stack.Item>
beyackle marked this conversation as resolved.
Show resolved Hide resolved
)}
</Stack>
</Stack>
{type === 'pending' && (
<Shimmer shimmerElements={[{ type: ShimmerElementType.line, height: 2 }]} styles={getShimmerStyles} />
)}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

import { useEffect } from 'react';
import { useRecoilValue } from 'recoil';
import formatMessage from 'format-message';
import querystring from 'query-string';

import { ClientStorage } from '../../utils/storage';
import { surveyEligibilityState, dispatcherState, machineInfoState } from '../../recoilModel/atoms/appState';
import { MachineInfo } from '../../recoilModel/types';
import { SURVEY_URL_BASE } from '../../constants';
import TelemetryClient from '../../telemetry/TelemetryClient';

const buildUrl = (info: MachineInfo) => {
// User OS
// hashed machineId
// composer version
// maybe include subscription ID; wait for global sign-in feature
// session ID (global telemetry GUID)
const version = process.env.COMPOSER_VERSION;

const parameters = {
Source: 'Composer',
os: info?.os || 'Unknown',
machineId: info?.id,
version,
};

return `${SURVEY_URL_BASE}?${querystring.stringify(parameters)}`;
};

export const useSurveyNotification = () => {
const { addNotification, deleteNotification } = useRecoilValue(dispatcherState);
const surveyEligible = useRecoilValue(surveyEligibilityState);
const machineInfo = useRecoilValue(machineInfoState);

useEffect(() => {
const url = buildUrl(machineInfo);
deleteNotification('survey');

if (surveyEligible) {
const surveyStorage = new ClientStorage(window.localStorage, 'survey');
TelemetryClient.track('HATSSurveyOffered');

addNotification({
id: 'survey',
type: 'question',
title: formatMessage('Would you mind taking a quick survey?'),
description: formatMessage('We read every response and will use your feedback to improve Composer.'),
leftLinks: [
{
label: formatMessage('Take survey'),
onClick: () => {
// This is safe; we control what the URL that gets built is
// eslint-disable-next-line security/detect-non-literal-fs-filename
window.open(url, '_blank');
TelemetryClient.track('HATSSurveyAccepted');
deleteNotification('survey');
},
},

{
// this is functionally identical to clicking the close box
label: formatMessage('Remind me later'),
onClick: () => {
TelemetryClient.track('HATSSurveyDismissed');
deleteNotification('survey');
},
},
],
rightLinks: [
{
label: formatMessage('No thanks'),
onClick: () => {
TelemetryClient.track('HATSSurveyRejected');
deleteNotification('survey');
surveyStorage.set('optedOut', true);
},
},
],
});
}
}, []);
};
7 changes: 4 additions & 3 deletions Composer/packages/client/src/constants.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -527,9 +527,10 @@ export const defaultBotPort = 3979;
export const defaultBotEndpoint = `http://localhost:${defaultBotPort}/api/messages`;

const DAYS_IN_MS = 1000 * 60 * 60 * 24;

export const SURVEY_PARAMETERS = {
daysUntilEligible: 5,
daysUntilEligible: 2,
timeUntilNextSurvey: 90 * DAYS_IN_MS,
chanceToAppear: 0.15,
chanceToAppear: 0.3,
};

export const SURVEY_URL_BASE = 'https://aka.ms/bfcomposersurvey';
3 changes: 3 additions & 0 deletions Composer/packages/client/src/pages/design/DesignPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { Split, SplitMeasuredSizes } from '@geoffcox/react-splitter';
import { dispatcherState } from '../../recoilModel';
import { renderThinSplitter } from '../../components/Split/ThinSplitter';
import { Conversation } from '../../components/Conversation';
import { useSurveyNotification } from '../../components/Notifications/useSurveyNotification';

import SideBar from './SideBar';
import CommandBar from './CommandBar';
Expand All @@ -33,6 +34,8 @@ const DesignPage: React.FC<RouteComponentProps<{ dialogId: string; projectId: st

const activeBot = skillId ?? projectId;

useSurveyNotification();

return (
<div css={contentWrapper} role="main">
<Split
Expand Down
1 change: 1 addition & 0 deletions Composer/packages/client/src/pages/home/Home.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ const Home: React.FC<RouteComponentProps> = () => {
// disabled: botName ? false : true,
// },
];

return (
<div css={home.outline}>
<div css={home.page}>
Expand Down
34 changes: 0 additions & 34 deletions Composer/packages/client/src/pages/publish/Publish.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -163,40 +163,6 @@ const Publish: React.FC<RouteComponentProps<{ projectId: string; targetName?: st
}, [botList]);

useEffect(() => {
addNotification({
id: 'test',
type: 'question',
title: 'Would you like to take a survey?',
description: "This is where we would ask the user if they'd like to take a survey.",
links: [
{
label: 'Take the survey?',
onClick: () => {
console.log('clicked');
},
},
{
label: 'Opt out of this and future surveys',
onClick: () => {
console.log('opted out');
},
},
],
});

addNotification({
id: 'test2',
type: 'congratulation',
title: "You've published 10 bots!",
description: 'Congratulations!',
link: {
label: 'Got it',
onClick: () => {
console.log('clicked this');
},
},
});

// Clear intervals when unmount
return () => {
if (pollingUpdaterList) {
Expand Down
6 changes: 6 additions & 0 deletions Composer/packages/client/src/recoilModel/atoms/appState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
AppUpdateState,
BoilerplateVersion,
Notification,
MachineInfo,
} from '../../recoilModel/types';
import { getUserSettings } from '../utils';
import onboardingStorage from '../../utils/onboardingStorage';
Expand Down Expand Up @@ -370,6 +371,11 @@ export const surveyEligibilityState = atom<boolean>({
default: false,
});

export const machineInfoState = atom<MachineInfo>({
key: getFullyQualifiedKey('machineInfoState'),
default: null,
});

export const showGetStartedTeachingBubbleState = atom<boolean>({
key: getFullyQualifiedKey('showGetStartedTeachingBubbleState'),
default: false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,15 @@ import {
userHasNodeInstalledState,
applicationErrorState,
surveyEligibilityState,
machineInfoState,
showGetStartedTeachingBubbleState,
showErrorDiagnosticsState,
showWarningDiagnosticsState,
projectsForDiagnosticsFilterState,
} from '../atoms/appState';
import { AppUpdaterStatus, CreationFlowStatus, CreationFlowType, SURVEY_PARAMETERS } from '../../constants';
import OnboardingState from '../../utils/onboardingStorage';
import { StateError, AppUpdateState } from '../../recoilModel/types';
import { StateError, AppUpdateState, MachineInfo } from '../../recoilModel/types';
import { DebugDrawerKeys } from '../../pages/design/DebugPanel/TabExtensions/types';
import httpClient from '../../utils/httpUtil';
import { ClientStorage } from '../../utils/storage';
Expand Down Expand Up @@ -163,7 +164,10 @@ export const applicationDispatcher = () => {
const surveyStorage = new ClientStorage(window.localStorage, 'survey');

const optedOut = surveyStorage.get('optedOut', false);
if (optedOut) return;
if (optedOut) {
set(surveyEligibilityState, false);
return;
}

let days = surveyStorage.get('days', 0);
const lastUsed = surveyStorage.get('dateLastUsed', null);
Expand All @@ -185,6 +189,10 @@ export const applicationDispatcher = () => {
surveyStorage.set('dateLastUsed', today);
});

const setMachineInfo = useRecoilCallback((callbackHelpers: CallbackInterface) => (info: MachineInfo) => {
callbackHelpers.set(machineInfoState, info);
});

const setShowGetStartedTeachingBubble = useRecoilCallback((callbackHelpers: CallbackInterface) => (show: boolean) => {
callbackHelpers.set(showGetStartedTeachingBubbleState, show);
});
Expand Down Expand Up @@ -218,6 +226,7 @@ export const applicationDispatcher = () => {
setDebugPanelExpansion,
setActiveTabInDebugPanel,
setSurveyEligibility,
setMachineInfo,
setShowGetStartedTeachingBubble,
setErrorDiagnosticsFilter,
setWarningDiagnosticsFilter,
Expand Down
5 changes: 5 additions & 0 deletions Composer/packages/client/src/recoilModel/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,3 +129,8 @@ export type WebChatInspectionData = {
};

export type RuntimeOutputData = { standardOutput: string; standardError: BotStartError | null };

export type MachineInfo = null | {
id: string;
os: string;
};
4 changes: 4 additions & 0 deletions Composer/packages/electron-server/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -295,11 +295,15 @@ async function run() {
setTimeout(() => startApp(signalThatMainWindowIsShowing), 500);

const mainWindow = getMainWindow();
const machineId = await getMachineId();

mainWindow?.webContents.send('session-update', 'session-started');

if (process.env.COMPOSER_DEV_TOOLS) {
mainWindow?.webContents.openDevTools();
}

mainWindow?.webContents.send('machine-info', { id: machineId, os: os.platform() });
});

// Quit when all windows are closed.
Expand Down
Loading