BibTeX information in web search import screen. (#560) #10784

MrMachiei · 2024-01-14T22:05:01Z

Closes koppor#560
Added BibTeX data to import web search screen:

After selection:

After unselect:

After selection then selecting another entry:

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.

tobiasdiez · 2024-01-16T04:23:34Z

Nice addition.

It takes, however, quite a bit of space away from the list of entries. Could you add a button "Show entry information" that toggles this functionality (default off). An alternative would be to show the info's on hovering the entry.

…abRef#560)

MrMachiei · 2024-01-16T20:53:28Z

@tobiasdiez as you wish ;)

tobiasdiez · 2024-01-17T02:38:23Z

Thanks, looks nice! 🙂 I don't have the time right now for a proper review of the code, but I'm sure another developer will look at it very soon!

Siedlerchr · 2024-01-18T19:45:30Z

src/main/java/org/jabref/gui/importer/ImportEntriesDialog.java

+        });
+    }
+
+    private String getSourceString(BibEntry entry) throws IOException {


Can you move the whole logic please to the viewModel?

koppor

Thank you for working on this. The provided screenshots realy help!

koppor · 2024-01-22T14:10:15Z

src/main/resources/l10n/JabRef_en.properties

@@ -1986,6 +1986,8 @@ Total\ items\ found\:=Total items found:
 Selected\ items\:=Selected items:
 Download\ linked\ online\ files=Download linked online files
 Select\ the\ entries\ to\ be\ imported\:=Select the entries to be imported\:
+Entry\ BibTeX\ data\:=Entry BibTeX data\:


There should not be new translations for this heading. The Entry Editor uses "BibTeX". Please re-use that string. Add the : manually in the code (or just style as heading so that no : is necessary).

koppor · 2024-01-22T14:12:09Z

src/main/resources/l10n/JabRef_en.properties

@@ -1986,6 +1986,8 @@ Total\ items\ found\:=Total items found:
 Selected\ items\:=Selected items:
 Download\ linked\ online\ files=Download linked online files
 Select\ the\ entries\ to\ be\ imported\:=Select the entries to be imported\:
+Entry\ BibTeX\ data\:=Entry BibTeX data\:
+Show\ entry\ information=Show entry information


It should be "Show BibTeX", because "information" is more general and could be more. For instance: citation data, is this entry already contained in the library, completeness of the bibtex data, ...

calixtus · 2024-01-22T17:18:19Z

+1 to show this information as tooltip on mouse over as you then won't have to deal with multiple entries in the code area that should be added. And if not, hide it with a checkbox.

MrMachiei · 2024-01-22T20:30:16Z

+1 to show this information as tooltip on mouse over as you then won't have to deal with multiple entries in the code area that should be added. And if not, hide it with a checkbox.

The option to hide with checkbox has been already added

MrMachiei · 2024-01-22T20:50:38Z

@koppor changes added
@calixtus as I mentioned above such a feature and screenshots were added earlier

koppor

Current look:

1: Remove the ":" (because it is a heading). I know that the heading at the beginning also has a ":". Nevertheless, the first one is not quite a heading, but an instruction.
2: Add "source", to be consistent with the heading (1)

In JabRef_en.properties, please group the strings together:

%0\ source=%0 source
Show\ BibTeX\ source=Show BibTeX source
Show/edit\ %0\ source=Show/edit %0 source

It is OK for me to have Show BibTeX source as new string.

Finally, if I activate "Show BibTeX source", the BibTeX source of the current selected entry should be shown.

Currently, it does not

Select dblp as source
Search for "vino4tosca"
Import dialog with one result is shown
Activate on "Show BibTeX"
See that there is no source shown.

koppor · 2024-01-24T14:22:26Z

src/main/java/org/jabref/gui/importer/ImportEntriesViewModel.java

+            new BibEntryWriter(fieldWriter, entryTypesManager).write(entry, bibWriter, selectedDb.getValue().getMode());
+        } catch (
+                IOException ioException) {
+            return null;


In Java, one should avoid null, because this leads to NullPointerExceptions, which are hard to debug.

In your case, you can fallback to a default value - the empty string.

Suggested change

return null;

return "";

You could also include the erorr message, but I think, this is not helpful for the user.

MrMachiei · 2024-01-24T22:53:42Z

Current look:

1: Remove the ":" (because it is a heading). I know that the heading at the beginning also has a ":". Nevertheless, the first one is not quite a heading, but an instruction.

2: Add "source", to be consistent with the heading (1)

In JabRef_en.properties, please group the strings together:
%0\ source=%0 source
Show\ BibTeX\ source=Show BibTeX source
Show/edit\ %0\ source=Show/edit %0 source
It is OK for me to have Show BibTeX source as new string.

Finally, if I activate "Show BibTeX source", the BibTeX source of the current selected entry should be shown.

Currently, it does not

Select dblp as source

Search for "vino4tosca"

Import dialog with one result is shown

Activate on "Show BibTeX"

See that there is no source shown.

@koppor
As you mentioned in requirements in koppor#560:
"I would like to have a) the BibTeX information or b) a list of fields (as soon as I click on an entry)."

The requirements clearly specifies that the info should be visible after clicking such entry. In the EDGE CASE you mentioned the entry is preselected as there's only one entry. I'd recommend to write better requirements and task specifications win the future, as currently we're talking about some different features.

Also I'd like to point out the new changes in the headers you proposed - you want the ":" mark to be deleted, as in previous part of this pull request you said "There should not be new translations for this heading. The Entry Editor uses "BibTeX". Please re-use that string. Add the : manually in the code (or just style as heading so that no : is necessary).".

And about the "Show BibTeX" text - you also wanted another text as you mention now - "It should be "Show BibTeX", because "information" is more general and could be more. For instance: citation data, is this entry already contained in the library, completeness of the bibtex data, ...".

I believe that such kind of reviews are working very poorly and frustrate the developer that implements it. In my opinion you need to re-think some part of this project code review process.

MrMachiei · 2024-01-24T23:07:22Z

@koppor As I'm a kind person, there're your requested changes.

koppor · 2024-01-25T00:06:53Z

First of all, thank you for your patience!

@koppor As you mentioned in requirements in koppor#560: "I would like to have a) the BibTeX information or b) a list of fields (as soon as I click on an entry)."

The requirements clearly specifies that the info should be visible after clicking such entry. In the EDGE CASE you mentioned the entry is preselected as there's only one entry. I'd recommend to write better requirements and task specifications win the future, as currently we're talking about some different features.

Sometimes, requirements then get very long and implementers say: "This is too much text, I cannot understand". In this project, we are not in the 80:10:10 effort split between specification, implementation, and test. Thinking of all edge cases takes time.

The benefit of working in a closed loop (and not in a waterfall-style model) is that there is feedback. One can try out things. Since you implement the changes fast, there is really a fast feedback possible.

And about the "Show BibTeX" text - you also wanted another text as you mention now - "It should be "Show BibTeX", because "information" is more general and could be more. For instance: citation data, is this entry already contained in the library, completeness of the bibtex data, ...".

I thought about texts, because your starting suggestions were not consistent to JabRef's UI. It seems you are new to the context of literature management with JabRef, which is OK. -- Note that the tab of the enty editor was screenshot by at koppor#560 (comment). There, you could read "biblatex source". Sorry, that I screenshotted the library in biblatex (and not BibTeX mode) and thus the concrete string was underspecified.

I believe that such kind of reviews are working very poorly and frustrate the developer that implements it. In my opinion you need to re-think some part of this project code review process.

Two things:

First, as said, this fast-paced loop helps to investigate results and to improve things. It is impossible to specify all requirements at the beginning. Delivering intermediate results help to evaluate the result. - Therefore, the agile movement got traction - and there are less and less projects following the waterfall model.

Second, typically requirements are not complete. The implementor has to think about the edge cases. Decide for themselves or ask for feedback. In a perfect world, the implementor has a relation to the resulting code and knows the demand of the using person(s) and can handle accordingly. My personal hope for JabRef is that contributors implementing something for the software can get into the context. Most of the contributors are students having to write scientific articles with references. Moreover, I assume they are using the computer and like good UX. I know that "good UX" is debatable and that this is kind of experience and that no one can expect that kind of thinking from typical contributors of JabRef.

Finally, thank you for the feedback. We are trying to provide better issue descriptions for university projects. Therefore, not all issues are listed at our list for candidates for university projects https://github.com/orgs/JabRef/projects/3/views/3?query=is%3Aopen+sort%3Aupdated-desc, but only selected ones.

You are one of the contributors reacting very fast to comments. Thank you for that. That also motivates us to spend our hobby time in interaction with you. (We are all doing this in our free time to make the world a better place).

MrMachiei · 2024-01-25T00:15:34Z

@koppor Thanks for your clarification. I'm just really used to strict requirements at work, when the business says "clicked" it means clicked, not selected, so my point of view is a little biased ;p

Im just glad that finally we've reached a consensus. Thanks for fast responses :)

MrMachiei mentioned this pull request Jan 14, 2024

Web search results should render the BibTeX too koppor/jabref#560

Closed

MrMachiei added 4 commits January 16, 2024 21:51

BibTeX information in web search import screen. (JabRef#560)

1cb357b

BibTeX information in web search import screen - add toggle button (J…

3e18d17

…abRef#560)

fixing style issues

26dc996

fixing some repo issues (JabRef#560)

654fd1c

MrMachiei force-pushed the implementation-of-issue-560 branch from 6f402a4 to 654fd1c Compare January 16, 2024 20:52

Siedlerchr requested a review from calixtus January 17, 2024 20:27

Siedlerchr reviewed Jan 18, 2024

View reviewed changes

move logic to viewModel (JabRef#560)

fae3f8b

koppor requested changes Jan 22, 2024

View reviewed changes

changing headers (JabRef#560)

1181abe

fix typo (JabRef#560)

5c0496e

koppor requested changes Jan 24, 2024

View reviewed changes

MrMachiei and others added 2 commits January 25, 2024 00:04

further changes... (JabRef#560)

e58f5f3

Merge branch 'main' into implementation-of-issue-560

aba5118

Refine text

a8272f4

koppor approved these changes Jan 25, 2024

View reviewed changes

koppor added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Jan 25, 2024

Siedlerchr approved these changes Jan 25, 2024

View reviewed changes

Siedlerchr added this pull request to the merge queue Jan 25, 2024

Merged via the queue into JabRef:main with commit bc3d223 Jan 25, 2024
19 checks passed

koppor mentioned this pull request Feb 15, 2024

Citation Relations are currently not too useful #10869

Open

4 tasks

koppor mentioned this pull request Mar 14, 2024

Improve CFF import/export and craft a round-trip test #10995

Merged

16 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BibTeX information in web search import screen. (#560) #10784

BibTeX information in web search import screen. (#560) #10784

MrMachiei commented Jan 14, 2024 •

edited

Loading

tobiasdiez commented Jan 16, 2024

MrMachiei commented Jan 16, 2024

tobiasdiez commented Jan 17, 2024

Siedlerchr Jan 18, 2024

MrMachiei Jan 20, 2024

koppor left a comment

koppor Jan 22, 2024

MrMachiei Jan 22, 2024

koppor Jan 22, 2024

MrMachiei Jan 22, 2024

calixtus commented Jan 22, 2024

MrMachiei commented Jan 22, 2024

MrMachiei commented Jan 22, 2024

koppor left a comment

koppor Jan 24, 2024

MrMachiei commented Jan 24, 2024

MrMachiei commented Jan 24, 2024

koppor commented Jan 25, 2024

MrMachiei commented Jan 25, 2024

BibTeX information in web search import screen. (#560) #10784

BibTeX information in web search import screen. (#560) #10784

Conversation

MrMachiei commented Jan 14, 2024 • edited Loading

tobiasdiez commented Jan 16, 2024

MrMachiei commented Jan 16, 2024

tobiasdiez commented Jan 17, 2024

Siedlerchr Jan 18, 2024

Choose a reason for hiding this comment

MrMachiei Jan 20, 2024

Choose a reason for hiding this comment

koppor left a comment

Choose a reason for hiding this comment

koppor Jan 22, 2024

Choose a reason for hiding this comment

MrMachiei Jan 22, 2024

Choose a reason for hiding this comment

koppor Jan 22, 2024

Choose a reason for hiding this comment

MrMachiei Jan 22, 2024

Choose a reason for hiding this comment

calixtus commented Jan 22, 2024

MrMachiei commented Jan 22, 2024

MrMachiei commented Jan 22, 2024

koppor left a comment

Choose a reason for hiding this comment

koppor Jan 24, 2024

Choose a reason for hiding this comment

MrMachiei commented Jan 24, 2024

MrMachiei commented Jan 24, 2024

koppor commented Jan 25, 2024

MrMachiei commented Jan 25, 2024

MrMachiei commented Jan 14, 2024 •

edited

Loading