Added example questions to AI chat#12747
Conversation
|
Please keep in mind the contributing guidelines. |
|
@palukku Thank you for helping with reviews. |
|
JabRef supports both light and dark modes, so please ensure that the colors are appropriate for both. The current text color might be too light, reducing readability, and the hover state is also not very legible.
|
# 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
| <ChatHistoryComponent fx:id="uiChatHistory" VBox.vgrow="ALWAYS" fitToWidth="true"/> | ||
| </Loadable> | ||
|
|
||
| <HBox spacing="10" alignment="CENTER"> |
There was a problem hiding this comment.
@palukku added external css styles and remove inline css styles
| @FXML private Button notificationsButton; | ||
| @FXML private ChatPromptComponent chatPrompt; | ||
| @FXML private Label noticeText; | ||
| @FXML private Hyperlink exQuestion1; |
|
@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. |
palukku
left a comment
There was a problem hiding this comment.
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 |
|
I now remember why we did not hard-code system prompts in the AI preferences: Because of internationalisation. Hard-coded prompts necessitate for JabRef maintainers to translate them to other languages, if the community calls for it, whereas user configurable prompts give users the possibility to use their language of choice. |
InAnYan
left a comment
There was a problem hiding this comment.
Awesome!
Left some small review comments
|
|
||
| <HBox spacing="10" alignment="CENTER" fx:id="exQuestionBox"> | ||
| <Label text="Try with examples" BorderPane.alignment="CENTER"/> | ||
| <Hyperlink fx:id="exQuestion1" text="What is the goal of the paper?" BorderPane.alignment="CENTER" styleClass="exampleQuestionStyle"/> |
There was a problem hiding this comment.
Thanks for introducing those questions.
Can you make them localized?
I think the proper way is to do this:
- Leave
textempty in FXML. - Then in some initialization method in Java class, call
setTextwithLocalization.lang("...").
There was a problem hiding this comment.
Can't you just use the same method as for the "Try with examples" Label by adding a % in front? Or aren't they detected by the localization tests?
| </Loadable> | ||
|
|
||
| <HBox spacing="10" alignment="CENTER" fx:id="exQuestionBox"> | ||
| <Label text="Try with examples" BorderPane.alignment="CENTER"/> |
There was a problem hiding this comment.
This should be localized with %.
See more at https://devdocs.jabref.org/code-howtos/localization.html#localization-in-fxml.
| } | ||
|
|
||
| private void initializeExampleQuestions() { | ||
| exQuestion1.setText(Localization.lang("What is the goal of the paper?")); |
There was a problem hiding this comment.
The strings for example questions should be defined as constants to avoid magic strings and improve maintainability.
There was a problem hiding this comment.
Yeah, private static final String ....
There was a problem hiding this comment.
@koppor This is a false positive. For l10n this doesn't work
There was a problem hiding this comment.
I was thinking about
private static final String EXAMPLE_QUESTION_1 = Localization.lang("What is the goal of the paper?"));
on top of the class - to make it very easy to update the example question.
I also can follow the line of argumetnation that find-in-all-files also finds the string and then can be replaced.
There was a problem hiding this comment.
Use the String you want to localize directly, do not use members or local variables: Localization.lang("Translate me"); instead of Localization.lang(someVariable) (possibly in the form someVariable = Localization.lang("Translate me")
https://devdocs.jabref.org/code-howtos/localization.html
There was a problem hiding this comment.
maybe I confused it with that one
|
@kaushikaW Please merge |
| addExampleQuestionAction(exQuestion3); | ||
| } | ||
|
|
||
| private void addExampleQuestionAction(Hyperlink hyperlink) { |
There was a problem hiding this comment.
The method addExampleQuestionAction contains nested logic that could be simplified by returning early when the hyperlink text is already in the chat history, following the fail-fast principle.
|
@trag-bot didn't find any issues in the code! ✅✨ |
|
@InAnYan why this PR filing do I have mentioned wrong issue no in the PR description. |
| -fx-underline: false !important; | ||
| -fx-text-fill: -fx-text-base-color; | ||
| } | ||
| .exampleQuestionStyle:hover { |
|
|
||
| private final ObservableList<Notification> notifications = FXCollections.observableArrayList(); | ||
|
|
||
|
|
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.mddescribed in a way that is understandable for the average user (if change is visible to the user)