Use Markdown in AI Summary tab implemented #12266

mulla028 · 2024-12-05T23:47:56Z

Description (UPDATED)

As it was pre-requested, I've implemented an option when user is able, using checkbox, to change the formation of the AI Summary. If user checks it, WebView with formatted into html markdown appears, otherwise text stays raw. It is pretty useful new feature that makes AI Summary user-friendly for those, who loves to see formatted text, instead of raw markdown that besides the text contains "weird", in user's opinion characters.

How Code Changed and Why?

In the issue discussion, I asked questions, and maintainers came up with an idea that we need a simple toggle, so I decided to add into org/jabref/gui/ai/components/summary/SummaryShowingComponent.fxml a CheckBox that appears one the left below the AI Summary
This toggle handled in org/jabref/gui/ai/components/summary/SummaryShowingComponent.javafile's method onMarkdownToggle(). Once user checks the CheckBox the TextArea disappears, and WebView come us.
TextArea was wrapped in StackPane to simplify managing and layering of that component.
WebView programmatically implemented (lazy initialization) to avoid findMissingLocalizationKeys and findObsoleteLocalizationKeys tests failure.
To formate raw text into markdown I used org/jabref/logic/layout/format/MarkdownFormatter.java that has been already implemented.

How to Copy Formatted Content?

Press anywhere in WebView component press ctrl + a/cmd + a then ctrl + c/cmd + c, and paste it in the desired place ^_^

Screenshots

Markdown CheckBox checked:

Markdown CheckBox unchecked:

Mandatory checks

I own the copyright of the code submitted and I licence it under the MIT license
Change in CHANGELOG.md described in a way that is understandable for the average user (if change is visible to the user)
Tests created for changes (if applicable)
Manually tested changed features in running JabRef (always required)
Screenshots added in PR description (for UI changes)
Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

…handle checkbox changes

github-actions

JUnit tests are failing. In the area "Some checks were not successful", locate "Tests / Unit tests (pull_request)" and click on "Details". This brings you to the test output.

You can then run these tests in IntelliJ to reproduce the failing tests locally. We offer a quick test running howto in the section Final build system checks in our setup guide.

calixtus · 2024-12-06T01:28:07Z

Well, right now the checkbox looks quite ugly. Please place it somewhere else...

calixtus · 2024-12-06T01:32:29Z

It's not good style to mix fxmls and hard coded creation of ui elements except in some edge cases. Please place both the text area and the Web view in a StackPane.

mulla028 · 2024-12-06T12:36:57Z

It's not good style to mix fxmls and hard coded creation of ui elements except in some edge cases. Please place both the text area and the Web view in a StackPane.

I tried not to mix them, but I came up with hard coded WebView, otherwise it wasn't passing unit tests

subhramit · 2024-12-07T17:58:34Z

I tried not to mix them, but I came up with hard coded WebView, otherwise it wasn't passing unit tests

Which unit tests?

mulla028 · 2024-12-07T18:04:31Z

I tried not to mix them, but I came up with hard coded WebView, otherwise it wasn't passing unit tests

Which unit tests?

In LocalizationConsistencyTest two tests didn't pass using WebView via .fxml: findObsoleteLocalizationKeys() and findMissingLocalizationKeys(). However, adding WebView to the UI programmatically, all tests pass.

subhramit · 2024-12-07T18:23:30Z

In LocalizationConsistencyTest two tests didn't pass using WebView via .fxml: findObsoleteLocalizationKeys() and findMissingLocalizationKeys(). However, adding WebView to the UI programmatically, all tests pass.

Revert it, let us take a look. We'll help you with the tests.

mulla028 · 2024-12-07T18:44:47Z

@subhramit just reverted it, you will witness that 2 unit-tests are not going to pass

github-actions

JUnit tests are failing. In the area "Some checks were not successful", locate "Tests / Unit tests (pull_request)" and click on "Details". This brings you to the test output.

You can then run these tests in IntelliJ to reproduce the failing tests locally. We offer a quick test running howto in the section Final build system checks in our setup guide.

subhramit

I have requested a few changes.

For the failing localization tests, go through:
https://devdocs.jabref.org/code-howtos/faq.html#orgjabreflogicl10nlocalizationconsistencytest-findmissinglocalizationkeys-failed and the one below it.

Please also avoid force-pushing your changes.

src/main/java/org/jabref/gui/ai/components/summary/SummaryShowingComponent.fxml

src/main/java/org/jabref/gui/ai/components/summary/SummaryShowingComponent.java

src/main/java/org/jabref/gui/ai/components/summary/SummaryShowingComponent.fxml

calixtus · 2024-12-07T19:09:59Z

I thought about it a bit. It is actually not the worst idea to lazy load the WebView for performance reasons. Please place the TextArea inside a StackPane and add the WebView on demand. That way all the managed and visible stuff can be avoided.

calixtus · 2024-12-07T19:10:46Z

And please move.the checkbox somewhere else.

mulla028 · 2024-12-07T20:14:03Z

And please move.the checkbox somewhere else.

Before I forced push, the previous commit had the changed position of CheckBox

koppor · 2024-12-08T08:29:46Z

And please move.the checkbox somewhere else.

Before I forced push, the previous commit had the changed position of CheckBox

Therefore, no force push.

Either try to resurrect the commit with "git reflog" or try to re-do it.

mulla028 · 2024-12-09T05:45:38Z

@koppor
Reset commit. Sorry for doing it, I can see how it's driven everyone crazy 😆

… | Lazy initialization of WebView implemented

mulla028 · 2024-12-09T06:45:20Z

@calixtus
Now, I think it looks way better. CheckBox doesn't seam that ugly(check updated description). May be you have any suggestions on it?

InAnYan

Great! Summary looks so much nicer now!

I was thinking: what if we could get rid of TextArea whatsoever?

Can you implement only one WebView that would have different HTMLs: one in plain mode, and other is rendered Markdown? This would greatly simplify the logic.

I also noticed 3 small problems:

Text is not wrapped.
WebView does not shrink.
Text is not styled with JabRef's main font. Can you look at how it's implemented in entry preview? The default font is Sans, but the summary in WebView is Serif.

src/main/java/org/jabref/gui/ai/components/summary/SummaryShowingComponent.java

mulla028 · 2024-12-09T23:18:13Z

@InAnYan
I don't know how to make a plain text shrink there. However, Markdown is getting shrink well!

InAnYan

Good, you used WebView for both plain-text mode and rendered Markdown, looks better (however, there is a slight remain of TextArea 😄). Overall, I see these problems:

It doesn't wrap text in plain text mode. That's strange, because in rendered Markdown mode there is some wrapping (maybe, because of the <pre> tag?)
In rendered Markdown mode, it wraps the text, but only if the width is bigger than some limit. What I mean is: wrapping doesn't work for all possible width changes. Maybe that's a JavaFX problem, or you need to know a lot of JavaFX internals to solve this problems. If you couldn't find the issue and fix it, it's okay to skip it. (But don't forget about wrapping in plain-text mode).

Fonts are not handled properly.

src/main/java/org/jabref/gui/ai/components/summary/SummaryShowingComponent.fxml

src/main/java/org/jabref/gui/ai/components/summary/SummaryShowingComponent.java

mulla028 · 2024-12-10T18:19:10Z

@koppor @subhramit @calixtus
How do you like it now ?

mulla028 · 2024-12-10T18:20:37Z

@InAnYan
Talking about the text wrapper, and it's limit. I think this is JavaFX issue. Unfortunately, it has nothing to do with my changes.

InAnYan · 2024-12-11T08:16:43Z

CHANGELOG.md

@@ -49,6 +49,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv
 - By double clicking on a local citation in the Citation Relations Tab you can now jump the linked entry. [#11955](https://github.com/JabRef/jabref/pull/11955)
 - We use the menu icon for background tasks as a progress indicator to visualise an import's progress when dragging and dropping several PDF files into the main table. [#12072](https://github.com/JabRef/jabref/pull/12072)
 - The PDF content importer now supports importing title from upto the second page of the PDF. [#12139](https://github.com/JabRef/jabref/issues/12139)
+- We added support of `Markdown` in `AI Summary` tab by using the `CheckBox` next to `Regenerate` button. [#12266](https://github.com/JabRef/jabref/pull/12266)


As AI features are not currently released, we don't put them in CHANGLELOG.md

InAnYan · 2024-12-14T15:54:44Z

src/main/java/org/jabref/gui/ai/components/summary/SummaryShowingComponent.java

+        contentWebView = WebViewStore.get();
+        VBox.setVgrow(contentWebView, Priority.ALWAYS);
+
+        themeManager.installCss(contentWebView.getEngine());


Unfortunately, this doesn't work..

@calixtus, @koppor is there any way for browser to use JabRef's style? Font family and size specifically? Or should we leave the default font in WebView (I guess it's some Serif)?

src/main/java/org/jabref/gui/ai/components/summary/SummaryShowingComponent.java

InAnYan · 2024-12-14T15:57:10Z

Overall, I think this PR is ready to be merged, just an annoying problem with Fonts

subhramit

Just one more comment from my side.

src/main/java/org/jabref/gui/ai/components/summary/SummaryShowingComponent.java

mulla028 added 2 commits December 5, 2024 18:22

Checkbox and WebView added to the FXML

cdd56e2

WebView and CheckBox added, onMarkdownToggle() method implemented to …

57fb5f6

…handle checkbox changes

github-actions bot reviewed Dec 5, 2024

View reviewed changes

mulla028 added 2 commits December 5, 2024 19:53

WebView erased from fxml

ad38825

WebView added and set programmatically

71e1f22

CheckBox Moved

e1816c4

mulla028 force-pushed the issue-12233 branch from e1816c4 to 57fb5f6 Compare December 7, 2024 18:42

github-actions bot reviewed Dec 7, 2024

View reviewed changes

subhramit requested changes Dec 7, 2024

View reviewed changes

src/main/java/org/jabref/gui/ai/components/summary/SummaryShowingComponent.fxml Outdated Show resolved Hide resolved

src/main/java/org/jabref/gui/ai/components/summary/SummaryShowingComponent.java Outdated Show resolved Hide resolved

subhramit reviewed Dec 7, 2024

View reviewed changes

src/main/java/org/jabref/gui/ai/components/summary/SummaryShowingComponent.fxml Outdated Show resolved Hide resolved

subhramit requested a review from koppor December 7, 2024 19:04

mulla028 added 2 commits December 9, 2024 01:00

TextArea wrapped around StackPane | Imports updated | Version updated

fe4b53f

StackPane added | MarkdownFormatter initialized outside of contructor…

e62d4f2

… | Lazy initialization of WebView implemented

mulla028 requested review from subhramit and calixtus December 9, 2024 06:02

CheckBox and The Button on the same level, wrapped around BorderPane

834e51e

InAnYan requested changes Dec 9, 2024

View reviewed changes

src/main/java/org/jabref/gui/ai/components/summary/SummaryShowingComponent.java Outdated Show resolved Hide resolved

src/main/java/org/jabref/gui/ai/components/summary/SummaryShowingComponent.java Outdated Show resolved Hide resolved

mulla028 added 2 commits December 9, 2024 18:07

replace TextArea with WebView for summary display

b5199d4

simplify summary component layout

4a0bd3b

mulla028 requested a review from InAnYan December 9, 2024 23:09

InAnYan requested changes Dec 10, 2024

View reviewed changes

mulla028 added 3 commits December 10, 2024 11:49

TextFlow removed

cab3a8e

<pre> changed to <body><div>

b74af64

Default FontFamily and Size applied using ThemeManager

9ee5fb8

mulla028 requested a review from InAnYan December 10, 2024 18:17

Described the PR in CHANGELOG.md

a4bace7

InAnYan requested changes Dec 14, 2024

View reviewed changes

subhramit requested changes Dec 14, 2024

View reviewed changes

src/main/java/org/jabref/gui/ai/components/summary/SummaryShowingComponent.java Outdated Show resolved Hide resolved

MarkdownFormatter renamed and now static

da853b1

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use Markdown in AI Summary tab implemented #12266

Use Markdown in AI Summary tab implemented #12266

mulla028 commented Dec 5, 2024 •

edited

Loading

github-actions bot left a comment

calixtus commented Dec 6, 2024

calixtus commented Dec 6, 2024

mulla028 commented Dec 6, 2024

subhramit commented Dec 7, 2024

mulla028 commented Dec 7, 2024

subhramit commented Dec 7, 2024 •

edited

Loading

mulla028 commented Dec 7, 2024 •

edited

Loading

github-actions bot left a comment

subhramit left a comment

calixtus commented Dec 7, 2024

calixtus commented Dec 7, 2024

mulla028 commented Dec 7, 2024

koppor commented Dec 8, 2024

mulla028 commented Dec 9, 2024 •

edited

Loading

mulla028 commented Dec 9, 2024 •

edited

Loading

InAnYan left a comment •

edited

Loading

mulla028 commented Dec 9, 2024

InAnYan left a comment

mulla028 commented Dec 10, 2024

mulla028 commented Dec 10, 2024

InAnYan Dec 11, 2024

InAnYan Dec 14, 2024

InAnYan commented Dec 14, 2024

subhramit left a comment

Use Markdown in AI Summary tab implemented #12266

Are you sure you want to change the base?

Use Markdown in AI Summary tab implemented #12266

Conversation

mulla028 commented Dec 5, 2024 • edited Loading

Description (UPDATED)

How Code Changed and Why?

How to Copy Formatted Content?

Screenshots

Mandatory checks

github-actions bot left a comment

Choose a reason for hiding this comment

calixtus commented Dec 6, 2024

calixtus commented Dec 6, 2024

mulla028 commented Dec 6, 2024

subhramit commented Dec 7, 2024

mulla028 commented Dec 7, 2024

subhramit commented Dec 7, 2024 • edited Loading

mulla028 commented Dec 7, 2024 • edited Loading

github-actions bot left a comment

Choose a reason for hiding this comment

subhramit left a comment

Choose a reason for hiding this comment

calixtus commented Dec 7, 2024

calixtus commented Dec 7, 2024

mulla028 commented Dec 7, 2024

koppor commented Dec 8, 2024

mulla028 commented Dec 9, 2024 • edited Loading

mulla028 commented Dec 9, 2024 • edited Loading

InAnYan left a comment • edited Loading

Choose a reason for hiding this comment

mulla028 commented Dec 9, 2024

InAnYan left a comment

Choose a reason for hiding this comment

mulla028 commented Dec 10, 2024

mulla028 commented Dec 10, 2024

InAnYan Dec 11, 2024

Choose a reason for hiding this comment

InAnYan Dec 14, 2024

Choose a reason for hiding this comment

InAnYan commented Dec 14, 2024

subhramit left a comment

Choose a reason for hiding this comment

mulla028 commented Dec 5, 2024 •

edited

Loading

subhramit commented Dec 7, 2024 •

edited

Loading

mulla028 commented Dec 7, 2024 •

edited

Loading

mulla028 commented Dec 9, 2024 •

edited

Loading

mulla028 commented Dec 9, 2024 •

edited

Loading

InAnYan left a comment •

edited

Loading