Skip to content

Commit

Permalink
UX improvements: A11y labels, fixing space input bug, enabling RLFH, …
Browse files Browse the repository at this point in the history 
…etc. (#179)

### Motivation and Context

<!-- Thank you for your contribution to the copilot-chat repo!
Please help reviewers and future users, providing the following
information:
  1. Why is this change required?
  2. What problem does it solve?
  3. What scenario does it contribute to?
  4. If it fixes an open issue, please link to the issue here.
-->

This PR contains the following UX improvements:
- Fixes bug where you couldn't enter space when editing chat name on
list item
- Fixes auto-naming bug to match all datetime syntaxes
- Removes inactive field of RLFH - users can now enable this from
settings
- Adds multiple A11y aria-labels and hover text 
- Limit privacy alert to users in Microsoft tenant only

### Description

<!-- Describe your changes, the overall approach, the underlying design.
These notes will help understanding how your code works. Thanks! -->

Persona component was conflicting with "Space" keydown behavior of
ChatListItem, so I removed it. Using the `title` property on the
component is sufficient to show hover text:
<img width="362" alt="Screenshot 2023-08-15 at 11 22 04 AM"
src="https://github.com/microsoft/chat-copilot/assets/125500434/78cf38ee-0138-485e-9ee7-cee3f2962823">
<img width="341" alt="Screenshot 2023-08-15 at 11 22 56 AM"
src="https://github.com/microsoft/chat-copilot/assets/125500434/4da12ac4-f596-49c2-b863-f3b34af68988">

### Contribution Checklist

<!-- Before submitting this PR, please make sure: -->

- [x] The code builds clean without any errors or warnings
- [x] The PR follows the [Contribution
Guidelines](https://github.com/microsoft/copilot-chat/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/copilot-chat/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
~~- [ ] All unit tests pass, and I have added new tests where possible~~
- [x] I didn't break anyone 😄
  • Loading branch information
@teresaqhoang
teresaqhoang authored Aug 15, 2023
1 parent 07075ae commit 6d5ba70
Show file tree
Hide file tree
Showing 11 changed files with 123 additions and 103 deletions.
15 changes: 14 additions & 1 deletion webapp/src/App.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,11 @@ import { UserSettingsMenu } from './components/header/UserSettingsMenu';
import { PluginGallery } from './components/open-api-plugins/PluginGallery';
import { BackendProbe, ChatView, Error, Loading, Login } from './components/views';
import { useChat } from './libs/hooks';
import { AlertType } from './libs/models/AlertType';
import { useAppDispatch, useAppSelector } from './redux/app/hooks';
import { RootState } from './redux/app/store';
import { FeatureKeys } from './redux/features/app/AppState';
import { setActiveUserInfo, setServiceOptions } from './redux/features/app/appSlice';
import { addAlert, setActiveUserInfo, setServiceOptions } from './redux/features/app/appSlice';
import { semanticKernelDarkTheme, semanticKernelLightTheme } from './styles';

export const useClasses = makeStyles({
Expand Down Expand Up @@ -81,6 +82,18 @@ const App: FC = () => {
username: account.name ?? account.username,
}),
);

// Privacy disclaimer for internal Microsoft users
if (account.username.split('@')[1] === 'microsoft.com') {
dispatch(
addAlert({
message:
'By using Chat Copilot, you agree to protect sensitive data, not store it in chat, and allow chat history collection for service improvements. This tool is for internal use only.',
type: AlertType.Info,
}),
);
}

setAppState(AppState.LoadingChats);
}
} else {
Expand Down
6 changes: 6 additions & 0 deletions webapp/src/components/chat/ChatInput.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,8 @@ export const ChatInput: React.FC<ChatInputProps> = ({ isDraggingOver, onDragLeav
<Alerts />
<div className={classes.content}>
<Textarea
title="Chat input"
aria-label="Chat input field. Click enter to submit input."
ref={textAreaRef}
id="chat-input"
resize="vertical"
Expand Down Expand Up @@ -245,6 +247,8 @@ export const ChatInput: React.FC<ChatInputProps> = ({ isDraggingOver, onDragLeav
appearance="transparent"
icon={<AttachRegular />}
onClick={() => documentFileRef.current?.click()}
title="Attach file"
aria-label="Attach file button"
/>
{documentImporting && <Spinner size="tiny" />}
</div>
Expand All @@ -258,6 +262,8 @@ export const ChatInput: React.FC<ChatInputProps> = ({ isDraggingOver, onDragLeav
/>
)}
<Button
title="Submit"
aria-label="Submit message"
appearance="transparent"
icon={<SendRegular />}
onClick={() => {
Expand Down
28 changes: 24 additions & 4 deletions webapp/src/components/chat/ChatWindow.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,18 +149,38 @@ export const ChatWindow: React.FC = () => {
</>
)}
<TabList selectedValue={selectedTab} onTabSelect={onTabSelect}>
<Tab data-testid="chatTab" id="chat" value="chat">
<Tab data-testid="chatTab" id="chat" value="chat" aria-label="Chat Tab" title="Chat Tab">
Chat
</Tab>
<Tab data-testid="documentsTab" id="documents" value="documents">
<Tab
data-testid="documentsTab"
id="documents"
value="documents"
aria-label="Documents Tab"
title="Documents Tab"
>
Documents
</Tab>
{features[FeatureKeys.PluginsPlannersAndPersonas].enabled && (
<>
<Tab data-testid="plansTab" id="plans" value="plans" icon={<Map16Regular />}>
<Tab
data-testid="plansTab"
id="plans"
value="plans"
icon={<Map16Regular />}
aria-label="Plans Tab"
title="Plans Tab"
>
Plans
</Tab>
<Tab data-testid="personaTab" id="persona" value="persona" icon={<Person16Regular />}>
<Tab
data-testid="personaTab"
id="persona"
value="persona"
icon={<Person16Regular />}
aria-label="Persona Tab"
title="Persona Tab"
>
Persona
</Tab>
</>
Expand Down
10 changes: 5 additions & 5 deletions webapp/src/components/chat/chat-history/ChatHistoryItem.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) Microsoft. All rights reserved.

import { Persona, Text, makeStyles, mergeClasses, shorthands } from '@fluentui/react-components';
import { ThumbDislike24Filled, ThumbLike16Filled } from '@fluentui/react-icons';
import { ThumbDislikeFilled, ThumbLikeFilled } from '@fluentui/react-icons';
import React from 'react';
import { GetResponseOptions, useChat } from '../../../libs/hooks/useChat';
import { AuthorRoles, ChatMessageType, IChatMessage, UserFeedback } from '../../../libs/models/ChatMessage';
Expand Down Expand Up @@ -139,11 +139,11 @@ export const ChatHistoryItem: React.FC<ChatHistoryItemProps> = ({ message, getRe
{content}
{showShowRLHFMessage && <UserFeedbackActions messageIndex={messageIndex} />}
</div>
{showShowRLHFMessage && message.userFeedback === UserFeedback.Positive && (
<ThumbLike16Filled color="gray" />
{features[FeatureKeys.RLHF].enabled && message.userFeedback === UserFeedback.Positive && (
<ThumbLikeFilled color="gray" />
)}
{showShowRLHFMessage && message.userFeedback === UserFeedback.Negative && (
<ThumbDislike24Filled color="gray" />
{features[FeatureKeys.RLHF].enabled && message.userFeedback === UserFeedback.Negative && (
<ThumbDislikeFilled color="gray" />
)}
</div>
);
Expand Down
131 changes: 52 additions & 79 deletions webapp/src/components/chat/chat-list/ChatListItem.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,17 +1,6 @@
import {
makeStyles,
mergeClasses,
Persona,
Popover,
PopoverSurface,
PopoverTrigger,
shorthands,
Text,
tokens,
} from '@fluentui/react-components';
import { makeStyles, mergeClasses, Persona, shorthands, Text, tokens } from '@fluentui/react-components';
import { ShieldTask16Regular } from '@fluentui/react-icons';
import { FC, useState } from 'react';
import { Constants } from '../../../Constants';
import { useAppDispatch, useAppSelector } from '../../../redux/app/hooks';
import { RootState } from '../../../redux/app/store';
import { FeatureKeys } from '../../../redux/features/app/AppState';
Expand Down Expand Up @@ -121,77 +110,61 @@ export const ChatListItem: FC<IChatListItemProps> = ({

const time = timestampToDateString(timestamp);
return (
<Popover
openOnHover={!isSelected}
mouseLeaveDelay={0}
positioning={{
position: 'after',
autoSize: 'width',
}}
<div
className={mergeClasses(classes.root, isSelected && classes.selected)}
onClick={onClick}
title={`Chat: ${header}`}
aria-label={`Chat list item: ${header}`}
>
<PopoverTrigger disableButtonEnhancement>
<div className={mergeClasses(classes.root, isSelected && classes.selected)} onClick={onClick}>
<Persona
avatar={{ image: { src: botProfilePicture } }}
presence={
!features[FeatureKeys.SimplifiedExperience].enabled ? { status: 'available' } : undefined
}
/>
{editingTitle ? (
<EditChatName
name={header}
<Persona
avatar={{ image: { src: botProfilePicture } }}
presence={!features[FeatureKeys.SimplifiedExperience].enabled ? { status: 'available' } : undefined}
/>
{editingTitle ? (
<EditChatName
name={header}
chatId={id}
exitEdits={() => {
setEditingTitle(false);
}}
/>
) : (
<>
<div className={classes.body}>
<div className={classes.header}>
<Text className={classes.title} title={header}>
{header}
{features[FeatureKeys.AzureContentSafety].enabled && (
<ShieldTask16Regular className={classes.protectedIcon} />
)}
</Text>
{!features[FeatureKeys.SimplifiedExperience].enabled && (
<Text className={classes.timestamp} size={300}>
{time}
</Text>
)}
</div>
{showPreview && (
<>
{
<Text id={`message-preview-${id}`} size={200} className={classes.previewText}>
{preview}
</Text>
}
</>
)}
</div>
{showActions && (
<ListItemActions
chatId={id}
exitEdits={() => {
setEditingTitle(false);
chatName={header}
onEditTitleClick={() => {
setEditingTitle(true);
}}
/>
) : (
<>
<div className={classes.body}>
<div className={classes.header}>
<Text className={classes.title} title={header}>
{header}
{features[FeatureKeys.AzureContentSafety].enabled && (
<ShieldTask16Regular className={classes.protectedIcon} />
)}
</Text>
{!features[FeatureKeys.SimplifiedExperience].enabled && (
<Text className={classes.timestamp} size={300}>
{time}
</Text>
)}
</div>
{showPreview && (
<>
{
<Text
id={`message-preview-${id}`}
size={200}
className={classes.previewText}
>
{preview}
</Text>
}
</>
)}
</div>
{showActions && (
<ListItemActions
chatId={id}
chatName={header}
onEditTitleClick={() => {
setEditingTitle(true);
}}
/>
)}
</>
)}
</div>
</PopoverTrigger>
<PopoverSurface className={classes.popoverSurface}>
<Text weight="bold">{Constants.bot.profile.fullName}</Text>
<Text>{time}</Text>
</PopoverSurface>
</Popover>
</>
)}
</div>
);
};
4 changes: 3 additions & 1 deletion webapp/src/components/chat/chat-list/ChatListSection.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,10 @@ export const ChatListSection: React.FC<IChatListSectionProps> = ({ header, conve
const lastMessage = messages[convo.messages.length - 1];
const isSelected = id === selectedId;

/* Regex to match the Copilot timestamp format that is used as the default chat name.
The format is: 'Copilot @ MM/DD/YYYY, hh:mm:ss AM/PM'. */
const autoGeneratedTitleRegex =
/Copilot @ [0-9]{1,2}\/[0-9]{1,2}\/[0-9]{1,4}, [0-9]{2}:[0-9]{2}:[0-9]{2} [A,P]M/;
/Copilot @ [0-9]{1,2}\/[0-9]{1,2}\/[0-9]{1,4}, [0-9]{1,2}:[0-9]{1,2}:[0-9]{1,2} [A,P]M/;
const firstUserMessage = messages.find(
(message) => message.authorRole !== AuthorRoles.Bot && message.type === ChatMessageType.Message,
);
Expand Down
11 changes: 8 additions & 3 deletions webapp/src/components/chat/chat-list/ListItemActions.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,19 @@ export const ListItemActions: React.FC<IListItemActionsProps> = ({ chatId, chatN
return (
<div className={classes.root}>
<Tooltip content={'Edit chat name'} relationship="label">
<Button icon={<Edit />} appearance="transparent" aria-label="Edit" onClick={onEditTitleClick} />
<Button
icon={<Edit />}
appearance="transparent"
aria-label="Edit chat name"
onClick={onEditTitleClick}
/>
</Tooltip>
<Tooltip content={'Download chat session'} relationship="label">
<Button
disabled={!features[FeatureKeys.BotAsDocs].enabled}
icon={<ArrowDownload16 />}
appearance="transparent"
aria-label="Edit"
aria-label="Download chat session"
onClick={onDownloadBotClick}
/>
</Tooltip>
Expand All @@ -64,7 +69,7 @@ export const ListItemActions: React.FC<IListItemActionsProps> = ({ chatId, chatN
disabled={!features[FeatureKeys.MultiUserChat].enabled}
icon={<Share20 />}
appearance="transparent"
aria-label="Edit"
aria-label="Share live chat code"
onClick={() => {
setIsGettingInvitationId(true);
}}
Expand Down
5 changes: 4 additions & 1 deletion webapp/src/components/chat/persona/PromptEditor.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ const useClasses = makeStyles({
display: 'flex',
marginLeft: 'auto',
},
dialog: {
maxWidth: '25%',
},
});

// The number of rows in the textarea.
Expand Down Expand Up @@ -89,7 +92,7 @@ export const PromptEditor: React.FC<PromptEditorProps> = ({
<PopoverTrigger disableButtonEnhancement>
<Button icon={<Info16 />} appearance="transparent" />
</PopoverTrigger>
<PopoverSurface>{info}</PopoverSurface>
<PopoverSurface className={classes.dialog}>{info}</PopoverSurface>
</Popover>
</div>
<Textarea
Expand Down
5 changes: 4 additions & 1 deletion webapp/src/components/chat/shared/EditChatName.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ export const EditChatName: React.FC<IEditChatNameProps> = ({ name, chatId, exitE
handleSave();
}
};

return (
<div
className={classes.root}
Expand All @@ -87,8 +88,10 @@ export const EditChatName: React.FC<IEditChatNameProps> = ({ name, chatId, exitE
flexDirection: `${textButtons ? 'column' : 'row'}`,
gap: `${textButtons ? tokens.spacingVerticalS : tokens.spacingVerticalNone}`,
}}
title={'Edit chat name'}
aria-label={`Edit chat name for "${name}"`}
>
<Input value={title} onChange={onTitleChange} id={title} onKeyDown={handleKeyDown} />
<Input value={title} onChange={onTitleChange} id={`input-${chatId}`} onKeyDown={handleKeyDown} autoFocus />
{textButtons && (
<div className={mergeClasses(classes.buttons, classes.textButtons)}>
<Button appearance="secondary" onClick={onClose}>
Expand Down
2 changes: 2 additions & 0 deletions webapp/src/components/open-api-plugins/PluginGallery.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ export const PluginGallery: React.FC = () => {
style={{ color: 'white' }}
appearance="transparent"
icon={<AppsAddIn24 color="white" />}
title="Plugins Gallery"
aria-label="Plugins Gallery"
>
Plugins
</Button>
Expand Down
9 changes: 1 addition & 8 deletions webapp/src/redux/features/app/AppState.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ export const Features = {
label: 'Reinforcement Learning from Human Feedback',
description: 'Enable users to vote on model-generated responses. For demonstration purposes only.',
// TODO: [Issue #42] Send and store feedback in backend
inactive: true,
},
[FeatureKeys.DeleteChats]: {
enabled: false,
Expand Down Expand Up @@ -124,13 +123,7 @@ export const Settings = [
];

export const initialState: AppState = {
alerts: [
{
message:
'By using Chat Copilot, you agree to protect sensitive data, not store it in chat, and allow chat history collection for service improvements. This tool is for internal use only.',
type: AlertType.Info,
},
],
alerts: [],
tokenUsage: {},
features: Features,
settings: Settings,
Expand Down

0 comments on commit 6d5ba70

Please sign in to comment.