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

refactor(webview): remove mention and add intent selector to toolbar #7287

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ describe('HumanMessageEditor', () => {

describe('states', () => {
function expectState(
{ addContextButton, submitButton }: ReturnType<typeof renderWithMocks>,
{ toolbar, submitButton }: ReturnType<typeof renderWithMocks>,
expected: {
toolbarVisible?: boolean
submitButtonVisible?: boolean
Expand All @@ -33,7 +33,7 @@ describe('HumanMessageEditor', () => {
}
): void {
if (expected.toolbarVisible !== undefined) {
notUnless(expect.soft(addContextButton), expected.toolbarVisible).toBeVisible()
notUnless(expect.soft(toolbar), expected.toolbarVisible).toBeVisible()
}
if (expected.submitButtonVisible !== undefined) {
notUnless(expect.soft(submitButton), expected.submitButtonVisible).toBeVisible()
Expand Down Expand Up @@ -149,7 +149,7 @@ type EditorHTMLElement = HTMLDivElement & {
function renderWithMocks(props: Partial<ComponentProps<typeof HumanMessageEditor>>): {
container: HTMLElement
editor: EditorHTMLElement
addContextButton: HTMLElement | null
toolbar: HTMLElement | null
submitButton: HTMLElement | null
onChange: Mock
onSubmit: Mock
Expand Down Expand Up @@ -180,10 +180,7 @@ function renderWithMocks(props: Partial<ComponentProps<typeof HumanMessageEditor
return {
container,
editor: container.querySelector<EditorHTMLElement>('[data-lexical-editor="true"]')!,
addContextButton: screen.queryByRole('button', {
name: 'Add context',
hidden: true,
}),
toolbar: container.querySelector('[data-testid="chat-editor-toolbar"]'),
submitButton: screen.queryByRole('button', {
name: /send|stop/i,
hidden: true,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,191 @@
import type { ChatMessage } from '@sourcegraph/cody-shared'
import { CodyIDE } from '@sourcegraph/cody-shared'
import { BetweenHorizonalEnd, InfoIcon, MessageSquare, Pencil, Search } from 'lucide-react'
import type { FC } from 'react'
import { useCallback, useMemo } from 'react'
import { Kbd } from '../../../../../../components/Kbd'
import { Badge } from '../../../../../../components/shadcn/ui/badge'
import { Command, CommandItem, CommandList } from '../../../../../../components/shadcn/ui/command'
import { ToolbarPopoverItem } from '../../../../../../components/shadcn/ui/toolbar'
import { cn } from '../../../../../../components/shadcn/utils'
import { useConfig } from '../../../../../../utils/useConfig'

export enum IntentEnum {
Chat = 'Chat',
Search = 'Search',
Edit = 'Edit',
Insert = 'Insert',
}

// Mapping between ChatMessage intent and IntentEnum for faster lookups
export const INTENT_MAPPING: Record<string, IntentEnum> = {
chat: IntentEnum.Chat,
search: IntentEnum.Search,
edit: IntentEnum.Edit,
insert: IntentEnum.Insert,
}

interface IntentOption {
title: string | React.ReactElement
icon: React.FC<{ className?: string }>
intent: ChatMessage['intent']
shortcut?: React.ReactNode
hidden?: boolean
disabled?: boolean
}

const defaultIntent: IntentOption = {
title: 'Run as chat',
icon: MessageSquare,
intent: 'chat',
shortcut: <Kbd macOS="return" linuxAndWindows="return" />,
}

// Memoize the enterprise badge and keyboard shortcuts to avoid recreating React elements
const ENTERPRISE_BADGE = (
<Badge>
Enterprise <InfoIcon className="tw-size-4 tw-ml-1" />
</Badge>
)

const SEARCH_SHORTCUT = (
<>
<Kbd macOS="cmd" linuxAndWindows="ctrl" />
<Kbd macOS="opt" linuxAndWindows="alt" />
<Kbd macOS="return" linuxAndWindows="return" />
</>
)

// Optimization: memoize search title to avoid recreation
const SEARCH_TITLE = (
<span className="tw-inline-flex tw-items-center tw-gap-4">
<span>Run as search</span>
<Badge>Beta</Badge>
</span>
)

function getIntentOptions({
isCodyWeb,
isDotComUser,
omniBoxEnabled,
}: {
isCodyWeb: boolean
isDotComUser: boolean
omniBoxEnabled: boolean
}): IntentOption[] {
return [
defaultIntent,
{
title: SEARCH_TITLE,
icon: Search,
intent: 'search',
hidden: !omniBoxEnabled,
disabled: isDotComUser,
shortcut: isDotComUser ? ENTERPRISE_BADGE : SEARCH_SHORTCUT,
},
{
title: 'Edit Code',
icon: Pencil,
intent: 'edit',
hidden: true,
Copy link
Member

Choose a reason for hiding this comment

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

IMO lets not hide Edit Code as an option. Cursor has it. It is particularly useful for non-agentic edits.

disabled: isCodyWeb,
},
{
title: 'Insert Code',
icon: BetweenHorizonalEnd,
intent: 'insert',
hidden: true,
disabled: isCodyWeb,
},
]
}

export const ModeSelectorField: React.FunctionComponent<{
omniBoxEnabled: boolean
intent: ChatMessage['intent']
className?: string
manuallySelectIntent: (intent?: ChatMessage['intent']) => void
}> = ({ className, intent, omniBoxEnabled, manuallySelectIntent }) => {
const {
clientCapabilities: { agentIDE },
isDotComUser,
} = useConfig()

const intentOptions = useMemo(
() =>
getIntentOptions({
isCodyWeb: agentIDE === CodyIDE.Web,
isDotComUser,
omniBoxEnabled,
}).filter(option => !option.hidden),
[agentIDE, isDotComUser, omniBoxEnabled]
)

const displayedIntent = INTENT_MAPPING[intent || 'chat'] || IntentEnum.Chat
Copy link
Member

Choose a reason for hiding this comment

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

Isn't the display text just title case of ChatMessage['intent']? Why to make it complex by introducing a whole new IntentEnum type and an INTENT_MAPPING.

This could simply be:

const displayedIntent = _.capitalize(intent || 'chat')


// Memoize the handler to avoid recreating on each render
const handleItemClick = useCallback(
(close: () => void) => (item: ChatMessage['intent']) => {
manuallySelectIntent(item)
close()
},
[manuallySelectIntent]
)

return (
<ToolbarPopoverItem
role="combobox"
iconEnd="chevron"
className={cn('tw-justify-between', className)}
tooltip="Select a mode"
aria-label="Select mode"
popoverContent={close => (
<div className="tw-flex tw-flex-col tw-max-h-[500px] tw-overflow-auto">
<ModeList onClick={handleItemClick(close)} intentOptions={intentOptions} />
</div>
)}
popoverContentProps={{
className: 'tw-min-w-[200px] tw-w-[75vw] tw-max-w-[300px] !tw-p-0',
onCloseAutoFocus: event => {
event.preventDefault()
},
}}
>
{displayedIntent}
</ToolbarPopoverItem>
)
}

export const ModeList: FC<{
onClick: (intent?: ChatMessage['intent']) => void
intentOptions: IntentOption[]
}> = ({ onClick, intentOptions }) => {
// Create a memoized handler for each item to prevent unnecessary recreations
const createItemHandler = useCallback(
Copy link
Member

@thenamankumar thenamankumar Mar 3, 2025

Choose a reason for hiding this comment

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

Not that there is a need to memoize every single smallish element in a list of 3 options. But this won't work as intended as per my understanding.

While createItemHandler is memoized itself, every call to createItemHandler always returns a new function. So onSelect on CommandItem always gets a new function on each render.

Thus rendering createItemHandler to be redundant.

(intent: ChatMessage['intent']) => () => {
onClick(intent)
},
[onClick]
)

return (
<Command>
<CommandList className="tw-p-2">
{intentOptions.map(option => (
<CommandItem
key={option.intent || 'auto'}
onSelect={createItemHandler(option.intent)}
disabled={option.disabled}
className="tw-flex tw-text-left tw-justify-between tw-rounded-sm tw-cursor-pointer tw-px-4"
>
<div className="tw-flex tw-gap-4">
<option.icon className="tw-size-8 tw-mt-1" />
{option.title}
</div>
{option.shortcut && <div className="tw-flex tw-gap-2">{option.shortcut}</div>}
</CommandItem>
))}
</CommandList>
</Command>
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ const meta: Meta<typeof SubmitButton> = {
state: args.state === 'submittable' ? 'waitingResponseComplete' : 'submittable',
})
}}
manuallySelectIntent={() => {}}
/>
)
},
Expand Down
Loading
Loading