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

Add up down arrow navigation in prompt field to access prompts in history #180

Closed

Conversation

francescoopiccoli
Copy link
Contributor

@francescoopiccoli francescoopiccoli commented Nov 22, 2024

Problem

Up and down keys inside the prompt input field should switch between previous prompts like in a terminal. To increase the UX and let the end user easily access to a previous prompt to modify/improve it, the approach used in terminals needs to be applied.

Once I receive validation for my implementation approach I will proceed with adding tests and documentation.

Solution

Retrieving and identifying commands and code snippets from previous chat prompts, required adding some extra properties and methods that were not current available.

Tests

  • I have tested this change on VSCode
  • I have tested this change on JetBrains

License

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@@ -311,6 +319,8 @@ export interface ChatPrompt {
escapedPrompt?: string;
command?: string;
context?: string[];
attachmentContent?: string;
inputText?: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this in comparison to prompt?

Copy link
Collaborator

Choose a reason for hiding this comment

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

It is not so preferable to add these to typedef, if we're not planning to send it back to consumer. Since consumer uses this and they might expect to have an input text and attachment content. Instead, create a private interface extending from ChatPrompt for your use case

@@ -245,6 +250,9 @@ export interface TreeNodeDetails {

export interface ChatItemContent {
body?: string | null;
inputText?: string | null;
Copy link
Contributor

Choose a reason for hiding this comment

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

Just an idea; might be nice to bundle these 3 under something like inputs: {}

src/static.ts Outdated
/**
* The navigation index for the chat prompts list for the up/down arrow key navigation
*/
navigationIndexChatPrompts?: number;
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe change to upDownKeyNavigationIndex for consistency with UP_DOWN_ARROW_KEY_PRESS naming

Copy link
Collaborator

Choose a reason for hiding this comment

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

We should already have them in the keys list.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should already have them in the keys list.

sorry I didnt get this, not sure I know what is the key list and what you mean with 'them'?

example/src/main.ts Outdated Show resolved Hide resolved
example/src/main.ts Outdated Show resolved Hide resolved
example/src/main.ts Outdated Show resolved Hide resolved
src/components/chat-item/chat-prompt-input.ts Outdated Show resolved Hide resolved
upDownArrowKeyPress?: (
tabId: string,
direction: 'up' | 'down',
eventId?: string) => void;
Copy link
Contributor

Choose a reason for hiding this comment

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

huh? what is string), isn't there a parenthesis missing? (same for line 248)

Copy link
Collaborator

Choose a reason for hiding this comment

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

where is the open? :)

Copy link
Contributor Author

@francescoopiccoli francescoopiccoli Nov 25, 2024

Choose a reason for hiding this comment

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

Not sure I got this. Maybe its just a formatting on github review? or a commit by commit review?
Here's what I see, I do not see any parenthesis missing 🙈

onFileActionClick?: (
    tabId: string,
    messageId: string,
    filePath: string,
    actionName: string,
    eventId?: string) => void;
 onTabBarButtonClick?: (
    tabId: string,
    buttonId: string,
    eventId?: string) => void;
 onUpDownArrowKeyPress?: (
    tabId: string,
    direction: 'up' | 'down',
    eventId?: string) => void;

@francescoopiccoli francescoopiccoli marked this pull request as ready for review November 25, 2024 10:15
@francescoopiccoli francescoopiccoli requested a review from a team as a code owner November 25, 2024 10:15
@francescoopiccoli francescoopiccoli deleted the frapicc/up-down-arrow-prompt-navigation branch November 27, 2024 09:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants