-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Added example questions to AI chat #12747
base: main
Are you sure you want to change the base?
Conversation
src/main/java/org/jabref/gui/ai/components/aichat/AiChatComponent.java
Outdated
Show resolved
Hide resolved
Please keep in mind the contributing guidelines. |
@palukku Thank you for helping with reviews. |
src/main/java/org/jabref/gui/ai/components/aichat/AiChatComponent.fxml
Outdated
Show resolved
Hide resolved
src/main/java/org/jabref/gui/ai/components/aichat/AiChatComponent.java
Outdated
Show resolved
Hide resolved
# Conflicts: # src/main/java/org/jabref/gui/Base.css # src/main/java/org/jabref/gui/ai/components/aichat/AiChatComponent.fxml # src/main/java/org/jabref/gui/ai/components/aichat/AiChatComponent.java
</Loadable> | ||
|
||
<HBox spacing="10" alignment="CENTER"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@palukku added external css styles and remove inline css styles
@@ -62,6 +63,9 @@ public class AiChatComponent extends VBox { | |||
@FXML private Button notificationsButton; | |||
@FXML private ChatPromptComponent chatPrompt; | |||
@FXML private Label noticeText; | |||
@FXML private Hyperlink exQuestion1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@palukku added inline FXML annotations
@palukku @subhramit Do I need to add UI changes I done in this issue to the CHANGELOG.md file |
No, just the summary should be fine. Add it to the changelog. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your code looks fine now and works as intended.
However, one thing is missing in my opinion: the example questions are always displayed. It would be nice to check the history; if a question has already been sent, remove its "button" from the box. Once all example questions have been asked, remove the entire example box.
You can use chatPrompt#getHistory to check that and remove the hyperlinks by obtaining the parent HBox and removing them from its children.
Well I also though of your opinion , but removing each example question as it's used creates a small issue—once all questions are gone, the user is cant see any example questions . is it ok |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made some adjustments to the style to better blend with JabRef's existing design. Additionally, I included the removal of buttons after use. Since the chat isn't persistent and varies across entries, we can safely remove each button individually after the corresponding question is asked. When JabRef is reopened, the chat will be empty again, and the suggested questions will reappear.
<HBox spacing="10" alignment="CENTER"> | ||
<Label | ||
fx:id="exQuestionLabel" | ||
text="Try with examples" | ||
BorderPane.alignment="CENTER" | ||
styleClass="exampleQuestionLabelStyle" | ||
/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<HBox spacing="10" alignment="CENTER"> | |
<Label | |
fx:id="exQuestionLabel" | |
text="Try with examples" | |
BorderPane.alignment="CENTER" | |
styleClass="exampleQuestionLabelStyle" | |
/> | |
<HBox fx:id="exQuestionBox" spacing="10" alignment="CENTER"> | |
<Label | |
text="Try with examples" | |
BorderPane.alignment="CENTER" | |
/> |
@@ -62,6 +63,9 @@ public class AiChatComponent extends VBox { | |||
@FXML private Button notificationsButton; | |||
@FXML private ChatPromptComponent chatPrompt; | |||
@FXML private Label noticeText; | |||
@FXML private Hyperlink exQuestion1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@FXML private Hyperlink exQuestion1; | |
@FXML private HBox exQuestionBox; | |
@FXML private Hyperlink exQuestion1; |
private void sendExampleQuestions() { | ||
exQuestion1.setOnAction(event -> { | ||
onSendMessage(exQuestion1.getText()); | ||
}); | ||
|
||
exQuestion2.setOnAction(event -> { | ||
onSendMessage(exQuestion2.getText()); | ||
}); | ||
|
||
exQuestion3.setOnAction(event -> { | ||
onSendMessage(exQuestion3.getText()); | ||
}); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
private void sendExampleQuestions() { | |
exQuestion1.setOnAction(event -> { | |
onSendMessage(exQuestion1.getText()); | |
}); | |
exQuestion2.setOnAction(event -> { | |
onSendMessage(exQuestion2.getText()); | |
}); | |
exQuestion3.setOnAction(event -> { | |
onSendMessage(exQuestion3.getText()); | |
}); | |
} | |
private void sendExampleQuestions() { | |
addExampleQuestionAction(exQuestion1); | |
addExampleQuestionAction(exQuestion2); | |
addExampleQuestionAction(exQuestion3); | |
} | |
private void addExampleQuestionAction(Hyperlink hyperlink) { | |
if (chatPrompt.getHistory().contains(hyperlink.getText())) { | |
exQuestionBox.getChildren().remove(hyperlink); | |
if (exQuestionBox.getChildren().size() == 1) { | |
this.getChildren().remove(exQuestionBox); | |
} | |
return; | |
} | |
hyperlink.setOnAction(event -> { | |
onSendMessage(hyperlink.getText()); | |
exQuestionBox.getChildren().remove(hyperlink); | |
if (exQuestionBox.getChildren().size() == 1) { | |
this.getChildren().remove(exQuestionBox); | |
} | |
}); | |
} |
@@ -10,6 +10,7 @@ | |||
import javafx.collections.ObservableList; | |||
import javafx.fxml.FXML; | |||
import javafx.scene.control.Button; | |||
import javafx.scene.control.Hyperlink; | |||
import javafx.scene.control.Label; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import javafx.scene.control.Label; | |
import javafx.scene.control.Label; | |
import javafx.scene.layout.HBox; |
this is needed for my code suggestion to work properly.
0d519e5
to
f1c8a10
Compare
@kaushikaW please take the latest pull from upstream, resolve merge conflicts and push a single commit for the merge to the updated state. Do not force-push. |
ok sure sorry for mistake |
Hi @palukku I go through your changes and added them to the code thanks for suggestions ! |
No need to state this. This increases load of maintainers. ALL maintainers get a notification of your message. They need to switch context etc. It is clear that you will work on that. The only reason I see for commenting is that you say: I don't have time now, but will commit something one week :) |
I apologize for responding so late. Will it still be possible to regenerate one of those questions? LLMs are not precise and tend to confabulate, so regeneration is one of the main use cases as it helps for brainstorming. I have not tried the PR yet, as I don't have much time and will not have much time in the coming weeks. |
Directly regenerating the example buttons is not possible, but if you're not happy with the answer you are free to ask the same question manually again or try to ask for a better answer and why the first response is not the one you expected. I don't see it as an issue for this pr since it's only a suggestion of things you can ask. But an "regenerate response" Button next to the answer of the ai would be an idea, maybe we can open up a new issue for this. |
As said here #12702 (comment) this would be a great feature to use ai to create (following) questions that are context aware. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried it out, works fine with both dark and light style.
|
||
|
||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove these extra newlines
@@ -270,3 +300,4 @@ private void deleteLastMessage() { | |||
} | |||
} | |||
} | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Revert this as well
if (exQuestionBox.getChildren().size() == 1) { | ||
this.getChildren().remove(exQuestionBox); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code duplication exists in two places where the same condition and removal logic is repeated. Should be extracted to a separate method.
Oh and please add the changes in short and user friendly words to the CHANGELOG.md! |
Fixes #12702
Added 3 example questions to the AI chat tab section (see the screenshot).
Currently, the 3 questions are hardcoded in the codebase and are not being generated by the AI model.
Some extra styles have been added to improve the user experience.
Mandatory checks
CHANGELOG.md
described in a way that is understandable for the average user (if change is visible to the user)