Skip to content

Commit

Permalink
app: separates redux states to reduce re-renders, create conversation…
Browse files Browse the repository at this point in the history
… form now closes on completion, other misc (#204)

* Disable react strict mode via env var for easier testing w/o as needed
* Prevent fetching speech recognizer if already fetching
* Fix issue loading user photos
  • Loading branch information
bkrabach authored Nov 4, 2024
1 parent 2721c87 commit b73b916
Show file tree
Hide file tree
Showing 52 changed files with 735 additions and 785 deletions.
13 changes: 13 additions & 0 deletions workbench-app/.vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,19 @@
"console": "integratedTerminal",
"runtimeExecutable": "npm",
"runtimeArgs": ["run", "dev"]
},
{
"type": "node",
"request": "launch",
"name": "app: semantic-workbench-app (no strict mode)",
"env": {
"VITE_DISABLE_STRICT_MODE": "true"
},
"cwd": "${workspaceFolder}",
"skipFiles": ["<node_internals>/**"],
"console": "integratedTerminal",
"runtimeExecutable": "npm",
"runtimeArgs": ["run", "dev"]
}
]
}
2 changes: 1 addition & 1 deletion workbench-app/src/Constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export const Constants = {
maxFileAttachmentsPerMessage: 10,
loaderDelayMs: 100,
responsiveBreakpoints: {
interactCanvas: '900px',
chatCanvas: '900px',
},
speechIdleTimeoutMs: 4000,
azureSpeechTokenRefreshIntervalMs: 540000, // 540000 ms = 9 minutes
Expand Down
2 changes: 1 addition & 1 deletion workbench-app/src/components/App/AppView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ interface AppViewProps {
export const AppView: React.FC<AppViewProps> = (props) => {
const { title, actions, fullSizeContent, children } = props;
const classes = useClasses();
const { completedFirstRun } = useAppSelector((state) => state.app);
const completedFirstRun = useAppSelector((state) => state.app.completedFirstRun);
const navigate = useNavigate();

React.useLayoutEffect(() => {
Expand Down
6 changes: 1 addition & 5 deletions workbench-app/src/components/App/DialogControl.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,7 @@ export const DialogControl: React.FC<DialogControlContent> = (props) => {
</Button>
</DialogTrigger>
)}
{additionalActions?.map((action, index) => (
<DialogTrigger key={index} disableButtonEnhancement>
{action}
</DialogTrigger>
))}
{additionalActions}
</DialogActions>
</DialogBody>
</DialogSurface>
Expand Down
2 changes: 1 addition & 1 deletion workbench-app/src/components/App/ErrorListFromAppState.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ interface ErrorListFromAppStateProps {
export const ErrorListFromAppState: React.FC<ErrorListFromAppStateProps> = (props) => {
const { className } = props;
const classes = useClasses();
const { errors } = useAppSelector((state: RootState) => state.app);
const errors = useAppSelector((state: RootState) => state.app.errors);
const dispatch = useAppDispatch();

if (!errors || errors.length === 0) {
Expand Down
2 changes: 1 addition & 1 deletion workbench-app/src/components/App/ExperimentalNotice.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ const useClasses = makeStyles({

export const ExperimentalNotice: React.FC = () => {
const classes = useClasses();
const { completedFirstRun } = useAppSelector((state) => state.app);
const completedFirstRun = useAppSelector((state) => state.app.completedFirstRun);
const dispatch = useAppDispatch();
const [showDialog, setShowDialog] = React.useState(!completedFirstRun?.experimental);
const [currentIndex, setCurrentIndex] = React.useState(0);
Expand Down
55 changes: 28 additions & 27 deletions workbench-app/src/components/App/ProfileSettings.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Copyright (c) Microsoft. All rights reserved.

import { useIsAuthenticated, useMsal } from '@azure/msal-react';
import { useAccount, useIsAuthenticated, useMsal } from '@azure/msal-react';
import {
Label,
Link,
Expand All @@ -15,7 +15,7 @@ import React from 'react';
import { AuthHelper } from '../../libs/AuthHelper';
import { useMicrosoftGraph } from '../../libs/useMicrosoftGraph';
import { useAppDispatch, useAppSelector } from '../../redux/app/hooks';
import { setUserPhoto } from '../../redux/features/app/appSlice';
import { setLocalUser } from '../../redux/features/localUser/localUserSlice';

const useClasses = makeStyles({
popoverSurface: {
Expand All @@ -33,31 +33,32 @@ export const ProfileSettings: React.FC = () => {
const classes = useClasses();
const { instance } = useMsal();
const isAuthenticated = useIsAuthenticated();
const account = useAccount();
const microsoftGraph = useMicrosoftGraph();
const { localUser, userPhoto } = useAppSelector((state) => state.app);
const localUserState = useAppSelector((state) => state.localUser);
const dispatch = useAppDispatch();

// FIXME: prevent multiple calls before the setUserPhoto is updated
// If not wrapped in a useEffect, an error is thrown when the state is updated
// while other components are still rendering. Putting in a useEffect
// prevents the error from being thrown, but then the photo may get fetched
// multiple times when multiple components are rendering at the same time
// and the state update has not yet been processed. Not the end of the world,
// as it tends to be just a few calls, but it's not ideal.
React.useEffect(() => {
if (isAuthenticated && !userPhoto.src && !userPhoto.isLoading) {
dispatch(setUserPhoto({ isLoading: true, src: undefined }));
(async () => {
const photo = await microsoftGraph.getMyPhotoAsync();
dispatch(
setUserPhoto({
isLoading: false,
src: photo,
}),
);
})();
}
}, [dispatch, isAuthenticated, microsoftGraph, userPhoto.isLoading, userPhoto.src]);
(async () => {
if (!isAuthenticated || !account?.name || localUserState.id) {
return;
}

const photo = await microsoftGraph.getMyPhotoAsync();

dispatch(
setLocalUser({
id: (account.homeAccountId || '').split('.').reverse().join('.'),
name: account.name,
email: account.username,
avatar: {
name: account.name,
image: photo ? { src: photo } : undefined,
},
}),
);
})();
}, [account, dispatch, isAuthenticated, localUserState.id, microsoftGraph]);

const handleSignOut = () => {
void AuthHelper.logoutAsync(instance);
Expand All @@ -70,15 +71,15 @@ export const ProfileSettings: React.FC = () => {
return (
<Popover positioning="below-end">
<PopoverTrigger>
<Persona className="user-avatar" avatar={localUser?.avatar} presence={{ status: 'available' }} />
<Persona className="user-avatar" avatar={localUserState.avatar} presence={{ status: 'available' }} />
</PopoverTrigger>
<PopoverSurface>
<div className={classes.popoverSurface}>
{isAuthenticated && localUser ? (
{isAuthenticated ? (
<>
<div className={classes.accountInfo}>
<Label>{localUser.name}</Label>
<Label size="small">{localUser.email}</Label>
<Label>{localUserState.name}</Label>
<Label size="small">{localUserState.email}</Label>
</div>
<Link onClick={handleSignOut}>Sign Out</Link>
</>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import { Tab, TabList, makeStyles, shorthands, tokens } from '@fluentui/react-components';
import React from 'react';
import { useInteractCanvasController } from '../../../libs/useInteractCanvasController';
import { useChatCanvasController } from '../../../libs/useChatCanvasController';
import { Assistant } from '../../../models/Assistant';
import { Conversation } from '../../../models/Conversation';
import { AssistantCanvas } from './AssistantCanvas';
Expand Down Expand Up @@ -39,7 +39,7 @@ interface AssistantCanvasListProps {
export const AssistantCanvasList: React.FC<AssistantCanvasListProps> = (props) => {
const { selectedAssistant, conversationAssistants, conversation } = props;
const classes = useClasses();
const interactCanvasController = useInteractCanvasController();
const chatCanvasController = useChatCanvasController();

if (conversationAssistants.length === 1) {
// Only one assistant, no need to show tabs, just show the single assistant
Expand All @@ -61,10 +61,10 @@ export const AssistantCanvasList: React.FC<AssistantCanvasListProps> = (props) =
);

// Set the new assistant as the active assistant
// If we can't find the assistant, we'll set the assistant to null
interactCanvasController.transitionToState({
assistantId: conversationAssistant?.id ?? null,
assistantStateId: null,
// If we can't find the assistant, we'll set the assistant to undefined
chatCanvasController.transitionToState({
selectedAssistantId: conversationAssistant?.id,
selectedAssistantStateId: undefined,
});
}}
size="small"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
tokens,
} from '@fluentui/react-components';
import React from 'react';
import { useInteractCanvasController } from '../../../libs/useInteractCanvasController';
import { useChatCanvasController } from '../../../libs/useChatCanvasController';
import { Assistant } from '../../../models/Assistant';
import { AssistantStateDescription } from '../../../models/AssistantStateDescription';
import { useAppSelector } from '../../../redux/app/hooks';
Expand Down Expand Up @@ -54,8 +54,8 @@ interface AssistantInspectorListProps {
export const AssistantInspectorList: React.FC<AssistantInspectorListProps> = (props) => {
const { conversationId, assistant, stateDescriptions } = props;
const classes = useClasses();
const { interactCanvasState } = useAppSelector((state) => state.app);
const interactCanvasController = useInteractCanvasController();
const chatCanvasState = useAppSelector((state) => state.chatCanvas);
const chatCanvasController = useChatCanvasController();

if (stateDescriptions.length === 1) {
// Only one assistant state, no need to show tabs, just show the single assistant state
Expand All @@ -69,7 +69,7 @@ export const AssistantInspectorList: React.FC<AssistantInspectorListProps> = (pr
}

const onTabSelect: SelectTabEventHandler = (_event: SelectTabEvent, data: SelectTabData) => {
interactCanvasController.transitionToState({ assistantStateId: data.value as string });
chatCanvasController.transitionToState({ selectedAssistantStateId: data.value as string });
};

if (stateDescriptions.length === 0) {
Expand All @@ -85,8 +85,9 @@ export const AssistantInspectorList: React.FC<AssistantInspectorListProps> = (pr
}

const selectedStateDescription =
stateDescriptions.find((stateDescription) => stateDescription.id === interactCanvasState?.assistantStateId) ??
stateDescriptions[0];
stateDescriptions.find(
(stateDescription) => stateDescription.id === chatCanvasState.selectedAssistantStateId,
) ?? stateDescriptions[0];
const selectedTab = selectedStateDescription.id;

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ import { Button, makeStyles, shorthands, tokens, Tooltip } from '@fluentui/react
import { AppsList24Regular, BookInformation24Regular, Dismiss24Regular } from '@fluentui/react-icons';
import { EventSourceMessage } from '@microsoft/fetch-event-source';
import React from 'react';
import { useChatCanvasController } from '../../../libs/useChatCanvasController';
import { useEnvironment } from '../../../libs/useEnvironment';
import { useInteractCanvasController } from '../../../libs/useInteractCanvasController';
import { WorkbenchEventSource, WorkbenchEventSourceType } from '../../../libs/WorkbenchEventSource';
import { useAppSelector } from '../../../redux/app/hooks';

Expand All @@ -31,20 +31,20 @@ interface CanvasControlsProps {
export const CanvasControls: React.FC<CanvasControlsProps> = (props) => {
const { conversationId } = props;
const classes = useClasses();
const { interactCanvasState } = useAppSelector((state) => state.app);
const chatCanvasState = useAppSelector((state) => state.chatCanvas);
const environment = useEnvironment();
const interactCanvasController = useInteractCanvasController();
const chatCanvasController = useChatCanvasController();

React.useEffect(() => {
var workbenchEventSource: WorkbenchEventSource | undefined;

const handleFocusEvent = async (event: EventSourceMessage) => {
const { data } = JSON.parse(event.data);
interactCanvasController.transitionToState({
chatCanvasController.transitionToState({
open: true,
mode: 'assistant',
assistantId: data['assistant_id'],
assistantStateId: data['state_id'],
selectedAssistantId: data['assistant_id'],
selectedAssistantStateId: data['state_id'],
});
};

Expand All @@ -60,35 +60,35 @@ export const CanvasControls: React.FC<CanvasControlsProps> = (props) => {
return () => {
workbenchEventSource?.removeEventListener('assistant.state.focus', handleFocusEvent);
};
}, [environment, conversationId, interactCanvasController]);
}, [environment, conversationId, chatCanvasController]);

const handleActivateConversation = () => {
interactCanvasController.transitionToState({ open: true, mode: 'conversation' });
chatCanvasController.transitionToState({ open: true, mode: 'conversation' });
};

const handleActivateAssistant = () => {
interactCanvasController.transitionToState({ open: true, mode: 'assistant' });
chatCanvasController.transitionToState({ open: true, mode: 'assistant' });
};

const handleDismiss = async () => {
interactCanvasController.transitionToState({ open: false });
chatCanvasController.transitionToState({ open: false });
};

const conversationActive = interactCanvasState?.mode === 'conversation' && interactCanvasState?.open;
const assistantActive = interactCanvasState?.mode === 'assistant' && interactCanvasState?.open;
const conversationActive = chatCanvasState.mode === 'conversation' && chatCanvasState.open;
const assistantActive = chatCanvasState.mode === 'assistant' && chatCanvasState.open;

return (
<div className={classes.root}>
<Tooltip content="Open conversation canvas" relationship="label">
<Button
disabled={conversationActive || interactCanvasController.isTransitioning}
disabled={conversationActive || chatCanvasController.isTransitioning}
icon={<AppsList24Regular />}
onClick={handleActivateConversation}
/>
</Tooltip>
<Tooltip content="Open assistant canvas" relationship="label">
<Button
disabled={assistantActive || interactCanvasController.isTransitioning}
disabled={assistantActive || chatCanvasController.isTransitioning}
icon={<BookInformation24Regular />}
onClick={handleActivateAssistant}
/>
Expand Down
Loading

0 comments on commit b73b916

Please sign in to comment.