Add a method to create a new file link manually #11539

Kunal77689 · 2024-07-27T03:19:15Z

Manual File Entry Creation:

Modified add button functionality so that rather than opening the file explorer, it opens a dialog box for manual file creation.

Mandatory checks

Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
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.

Manual File Entry Creation: Modified add button functionality so that rather than opening the file explorer, it opens a dialog box for manual file creation. [JabRef#11017]

koppor

The dialog code seems to be copied and slightly be adapted. Isn't it possible to adapt the existing code?

Code duplication leads to higher maintenance effort.

Kunal77689 · 2024-07-27T10:28:03Z

@koppor Sure I can give it a shot, thanks

- Deleted fxml and java files which were handling edit and add action separately and added a unified way which handles edit and add action dynamically

subhramit · 2024-07-27T18:45:04Z

src/main/java/org/jabref/gui/linkedfile/LinkedFileDialogController.java

+    public LinkedFileDialogController(LinkedFile linkedFile, boolean isEditMode) {
+        this.linkedFile = linkedFile != null ? linkedFile : new LinkedFile("", "", "");
+        this.isEditMode = isEditMode;


Instead of making it so complicated, you can overload the constructor. One will take a LinkedFile for editing, another will not take any argument, for adding file. Invoke them accordingly.
This way, 1. you won't have to pass null values 2. won't need an additional isEditMode flag.

- Overloaded the constructor so that one will take a LinkedFile for editing, another will not take any argument.

subhramit

Thank you for your changes. A few more suggestions.

CHANGELOG.md

src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditorViewModel.java

- Removed extra blank line in LinkedFilesEditorViewModel -Added link to the pr in the CHANGELOG.md

- Fixed checkstyle erorrs

subhramit

Good work so far. Now let's focus on the failing unit tests.

src/main/java/org/jabref/gui/linkedfile/LinkedFileDialogController.java

- Fixed findObsoleteLocalizationKeys() error by removing unwanted keys. - Fixed findMissingLocalizationKeys() by adding "Add file link" as a key

subhramit

Some comments on the renaming.
@koppor over to you now.

subhramit · 2024-08-01T00:27:17Z

src/main/java/org/jabref/gui/linkedfile/LinkedFileDialogController.java

-public class LinkedFileEditDialogView extends BaseDialog<LinkedFile> {
+public class LinkedFileDialogController extends BaseDialog<LinkedFile> {


This would be an architectural pattern shift. Read about what a model, view and controller is supposed to do, and also about the MVVM pattern followed in JabRef's JavaFX modules:
https://devdocs.jabref.org/code-howtos/javafx.html

@koppor is a controller needed here? the renaming seems off to me.

AFAIK, we always call it "View". We have background material collected at https://devdocs.jabref.org/code-howtos/javafx.html#architecture-model---view---controller---viewmodel-mvcvm.

Please revert the change, afterwards, @calixtus should double check on this.

subhramit · 2024-08-01T01:00:12Z

@Kunal77689 meanwhile, you can update the PR description (learn about keywords like Closes, Fixes that reference issues, and write a description of your changes). Look at a few other PRs, how they are structured (nothing hard and fast, but for example the screenshots should be above the mandatory checks).

koppor

Some code comments.

Moreover: general remarks

The hower of the "plus" button at General/File should show "Add", not open:

The howver of

The file type should be pre-filled with .pdf:

If possible, the fields should be prefilled with some "prompt text"

input.setPromptText(...);

Do you feel confident to lookup some matching strings in the localization file?

The two dialogs have different styles:

I think, some style loading is missing?

koppor · 2024-08-01T10:58:01Z

src/main/java/org/jabref/gui/linkedfile/LinkedFileDialogController.fxml

@@ -48,7 +49,7 @@
            <ComboBox fx:id="fileType"
                      GridPane.columnIndex="1" GridPane.hgrow="ALWAYS" GridPane.rowIndex="2"/>

-            <Label text="%Source URL" GridPane.rowIndex="3"/>


The % is an indication that this string should be looked up in the localization. Please keep it.

koppor · 2024-08-01T10:58:05Z

src/main/java/org/jabref/gui/linkedfile/LinkedFileDialogController.fxml

@@ -36,7 +37,7 @@
                    <JabRefIconView glyph="OPEN"/>
                </graphic>
                <tooltip>
-                    <Tooltip text="%Browse"/>


The % is an indication that this string should be looked up in the localization. Please keep it.

koppor · 2024-08-01T10:58:09Z

src/main/java/org/jabref/gui/linkedfile/LinkedFileDialogController.fxml

            </rowConstraints>

-            <Label text="%Link"/>


The % is an indication that this string should be looked up in the localization. Please keep it.

Maybe, this is also an artifact that the string is put in the Java Code? Then use "" instead of some text.

koppor · 2024-08-01T11:05:02Z

src/main/java/org/jabref/gui/linkedfile/LinkedFileDialogController.java

-public class LinkedFileEditDialogView extends BaseDialog<LinkedFile> {
+public class LinkedFileDialogController extends BaseDialog<LinkedFile> {


AFAIK, we always call it "View". We have background material collected at https://devdocs.jabref.org/code-howtos/javafx.html#architecture-model---view---controller---viewmodel-mvcvm.

Please revert the change, afterwards, @calixtus should double check on this.

src/main/java/org/jabref/gui/linkedfile/LinkedFileDialogController.java

subhramit · 2024-08-01T11:59:40Z

Somehow some comments are getting duplicated. Strange.

LoayGhreeb · 2024-08-02T01:02:31Z

src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditor.java

-        viewModel.addNewFile();
+        LinkedFileDialogController controller = new LinkedFileDialogController();
+
+        controller.showAndWait().ifPresent(result -> {


You have to use org.jabref.gui.DialogService#showCustomDialogAndWait(javafx.scene.control.Dialog<R>) to get the right styling for the dialog.

- Renamed files from controller->view - Made sure both edit and add dialog boxes follow same layout - The add dialog box will have pdf as the default option - fixed minor bugs in some places where some "%" were removed in some of the previous commits, restored those symbols - on hover, rather than showing open, it will show add

koppor · 2024-08-06T15:19:45Z

src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditor.java

+
+        // Handle the result if the user clicked OK or Add
+        if (newLinkedFile != null) {
+            // Add the new file to your list or handle it as needed


Which AI assistant did you use? I want to learn about their capabiltiies.

@koppor I generally use ChatGPT

koppor · 2024-08-06T15:25:36Z

src/main/java/org/jabref/gui/linkedfile/LinkedFileDialogView.java

@@ -18,7 +19,10 @@
 import com.airhacks.afterburner.views.ViewLoader;
 import jakarta.inject.Inject;

-public class LinkedFileEditDialogView extends BaseDialog<LinkedFile> {
+public class LinkedFileDialogView extends BaseDialog<LinkedFile> {


Please do not trust the AI in all cases.

The new name is not consistent to the name of the viewmodel

@koppor My bad, I did not ask AI for the naming. I missed this link you provided above https://devdocs.jabref.org/code-howtos/javafx.html#architecture-model---view---controller---viewmodel-mvcvm, I just went on youtube and watched some videos about mvc structure and it mentioned about overall what model view and controller usually are responsible for so I thought because we are update the dialog box, it should come under view.

The thing is a Dialog. In other places in JabRef, dialogs also have the suffix Dialog. I renamed.

…popup_form_kunalsikka_dev

koppor · 2024-08-06T16:09:00Z

@Kunal77689 I took the liberty to finish this PR. I tried to do fine-grained commits to enable you to follow.

Our team invests significant time and resources, with a 2-to-1 ratio of senior developers mentoring each student. This approach is meant to help you grow your skills and deepen your understanding of coding and open-source contributions. I think, with the three reviewing rounds and this commits we fulfilled our duty.

Kunal77689 · 2024-08-06T16:18:39Z

@koppor @subhramit Thank you so much for guidance throughout the task. This task helped me familiarize myself with parts of Jabref codebase which should help me in future contributions. I am looking forward to contributing more effectively in future

subhramit · 2024-12-08T18:13:56Z

@Kunal77689 Hello Kunal. We noticed that under the "employees" heading of our LinkedIn page, you have stated yourself as being "self-employed" under the organization. Please note that you are not employed (by anyone or yourself) under the organization. You also claim to have "optimized user efficiency by over 50%", and we are completely unaware of the source from which you got the data, what metric you used, etc.

We kindly request you to take that down, as the PR you filed was not even completely your work, and the data is misleading. One AI generated PR does not count as an "experience", "project" or "employment".

Kunal77689 · 2024-12-08T18:22:01Z

Hello There was no such thing as open source so I added self employed but sure, I am gonna change that. Also, I will change the description too. It wasn't fully AI generated, I took some help to maintain code clarity Kunal

…

On Sun, Dec 8, 2024 at 2:44 PM Subhramit Basu Bhowmick < ***@***.***> wrote: @Kunal77689 <https://github.com/Kunal77689> Hello Kunal. We noticed that under the "employees" heading of our LinkedIn page, you have stated yourself as being "self-employed" under the organization. Please note that you are not employed (by anyone or yourself) under the organization. You also claim to have "optimized user efficiency by over 50%", and we are completely unaware of the source from which you got the data, what metric you used, etc. We kindly request you to take that down, as the PR you filed was not even completely your work, and the data is misleading. — Reply to this email directly, view it on GitHub <#11539 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AT3EIRNFT5BILGM3VKILJNL2ESD7XAVCNFSM6AAAAABLRPYKBWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKMRWGI4TQMRWG4> . You are receiving this because you were mentioned.Message ID: ***@***.***>

Kunal77689 added 2 commits July 27, 2024 00:36

Add a method to create a new link manually JabRef#11017

c2f88da

Manual File Entry Creation: Modified add button functionality so that rather than opening the file explorer, it opens a dialog box for manual file creation. [JabRef#11017]

Updated CHANGELOG.md

86bfc75

Kunal77689 changed the title ~~Manual file creation popup form kunalsikka dev~~ Manual file creation popup form kunalsikka dev #11017 Jul 27, 2024

Kunal77689 mentioned this pull request Jul 27, 2024

Added a method to create a new link manually #11017 #11533

Closed

6 tasks

Kunal77689 changed the title ~~Manual file creation popup form kunalsikka dev #11017~~ Added a method to create a new link manually #11017 Jul 27, 2024

koppor requested changes Jul 27, 2024

View reviewed changes

Added a method to create a new link manually JabRef#11017

782e5ae

- Deleted fxml and java files which were handling edit and add action separately and added a unified way which handles edit and add action dynamically

subhramit requested changes Jul 27, 2024

View reviewed changes

Added a method to create a new link manually JabRef#11017

f1c5524

- Overloaded the constructor so that one will take a LinkedFile for editing, another will not take any argument.

Kunal77689 requested review from koppor and subhramit July 28, 2024 16:40

subhramit reviewed Jul 30, 2024

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditorViewModel.java Outdated Show resolved Hide resolved

Kunal77689 added 3 commits July 31, 2024 16:05

Added a method to create a new link manually JabRef#11017

a8908a7

- Removed extra blank line in LinkedFilesEditorViewModel -Added link to the pr in the CHANGELOG.md

Added a method to create a new link manually JabRef#11017

2dd6f93

- Fixed checkstyle erorrs

Added a method to create a new link manually JabRef#11017

a2799b2

- Fixed checkstyle erorrs

Kunal77689 requested a review from subhramit July 31, 2024 22:13

subhramit requested changes Jul 31, 2024

View reviewed changes

Added a method to create a new link manually JabRef#11017

2415aee

- Fixed findObsoleteLocalizationKeys() error by removing unwanted keys. - Fixed findMissingLocalizationKeys() by adding "Add file link" as a key

Kunal77689 requested a review from subhramit July 31, 2024 23:06

subhramit requested changes Aug 1, 2024

View reviewed changes

JabRef deleted a comment from Kunal77689 Aug 1, 2024

subhramit changed the title ~~Added a method to create a new link manually #11017~~ Add a method to create a new file link manually Aug 1, 2024

koppor mentioned this pull request Aug 1, 2024

Add comments to FXML localization parsing #11558

Merged

6 tasks

InAnYan mentioned this pull request Aug 1, 2024

Add more explanation for localization in Java code and FXML #11559

Merged

3 tasks

koppor requested changes Aug 1, 2024

View reviewed changes

LoayGhreeb reviewed Aug 2, 2024

View reviewed changes

Kunal77689 requested review from koppor and subhramit August 6, 2024 02:55

Merge branch 'main' into manual_file_creation_popup_form_kunalsikka_dev

5f6e364

koppor reviewed Aug 6, 2024

View reviewed changes

Use Java 8 Optional

2a120d4

koppor reviewed Aug 6, 2024

View reviewed changes

koppor added 9 commits August 6, 2024 17:26

Fix class names

ef38e49

Fix name (again)

85049e5

Fix method name and reuse code

5066a9a

Fix double dialog initialization

d19dc1c

Fix PDF type

eed2a5f

Remove generated JavaDoc

4e80a5d

Remove dead code

c6f99c7

Merge remote-tracking branch 'origin/main' into manual_file_creation_…

bf4fc04

…popup_form_kunalsikka_dev

Re-add missing localizations

b3a781f

koppor enabled auto-merge August 6, 2024 16:09

koppor approved these changes Aug 6, 2024

View reviewed changes

subhramit approved these changes Aug 6, 2024

View reviewed changes

koppor added this pull request to the merge queue Aug 6, 2024

Merged via the queue into JabRef:main with commit 3c02efd Aug 6, 2024
22 checks passed

subhramit mentioned this pull request Oct 12, 2024

Establish guidelines for usage of AI in PRs #11940

Merged

6 tasks

This was referenced Nov 21, 2024

adding file with illegal character does not proceed #12212

Closed

move check for invalid characters to linked files editor #12219

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a method to create a new file link manually #11539

Add a method to create a new file link manually #11539

Kunal77689 commented Jul 27, 2024 •

edited

Loading

koppor left a comment

Kunal77689 commented Jul 27, 2024

subhramit Jul 27, 2024 •

edited

Loading

subhramit left a comment

subhramit left a comment •

edited

Loading

subhramit left a comment •

edited

Loading

subhramit Aug 1, 2024 •

edited

Loading

koppor Aug 1, 2024

subhramit commented Aug 1, 2024 •

edited

Loading

koppor left a comment

koppor Aug 1, 2024

koppor Aug 1, 2024

koppor Aug 1, 2024

koppor Aug 2, 2024

koppor Aug 1, 2024

subhramit commented Aug 1, 2024

LoayGhreeb Aug 2, 2024

koppor Aug 6, 2024

Kunal77689 Aug 6, 2024

koppor Aug 6, 2024

Kunal77689 Aug 6, 2024 •

edited

Loading

koppor Aug 6, 2024

koppor commented Aug 6, 2024

Kunal77689 commented Aug 6, 2024

subhramit commented Dec 8, 2024 •

edited

Loading

Kunal77689 commented Dec 8, 2024 via email •

edited

Loading

		public class LinkedFileEditDialogView extends BaseDialog<LinkedFile> {
		public class LinkedFileDialogController extends BaseDialog<LinkedFile> {

Add a method to create a new file link manually #11539

Add a method to create a new file link manually #11539

Conversation

Kunal77689 commented Jul 27, 2024 • edited Loading

Mandatory checks

koppor left a comment

Choose a reason for hiding this comment

Kunal77689 commented Jul 27, 2024

subhramit Jul 27, 2024 • edited Loading

Choose a reason for hiding this comment

subhramit left a comment

Choose a reason for hiding this comment

subhramit left a comment • edited Loading

Choose a reason for hiding this comment

subhramit left a comment • edited Loading

Choose a reason for hiding this comment

subhramit Aug 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

subhramit commented Aug 1, 2024 • edited Loading

koppor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

subhramit commented Aug 1, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kunal77689 Aug 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

koppor commented Aug 6, 2024

Kunal77689 commented Aug 6, 2024

subhramit commented Dec 8, 2024 • edited Loading

Kunal77689 commented Dec 8, 2024 via email • edited Loading

Kunal77689 commented Jul 27, 2024 •

edited

Loading

subhramit Jul 27, 2024 •

edited

Loading

subhramit left a comment •

edited

Loading

subhramit left a comment •

edited

Loading

subhramit Aug 1, 2024 •

edited

Loading

subhramit commented Aug 1, 2024 •

edited

Loading

Kunal77689 Aug 6, 2024 •

edited

Loading

subhramit commented Dec 8, 2024 •

edited

Loading

Kunal77689 commented Dec 8, 2024 via email •

edited

Loading