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.

Sign up for GitHub

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

feat: up/down arrow navigation now only triggers when cursor is at the start or end of multiline inputs #208

Merged
merged 1 commit into from
Dec 20, 2024
Merged

feat: up/down arrow navigation now only triggers when cursor is at the start or end of multiline inputs #208

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
20 changes: 20 additions & 0 deletions src/components/chat-item/chat-prompt-input.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,26 @@ export class ChatPromptInput {
this.quickPickOpen = true;
}
} else if (navigationalKeys.includes(e.key)) {
const inputText = this.promptTextInput.getTextInputValue();
const cursorPosition = this.promptTextInput.getCursorPos();

const textBeforeCursor = inputText.substring(0, cursorPosition);
const textAfterCursor = inputText.substring(cursorPosition);

// Check if cursor is on the first line
const isFirstLine = !textBeforeCursor.includes('\n');

// Check if cursor is on the last line
const isLastLine = !textAfterCursor.includes('\n');
Comment on lines +333 to +337
Copy link
Contributor

@francescoopiccoli francescoopiccoli Dec 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

just to double check, is it safe to deduce first/last line by checking for \n? i am thinking of some edge cases with input (maybe being copy pasted) starting/ending with empty lines

Copy link
Contributor

@francescoopiccoli francescoopiccoli Dec 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I did some more testing, and I think it is not a safe deduction to check for '\n' to deduce first/last line, you can write on multiple lines without using new lines (\n). To fix this we might need to check the height of the input field or count the n of lines somehow, something roughly in this style, even though it looks a bit hacky and it might be tricky in certain edge cases :

// get the height of the prompt text input
const promptTextInputHeight = this.promptTextInput.render.offsetHeight;
// avg line height
const avgLineHeight = 20;
// derive an estimate avg n of lines 
const estimatedLines = Math.ceil(promptTextInputHeight / avgLineHeight);
// count n of chars 
const inputTextNofChars = inputText.length;
// derive an estimate avg n of chars per line
const avgCharsPerLine = Math.ceil(inputTextNofChars / estimatedLines);
// derive line in which we are based on cursorPosition and avgLines
const line = Math.ceil(cursorPosition / avgCharsPerLine);
// define isFirstLine
const isFirstLine = line === 0;
// define isLastLine
const isLastLine = line === estimatedLines - 1;

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agree with Francesco.


// Only allow up/down arrow navigation when user reaches the end of multiline input
if (e.key === KeyMap.ARROW_UP && !isFirstLine) {
return;
}
if (e.key === KeyMap.ARROW_DOWN && !isLastLine) {
return;
}

if (this.userPromptHistoryIndex === -1 || this.userPromptHistoryIndex === this.userPromptHistory.length) {
this.lastUnsentUserPrompt = {
inputText: this.promptTextInput.getTextInputValue(),
Expand Down
Loading