Improve user friendliness of automatically linked files #7484

osclind · 2021-03-03T11:25:22Z

We have implemented a new textfield for files that are automatically detected by JabRef and listed as files in the general tab of the entry editor. We also changed the icon for automatically linked files from a BRIEFCASE_CHECK to LINK_PLUS, since we found it more intuitive. This fixes #3607.

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 documentation: Is the information available and up to date? If not created an issue at https://github.com/JabRef/user-documentation/issues or, even better, submitted a pull request to the documentation repository.

We thought that creating an issue would be redundant if our changes weren't accepted, so we have not completed the documentation task. The documentation refers to the icon as a "suitcase", which would not be accurate, were these changes to be accepted. Instead it should be referred to as a link with a plus sign.

Screenshots

Before changes

File not linked

File linked

After changes

File not linked

File linked

- added (Auto) in front of the name of the pdf - changed the icon for the linking action to LINK_PLUS

- Removed unused import statements introduced in 64a6005 - Removed extraneous newline introduced in 64a6005

- remove duplicates of empty rows missed in 50e98a7

calixtus · 2021-03-04T08:27:09Z

Hi folks, thanks for your contribution. At very first sight, I like this idea.
About the icon: this seems ok, just a few days ago we switched our iconset to iconli, the previous iconset was very limited, so the briefcase icon was just a workaround. The new icon seem reasonable.
About the label: I'm not really sure, if we should display an icon or a label. Maybe the other @JabRef/developers have a preference. But if we use a label, it needs to be localized. (see https://devdocs.jabref.org/getting-into-the-code/code-howtos#using-localization-correctly ).

LukasGutenberg · 2021-03-04T10:04:15Z

Hello @calixtus. @tobiasdiez recommended an implementation with a description that says '(Auto)' in issue #3607, we agreed that it would make the display clearer and chose to implement it as a separate text field in the HBox, as the description is currently bound to the linked file's description property. We estimated that too much refactoring would be required to initially set the description to '(Auto)' and then change it to be bound to the file's description property, if the user chooses to link the file. We also thought that it made more sense to display the text prior to the file name. We'll await feedback from another dev about the icon vs. text dilemma.

calixtus · 2021-03-04T10:22:20Z

Thank you for your quick response. I did not see the discussion with tobias diez earlier, sorry, but if that's what you both agreed on, than its perfectly fine.

tobiasdiez

The code changes look good to me. Thanks!

I'm also not sure about the best way to make it clear that the file is automatically found. The current briefcase icon was more confusing than helpful. But maybe it's already enough to change the icon to the "link-add" symbol and move this symbol in front of the file name (i.e where "(Auto)" is written with this PR)? Another idea would be to set the transparency of automatically found files to something like 70%... @calixtus what do you think? @osclind & team, could you maybe experiment a bit with different solutions? It never hurts to have a fresh pair of eyes looking at this, and bringing in new ideas.

martinfalke · 2021-03-04T11:10:23Z

We'll try out a few solutions and update the thread with screenshots of the different results, then you can give feedback and we'll go with whatever looks best to you guys.

LukasGutenberg · 2021-03-05T10:17:57Z

After pressing button

Button to the left

Button where (Auto) was

With opacity

@calixtus @tobiasdiez
Here are some examples of the possible implementations that you suggested earlier.
Tell us what you think, we can of course make further changes if you want.

tobiasdiez · 2021-03-05T10:32:18Z

Thanks a lot! I like the "Button to the left" solution the most, and would say it should be sufficiently clear that this is not a normally-linked file. So if there is a short description as a tooltip for the link button, I would say this is fine. @JabRef/developers what do you think?

LukasGutenberg · 2021-03-05T10:42:57Z

The tooltip is unchanged since the earlier button as we thought it was well formulated and descriptive.

calixtus · 2021-03-05T13:11:59Z

I'm really not sure, what would be the best solution. On one hand, it shows now symbols to the left and to the right and it's hard to guess, what means what. On the other hand, it really should stand out, if there is an autolinked file. Guess there is no right solution.
What if there are more than one linked files listed?

LukasGutenberg · 2021-03-08T08:15:17Z

@calixtus Different entries are handled separately, we simply make use of the isAutomaticallyFound property to make changes.

Several automatically found

One added

calixtus · 2021-03-08T08:28:21Z

@calixtus Different entries are handled separately

Of course, it's just about the look and feel if several different list entries are shown. Thanks for testing out, I personally like the variant "icon on the very left".

LukasGutenberg · 2021-03-08T08:37:32Z

Do we use opacity and what percentage would you like? Currently we use 70% for automatically found files and 100% for files that have been added.

calixtus · 2021-03-08T08:50:09Z

It's just fine, as it is. If you use opacity please also check out, if it shows up right in the dark theme.

LukasGutenberg · 2021-03-08T08:59:35Z

It works for dark mode, I'll push the latest changes.

- auto removed - link button moved to the left - lowered opacity for automatically found files, unchanged for added

tobiasdiez

Thanks a lot! I have a few more nitpickings, otherwise it looks good to me.

tobiasdiez · 2021-03-08T09:19:27Z

CHANGELOG.md

@@ -29,6 +29,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
 - The export to MS Office XML now uses the month name for the field `MonthAcessed` instead of the two digit number [#7354](https://github.com/JabRef/jabref/issues/7354)
 - We included some standalone dialogs from the options menu in the main preference dialog and fixed some visual issues in the preferences dialog. [#7384](https://github.com/JabRef/jabref/pull/7384)
 - We improved the linking of the `python3` interpreter via the shebang to dynamically use the systems default Python. Related to [JabRef-Browser-Extension #177](https://github.com/JabRef/JabRef-Browser-Extension/issues/177)
+- Automatically found pdf files now have the linking button to the far left and uses the icon LINK_PLUS instead of BRIEFCASE_CHECK. The file name also has lowered opacity(70%) until added. [#3607](https://github.com/JabRef/JabRef-Browser-Extension/issues/3607)


The link to the issue is not correct. Moreover, the changelog is for users, so please don't use technical terms such as LINK_PLUS.

tobiasdiez · 2021-03-08T09:21:40Z

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

+ acceptAutoLinkedFile.managedProperty().bind(linkedFile.isAutomaticallyFoundProperty());
+ acceptAutoLinkedFile.setOnAction(event -> {
+ linkedFile.acceptAsLinked();
+ linkedFile.setOpacityProperty(1.0);


Move linkedFile.setOpacityProperty(1.0); into acceptAsLinked. The viewmodel should contain all the logic.

tobiasdiez · 2021-03-08T09:22:01Z

src/main/resources/l10n/JabRef_en.properties

@@ -1732,6 +1732,7 @@ Don't\ share=Don't share
 Share\ anonymous\ statistics=Share anonymous statistics
 Telemetry\:\ Help\ make\ JabRef\ better=Telemetry: Help make JabRef better
 To\ improve\ the\ user\ experience,\ we\ would\ like\ to\ collect\ anonymous\ statistics\ on\ the\ features\ you\ use.\ We\ will\ only\ record\ what\ features\ you\ access\ and\ how\ often\ you\ do\ it.\ We\ will\ neither\ collect\ any\ personal\ data\ nor\ the\ content\ of\ bibliographic\ items.\ If\ you\ choose\ to\ allow\ data\ collection,\ you\ can\ later\ disable\ it\ via\ Options\ ->\ Preferences\ ->\ General.=To improve the user experience, we would like to collect anonymous statistics on the features you use. We will only record what features you access and how often you do it. We will neither collect any personal data nor the content of bibliographic items. If you choose to allow data collection, you can later disable it via Options -> Preferences -> General.
+(Auto)=(Auto)


This is no longer needed, I guess.

tobiasdiez · 2021-03-08T09:23:00Z

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

@@ -196,6 +196,7 @@ public void bindToEntry(BibEntry entry) {
 preferences.getFilePreferences(),
 externalFileTypes);
 newLinkedFile.markAsAutomaticallyFound();


Move newLinkedFile.setOpacityProperty(0.3); to markAsAutomaticallyFound so that it is consistent, even if some other code marks a file as automatically found.

- made changelog easier to read - moved opacity operations to correct locations - readjusted opacity to correct value - removed unused localization

LukasGutenberg · 2021-03-08T09:50:19Z

i realized that opacity was set way too low and that the correct images should be:

this is how it looked in the first set of images with opacity

LukasGutenberg · 2021-03-08T10:07:37Z

we will write some tests for this as well

calixtus · 2021-03-08T10:12:12Z

If you like one more more idea on this: The "Königsweg" would be to assign a css PseudoState, so the opacity would be addressable in the stylesheet and be customizable.

calixtus · 2021-03-08T10:21:51Z

See for example ImportEntriesDialog::initialize() (lines 105 and 145)
This way you could subscribe in the view (EasyBind) changes of isAutomaticallyFoundProperty() to the pseudo class and remove the opacityProperty, which is some kind of a rule breaker of the MVVM design pattern.

tobiasdiez · 2021-03-08T10:40:35Z

King's Trail has a different meaning in Sweden: https://en.wikipedia.org/wiki/Kungsleden (highly recommended hike!). 😃 Otherwise I agree!

calixtus · 2021-03-08T10:45:14Z

Wouldn't say so, both the Kungsleden and using psuedo classes for opacity are distinguished by special beauty. 🙈 😆

- opacity is now set through a css pseudoclass instead of a property

LukasGutenberg · 2021-03-08T16:45:50Z

We did not manage to figure out how to use EasyBind but the program now makes use of css to change the opacity when the text is generated and when the button is pressed.

calixtus · 2021-03-08T21:47:00Z

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

+ acceptAutoLinkedFile.setOnAction(event -> {
+ linkedFile.acceptAsLinked();
+ link.pseudoClassStateChanged(opacity, linkedFile.isAutomaticallyFound());
+ });


Suggested change

acceptAutoLinkedFile.setOnAction(event -> {

linkedFile.acceptAsLinked();

link.pseudoClassStateChanged(opacity, linkedFile.isAutomaticallyFound());

});

acceptAutoLinkedFile.setOnAction(event -> linkedFile.acceptAsLinked());

EasyBind.subscribe(linkedFile.isAutomaticallyFoundProperty(), found -> link.pseudoClassStateChanged(opacity, found));

This should do it.

jabref/src/main/java/org/jabref/gui/util/BindingsHelper.java

Line 35 in 9aad6fc

public static Subscription includePseudoClassWhen(Node node, PseudoClass pseudoClass, ObservableValue<? extends Boolean> condition) {

- changed how opacity is updated

LukasGutenberg · 2021-03-09T09:18:30Z

@calixtus @tobiasdiez
Thank you for your time and engagement. We appreciate all the help and advice you have given us.

tobiasdiez

We have to thank you guys!

And sorry that the process was a bit back and forth. Sometimes the hardest part is not in the actual coding but figuring out what would be the best user experience. We hope you enjoyed the project nonetheless and hope to see further contributions ;-)

mlep · 2021-03-11T08:23:33Z

I have just tested it: Nice!
Thank you!

LukasGutenberg and others added 4 commits March 3, 2021 09:56

feature JabRef#3607 autolink redesign

64a6005

- added (Auto) in front of the name of the pdf - changed the icon for the linking action to LINK_PLUS

fix JabRef#3607 autolink cleanup

50e98a7

- Removed unused import statements introduced in 64a6005 - Removed extraneous newline introduced in 64a6005

Fix JabRef#3607 autolink cleanup

cbdcbc7

- remove duplicates of empty rows missed in 50e98a7

Updates changelog for JabRef#3607

931e454

fixed localization for JabRef#3607

41c825f

calixtus approved these changes Mar 4, 2021

View reviewed changes

tobiasdiez reviewed Mar 4, 2021

View reviewed changes

Siedlerchr added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Mar 6, 2021

feature JabRef#3607 final version

6f9f5cb

- auto removed - link button moved to the left - lowered opacity for automatically found files, unchanged for added

osclind requested a review from tobiasdiez March 8, 2021 09:04

Updated Changelog

134e945

tobiasdiez requested changes Mar 8, 2021

View reviewed changes

fix JabRef#3607

cb1cb5e

- made changelog easier to read - moved opacity operations to correct locations - readjusted opacity to correct value - removed unused localization

LukasGutenberg and others added 2 commits March 8, 2021 17:31

refactor JabRef#3607

aa5eea2

- opacity is now set through a css pseudoclass instead of a property

fixed link

b0eb124

calixtus reviewed Mar 8, 2021

View reviewed changes

LukasGutenberg and others added 2 commits March 9, 2021 09:36

Merge branch 'master' into fix-for-issue-JabRef#3607

4d9a709

refactor JabRef#3607

b5b42b0

- changed how opacity is updated

osclind requested a review from tobiasdiez March 9, 2021 09:50

tobiasdiez approved these changes Mar 9, 2021

View reviewed changes

calixtus approved these changes Mar 9, 2021

View reviewed changes

calixtus merged commit 5855bcf into JabRef:master Mar 9, 2021

martinfalke mentioned this pull request Mar 15, 2021

Update description of icon for automatically linked files JabRef/user-documentation#353

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve user friendliness of automatically linked files #7484

Improve user friendliness of automatically linked files #7484

osclind commented Mar 3, 2021

calixtus commented Mar 4, 2021

LukasGutenberg commented Mar 4, 2021

calixtus commented Mar 4, 2021

tobiasdiez left a comment

martinfalke commented Mar 4, 2021

LukasGutenberg commented Mar 5, 2021

tobiasdiez commented Mar 5, 2021

LukasGutenberg commented Mar 5, 2021

calixtus commented Mar 5, 2021

LukasGutenberg commented Mar 8, 2021

calixtus commented Mar 8, 2021

LukasGutenberg commented Mar 8, 2021

calixtus commented Mar 8, 2021

LukasGutenberg commented Mar 8, 2021

tobiasdiez left a comment

tobiasdiez Mar 8, 2021

tobiasdiez Mar 8, 2021

tobiasdiez Mar 8, 2021

tobiasdiez Mar 8, 2021

LukasGutenberg commented Mar 8, 2021 •

edited

Loading

LukasGutenberg commented Mar 8, 2021

calixtus commented Mar 8, 2021

calixtus commented Mar 8, 2021

tobiasdiez commented Mar 8, 2021

calixtus commented Mar 8, 2021

LukasGutenberg commented Mar 8, 2021

calixtus Mar 8, 2021

tobiasdiez Mar 8, 2021

LukasGutenberg commented Mar 9, 2021

tobiasdiez left a comment

mlep commented Mar 11, 2021

Improve user friendliness of automatically linked files #7484

Improve user friendliness of automatically linked files #7484

Conversation

osclind commented Mar 3, 2021

Screenshots

Before changes

File not linked

File linked

After changes

File not linked

File linked

calixtus commented Mar 4, 2021

LukasGutenberg commented Mar 4, 2021

calixtus commented Mar 4, 2021

tobiasdiez left a comment

Choose a reason for hiding this comment

martinfalke commented Mar 4, 2021

LukasGutenberg commented Mar 5, 2021

After pressing button

Button to the left

Button where (Auto) was

With opacity

tobiasdiez commented Mar 5, 2021

LukasGutenberg commented Mar 5, 2021

calixtus commented Mar 5, 2021

LukasGutenberg commented Mar 8, 2021

Several automatically found

One added

calixtus commented Mar 8, 2021

LukasGutenberg commented Mar 8, 2021

calixtus commented Mar 8, 2021

LukasGutenberg commented Mar 8, 2021

tobiasdiez left a comment

Choose a reason for hiding this comment

tobiasdiez Mar 8, 2021

Choose a reason for hiding this comment

tobiasdiez Mar 8, 2021

Choose a reason for hiding this comment

tobiasdiez Mar 8, 2021

Choose a reason for hiding this comment

tobiasdiez Mar 8, 2021

Choose a reason for hiding this comment

LukasGutenberg commented Mar 8, 2021 • edited Loading

LukasGutenberg commented Mar 8, 2021

calixtus commented Mar 8, 2021

calixtus commented Mar 8, 2021

tobiasdiez commented Mar 8, 2021

calixtus commented Mar 8, 2021

LukasGutenberg commented Mar 8, 2021

calixtus Mar 8, 2021

Choose a reason for hiding this comment

tobiasdiez Mar 8, 2021

Choose a reason for hiding this comment

LukasGutenberg commented Mar 9, 2021

tobiasdiez left a comment

Choose a reason for hiding this comment

mlep commented Mar 11, 2021

LukasGutenberg commented Mar 8, 2021 •

edited

Loading