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

Prevent sending empty message #126

Merged
merged 3 commits into from
Dec 18, 2024
Merged

Conversation

brichet
Copy link
Collaborator

@brichet brichet commented Dec 16, 2024

Fixes #118

Port jupyterlab/jupyter-ai#946

EDIT

  • prevent sending empty message (only whitespace) with keyboard and button
  • remove condition on 'cancel' button to disable it

@brichet brichet added the bug Something isn't working label Dec 16, 2024
Copy link
Contributor

Binder 👈 Launch a Binder on branch brichet/jupyter-chat/do_not_send_empty

@brichet brichet changed the title Don't send empty message Prevent sending empty message Dec 16, 2024
Copy link
Member

@dlqqq dlqqq left a comment

Choose a reason for hiding this comment

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

@brichet Thanks! One minor comment below.

Comment on lines 139 to 144
// Do not send empty messages, and avoid adding new line in empty message.
if (!input.trim()) {
event.stopPropagation();
event.preventDefault();
return;
}
Copy link
Member

Choose a reason for hiding this comment

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

This handler only sends the message if !input.trim(), but the SendButton component is enabled by a different condition (when input.length >= 0).

<SendButton
  model={model}
  sendWithShiftEnter={sendWithShiftEnter}
  inputExists={input.length > 0}
  onSend={onSend}
  hideIncludeSelection={hideIncludeSelection}
  hasButtonOnLeft={!!props.onCancel}
/>

Currently, if the input contains only white space, the send button will be enabled (which shouldn't happen), but pressing Enter/Shift-Enter does nothing. This is a difference in behavior caused by using two separate state variables.

I recommend defining a new local variable const inputExists = !!input.trim() in the component's top scope, and using that variable in both the handler & the send button.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch, thanks

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated in b28a698

@brichet
Copy link
Collaborator Author

brichet commented Dec 18, 2024

The condition to disable the button was also applied to the 'cancel' button (visible when editing a message).
I removed this condition, since an edition should always be cancellable, AFAIK.

Copy link
Member

@dlqqq dlqqq left a comment

Choose a reason for hiding this comment

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

@brichet TY! Great work

@brichet brichet merged commit 4b0bbdc into jupyterlab:main Dec 18, 2024
12 checks passed
@brichet brichet deleted the do_not_send_empty branch December 18, 2024 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working @jupyter/chat
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Keyboard sends empty messages when input is empty
2 participants