Make DOI lookup available again by koppor · Pull Request #14931 · JabRef/jabref

koppor · 2026-01-26T07:24:53Z

User description

Follow-up to #14831

Buttons are available again
JavaFX code style - not plain hacking

Instead of

Have this

I need this for my daily workflow, so I auto merge.

Steps to test

Play around with a DOI field.

Mandatory checks

I own the copyright of the code submitted and I license it under the MIT license
I manually tested my changes in running JabRef (always required)
[/] I added JUnit tests for changes (if applicable)
I added screenshots in the PR description (if change is visible to the user)
I described the change in CHANGELOG.md in a way that is understandable for the average user (if change is visible to the user)
I checked the user 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 updating file(s) in https://github.com/JabRef/user-documentation/tree/main/en.

PR Type

Enhancement, Bug fix

Description

Restore DOI lookup buttons with improved JavaFX binding approach
Move button visibility/disable logic from Java code to FXML
Add helper method for identifier lookup progress state binding
Update UX documentation with button disable vs hide pattern
Fix CHANGELOG entry to clarify button disable behavior

Diagram Walkthrough

flowchart LR
  A["Java Code Logic"] -->|Removed Stream-based bindings| B["FXML Bindings"]
  C["BaseIdentifierEditorViewModel"] -->|Add getIdentifierLookupNotInProgress| D["IdentifierEditor.fxml"]
  D -->|Use new property binding| E["Button visibility/disable"]
  F["UX Requirements"] -->|Document disable pattern| G["Updated ux.md"]

File Walkthrough

Relevant files

Enhancement

BaseIdentifierEditorViewModel.java `Add helper method for lookup progress state` jabgui/src/main/java/org/jabref/gui/fieldeditors/identifier/BaseIdentifierEditorViewModel.java Add new public method `getIdentifierLookupNotInProgress()` to expose inverted lookup progress state Provides convenient binding target for FXML to control button icon visibility	+4/-0
IdentifierEditor.fxml `Refactor FXML bindings for button state management` jabgui/src/main/resources/org/jabref/gui/fieldeditors/identifier/IdentifierEditor.fxml Reorganize button definitions with improved formatting and spacing Move `disable` attribute to consistent position in button declarations Replace `identifierLookupInProgress == false` with new `identifierLookupNotInProgress` binding Update `lookupIdentifierButton` to use new property for icon visibility control Ensure all buttons use FXML-based visibility and disable bindings	+14/-5

Refactoring

IdentifierEditor.java `Move button binding logic to FXML` jabgui/src/main/java/org/jabref/gui/fieldeditors/identifier/IdentifierEditor.java Remove Stream-based button binding logic from constructor Remove conditional visibility logic for shortenDOIButton based on field type Remove unused imports (`Stream`, `StandardField`) Add comment indicating button visibility/disable is now handled in FXML	+1/-15

Documentation

CHANGELOG.md `Clarify button disable behavior in changelog` CHANGELOG.md Update entry to clarify that buttons are disabled rather than hidden Change reference from PR to issue number for consistency Improve user-facing description of the fix	+1/-1
ux.md `Document button disable vs hide UX pattern` docs/requirements/ux.md Add new UX requirement section on button disable vs hide pattern Document that generally available functionality should be disabled when unavailable Provide concrete example of link button behavior	+7/-1

qodo-free-for-open-source-projects · 2026-01-26T07:25:26Z

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

qodo-free-for-open-source-projects · 2026-01-26T07:26:44Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	✅ ~~Fix broken UI reactivity~~ Suggestion Impact: The suggestion was directly implemented in the commit. The new identifierLookupNotInProgressProperty() method was added that returns a BooleanBinding, and the getIdentifierLookupNotInProgress() method was refactored to call this new property method, exactly as suggested. code diff: + public BooleanBinding identifierLookupNotInProgressProperty() { + return identifierLookupInProgress.not(); + } + public boolean getIdentifierLookupNotInProgress() { - return identifierLookupInProgress.not().get(); + return identifierLookupNotInProgressProperty().get(); Add an `identifierLookupNotInProgressProperty()` method to ensure the UI binding is reactive. The current implementation will only set the value once and will not update when the underlying property changes. jabgui/src/main/java/org/jabref/gui/fieldeditors/identifier/BaseIdentifierEditorViewModel.java [138-140] -public boolean getIdentifierLookupNotInProgress() { - return identifierLookupInProgress.not().get(); +public BooleanBinding identifierLookupNotInProgressProperty() { + return identifierLookupInProgress.not(); } +public boolean getIdentifierLookupNotInProgress() { + return identifierLookupNotInProgressProperty().get(); +} + `[Suggestion processed]` Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies that the FXML binding for `identifierLookupNotInProgress` will not be reactive without a corresponding `...Property()` method, which would break the intended UI behavior of toggling icon visibility.	Medium
Learned best practice	✅ ~~Improve changelog entry clarity~~ Suggestion Impact: The commit directly implements the suggested change by replacing the vague phrase "are now disabled if they cannot be used" with the more specific "are now disabled when the field is empty or contains an invalid identifier" code diff: -- Buttons in empty identifier fields (DOI, Eprint, ISBN) are now disabled if they cannot be used. [#14821](https://github.com/JabRef/jabref/issues/14821) +- Buttons in identifier fields (DOI, Eprint, ISBN) are now disabled when the field is empty or contains an invalid identifier. [#14821](https://github.com/JabRef/jabref/issues/14821) The changelog entry should clarify that buttons are disabled when the field is empty or invalid, not just when they "cannot be used" which is vague. Be more specific about the conditions. CHANGELOG.md [33] -- Buttons in empty identifier fields (DOI, Eprint, ISBN) are now disabled if they cannot be used. [#14821](https://github.com/JabRef/jabref/issues/14821) +- Buttons in identifier fields (DOI, Eprint, ISBN) are now disabled when the field is empty or contains an invalid identifier. [#14821](https://github.com/JabRef/jabref/issues/14821) `[Suggestion processed]` Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Fix typographical errors in comments, documentation, changelog entries, and user-facing strings to maintain professionalism and clarity	Low
Update

Co-authored-by: qodo-free-for-open-source-projects[bot] <189517486+qodo-free-for-open-source-projects[bot]@users.noreply.github.com>

Copilot

Pull request overview

This PR reverts changes from PR #14831 that hid identifier action buttons when fields were empty. The new approach follows a UX principle where generally available functionality should be shown as disabled rather than hidden. The changes move button visibility and disable logic from Java code to FXML declarations for a cleaner, more declarative JavaFX style.

Changes:

Removed Java code that bound button visibility to field text state (empty vs non-empty)
Reformatted FXML for better readability with proper attribute alignment
Added UX requirement documentation specifying that generally available buttons should be disabled rather than hidden
Updated CHANGELOG to reflect the new behavior

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
IdentifierEditor.fxml	Reformatted button declarations for better readability; attempted to use new property for negated lookup progress state
IdentifierEditor.java	Removed visibility binding logic that hid buttons when field was empty; added comment about FXML handling
BaseIdentifierEditorViewModel.java	Added getter method for negated identifier lookup progress state
docs/requirements/ux.md	Added UX requirement documentation about showing disabled buttons instead of hiding them
CHANGELOG.md	Updated description to reflect that buttons are now disabled instead of hidden

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-26T07:34:35Z

-            shortenDOIButton.visibleProperty().unbind();
-            shortenDOIButton.setVisible(false);
-        }
+        // whether a button is shown or disabled is done in .fmxl


Typo in comment: "fmxl" should be "fxml".

Suggested change

// whether a button is shown or disabled is done in .fmxl

// whether a button is shown or disabled is done in .fxml

Copilot · 2026-01-26T07:34:36Z

+    public BooleanBinding identifierLookupNotInProgressProperty() {
+        return identifierLookupInProgress.not();


This getter method is insufficient for JavaFX FXML binding. When using property bindings like ${controller.viewModel.identifierLookupNotInProgress} in FXML, JavaFX looks for a method named identifierLookupNotInProgressProperty() that returns a BooleanProperty, not just a getter. The current implementation will fail at runtime when the FXML tries to bind to this property. You need to either:

Add a BooleanProperty identifierLookupNotInProgressProperty() method that returns a binding like identifierLookupInProgress.not(), or

Use the expression ${!controller.viewModel.identifierLookupInProgress} in the FXML instead (as done in CitationCountEditor.fxml line 16)

Suggested change

public BooleanBinding identifierLookupNotInProgressProperty() {

return identifierLookupInProgress.not();

private final BooleanProperty identifierLookupNotInProgress = new SimpleBooleanProperty();

public BooleanProperty identifierLookupNotInProgressProperty() {

if (!identifierLookupNotInProgress.isBound()) {

identifierLookupNotInProgress.bind(identifierLookupInProgress.not());

}

return identifierLookupNotInProgress;

Copilot · 2026-01-26T07:34:36Z

            <StackPane>
                <JabRefIconView glyph="LOOKUP_IDENTIFIER"
-                                visible="${controller.viewModel.identifierLookupInProgress == false}"/>
+                                visible="${controller.viewModel.identifierLookupNotInProgress}"/>


This FXML binding will fail at runtime. JavaFX property bindings require a method named identifierLookupNotInProgressProperty() that returns a BooleanProperty, but only a getter method exists. Consider using ${!controller.viewModel.identifierLookupInProgress} instead, which is the pattern used in CitationCountEditor.fxml for a similar case.

Suggested change

visible="${controller.viewModel.identifierLookupNotInProgress}"/>

visible="${!controller.viewModel.identifierLookupInProgress}"/>

koppor added 2 commits January 26, 2026 08:21

Make DOI lookup available again

a4f21d6

Fix CHANGELOG.md item

7fae0e6

koppor requested a review from Copilot January 26, 2026 07:24

koppor added the automerge PR is tagged with that label will be merged if workflows are green label Jan 26, 2026

jabref-machine approved these changes Jan 26, 2026

View reviewed changes

jabref-machine previously approved these changes Jan 26, 2026

View reviewed changes

koppor enabled auto-merge January 26, 2026 07:25

Copilot started reviewing on behalf of koppor January 26, 2026 07:25 View session

qodo-free-for-open-source-projects Bot added the Review effort 2/5 label Jan 26, 2026

koppor mentioned this pull request Jan 26, 2026

Fix #14821: Hide identifier action buttons when field is empty #14831

Merged

6 tasks

qodo-free-for-open-source-projects Bot reviewed Jan 26, 2026

View reviewed changes

Comment thread CHANGELOG.md Outdated

qodo-free-for-open-source-projects Bot reviewed Jan 26, 2026

View reviewed changes

Comment thread jabgui/src/main/java/org/jabref/gui/fieldeditors/identifier/BaseIdentifierEditorViewModel.java

Apply suggestions from code review

b2b840c

Co-authored-by: qodo-free-for-open-source-projects[bot] <189517486+qodo-free-for-open-source-projects[bot]@users.noreply.github.com>

koppor dismissed jabref-machine’s stale review via b2b840c January 26, 2026 07:29

jabref-machine previously approved these changes Jan 26, 2026

View reviewed changes

REfine CHANGELOG.md

c19138b

koppor dismissed jabref-machine’s stale review via c19138b January 26, 2026 07:30

jabref-machine previously approved these changes Jan 26, 2026

View reviewed changes

Copilot AI reviewed Jan 26, 2026

View reviewed changes

Compile fix

95a49b0

koppor dismissed jabref-machine’s stale review via 95a49b0 January 26, 2026 08:18

jabref-machine approved these changes Jan 26, 2026

View reviewed changes

koppor added this pull request to the merge queue Jan 26, 2026

github-actions Bot added the status: to-be-merged PRs which are accepted and should go into the merge-queue. label Jan 26, 2026

Merged via the queue into main with commit 973f827 Jan 26, 2026
57 checks passed

koppor deleted the fix-lookup branch January 26, 2026 08:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Make DOI lookup available again#14931

Make DOI lookup available again#14931
koppor merged 5 commits into
mainfrom
fix-lookup

koppor commented Jan 26, 2026 •

edited by qodo-free-for-open-source-projects Bot

Loading

Uh oh!

qodo-free-for-open-source-projects Bot commented Jan 26, 2026 •

edited

Loading

Uh oh!

qodo-free-for-open-source-projects Bot commented Jan 26, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 26, 2026

Uh oh!

Copilot AI Jan 26, 2026

Uh oh!

Copilot AI Jan 26, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	// whether a button is shown or disabled is done in .fmxl
	// whether a button is shown or disabled is done in .fxml

		public BooleanBinding identifierLookupNotInProgressProperty() {
		return identifierLookupInProgress.not();

-    public BooleanBinding identifierLookupNotInProgressProperty() {
-        return identifierLookupInProgress.not();
+    private final BooleanProperty identifierLookupNotInProgress = new SimpleBooleanProperty();
+    public BooleanProperty identifierLookupNotInProgressProperty() {
+        if (!identifierLookupNotInProgress.isBound()) {
+            identifierLookupNotInProgress.bind(identifierLookupInProgress.not());
+        }
+        return identifierLookupNotInProgress;

	visible="${controller.viewModel.identifierLookupNotInProgress}"/>
	visible="${!controller.viewModel.identifierLookupInProgress}"/>

Uh oh!

Conversation

koppor commented Jan 26, 2026 • edited by qodo-free-for-open-source-projects Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

User description

Steps to test

Mandatory checks

PR Type

Description

Diagram Walkthrough

File Walkthrough

Uh oh!

qodo-free-for-open-source-projects Bot commented Jan 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Compliance Guide 🔍

Uh oh!

qodo-free-for-open-source-projects Bot commented Jan 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Code Suggestions ✨

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

koppor commented Jan 26, 2026 •

edited by qodo-free-for-open-source-projects Bot

Loading

qodo-free-for-open-source-projects Bot commented Jan 26, 2026 •

edited

Loading

qodo-free-for-open-source-projects Bot commented Jan 26, 2026 •

edited

Loading