Skip to content

Replace setStyle calls with CSS style classes#15939

Closed
SyedAbd110786 wants to merge 8 commits into
JabRef:mainfrom
SyedAbd110786:fix/replace-fonticon-setstyle
Closed

Replace setStyle calls with CSS style classes#15939
SyedAbd110786 wants to merge 8 commits into
JabRef:mainfrom
SyedAbd110786:fix/replace-fonticon-setstyle

Conversation

@SyedAbd110786

@SyedAbd110786 SyedAbd110786 commented Jun 9, 2026

Copy link
Copy Markdown

Related issues and pull requests

Closes #15934

PR Description

Replaced multiple inline setStyle(...) calls across the UI layer with proper CSS style classes, and added corresponding utility classes to jabref-javafx.css. This ensures custom themes such as Atlanta are no longer overridden by hardcoded inline styles.

jabref-contrib-policy:4.2:reviewed:ok

Analogies: Like honey that adapts to any container it is poured into, these style classes flow naturally into any theme. Like chocolate that complements different desserts without overpowering them, the CSS classes work alongside existing stylesheets without forcing their own appearance. Like the moon that reflects light rather than generating its own, these changes defer visual decisions to the theme layer rather than hardcoding them inline.

Steps to test

  1. Open JabRef
  2. Go to Search and perform a fulltext search — verify bold and italic text renders correctly in results
  3. Open any entry with linked files — verify cursor changes to hand on hover
  4. Open Manage Citations dialog — verify bold text appears correct
  5. Open Welcome tab — verify background appears correct
  6. Apply a custom theme from Preferences → Appearance — verify styles are not overridden
Screenshot (47)

AI usage

Claude (claude-sonnet-4-6) was used to assist with identifying affected files and suggesting CSS class replacements. All changes were reviewed, understood, and owned by the contributor.

Checklist

  • I own the copyright of the code submitted and I license it under the MIT license
  • If AI tools were used, I disclosed them in the "AI usage" section and reviewed, understood, and take full ownership of all AI-generated code
  • I manually tested my changes in running JabRef
  • [/] I added JUnit tests for changes (if applicable)
  • [/] I added screenshots in the PR description (if change is visible to the user)
  • I added a screenshot in the PR description showing a library with a single entry with me as author and as title the issue number
  • [/] I described the change in CHANGELOG.md in a way that can be understood by the average user (if change is visible to the user)
  • [/] I checked the user documentation for up to dateness and submitted a pull request to our user documentation repository

- Add utility CSS classes (bold, italic, small-text, bold-italic) to jabref-javafx.css
- Replace inline -fx-font-weight and -fx-font-style setStyle calls with getStyleClass()
- Replace -fx-cursor: hand with setCursor(Cursor.HAND) in FieldValueCell
- Remove empty setStyle call in DeleteFileAction
- Add CSS classes for LinkedFilesEditor padding and WelcomeTab background
- Skip InternalMaterialDesignIcon (dynamic runtime color, intentional)

Fixes JabRef#15934
@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (1) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0)

Grey Divider


Remediation recommended

1. welcome-scroll-pane comment contradicts 📘 Rule violation ⚙ Maintainability
Description
The comment claims that using a class selector is insufficient, but the code now applies a CSS class
(welcome-scroll-pane) instead of an inline style. This misleading comment can confuse maintainers
and cause incorrect future changes.
Code

jabgui/src/main/java/org/jabref/gui/welcome/WelcomeTab.java[229]

+        scrollPane.getStyleClass().add("welcome-scroll-pane"); // Using class selector is insufficient to prevent background from turning white on click.
Evidence
PR Compliance ID 5 requires comments to add value and not mislead/just restate code. The modified
line adds welcome-scroll-pane via getStyleClass().add(...) while the trailing comment states
that a class selector is insufficient, which contradicts the implementation.

AGENTS.md: Use Markdown Javadoc (///) for Multi-line Comments and Avoid Trivial Comments: AGENTS.md: Use Markdown Javadoc (///) for Multi-line Comments and Avoid Trivial Comments: AGENTS.md: Use Markdown Javadoc (///) for Multi-line Comments and Avoid Trivial Comments: AGENTS.md: Use Markdown Javadoc (///) for Multi-line Comments and Avoid Trivial Comments: AGENTS.md: Use Markdown Javadoc (///) for Multi-line Comments and Avoid Trivial Comments: AGENTS.md: Use Markdown Javadoc (///) for Multi-line Comments and Avoid Trivial Comments: AGENTS.md: Use Markdown Javadoc (///) for Multi-line Comments and Avoid Trivial Comments: AGENTS.md: Use Markdown Javadoc (///) for Multi-line Comments and Avoid Trivial Comments
jabgui/src/main/java/org/jabref/gui/welcome/WelcomeTab.java[229-229]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A line comment contradicts the updated implementation: it states that using a class selector is insufficient, but the code now uses a CSS class (`welcome-scroll-pane`). This is misleading documentation.
## Issue Context
The PR replaces inline `setStyle(...)` with CSS classes. The current comment appears to have been written for the previous inline-style approach and should be updated to reflect the new rationale (or removed if no longer needed).
## Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/welcome/WelcomeTab.java[229-229]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Welcome scroll bg regression 🐞 Bug ≡ Correctness
Description
WelcomeTab replaces an inline transparent background style with the 'welcome-scroll-pane' style
class, but this does not override the existing '.scroll-pane:focused' rule, so the scroll pane can
still turn opaque/white when clicked/focused. This reintroduces the exact behavior the comment warns
about.
Code

jabgui/src/main/java/org/jabref/gui/welcome/WelcomeTab.java[229]

+        scrollPane.getStyleClass().add("welcome-scroll-pane"); // Using class selector is insufficient to prevent background from turning white on click.
Evidence
The base stylesheet explicitly sets focused scroll panes to a non-transparent background. The new
.welcome-scroll-pane rule does not include :focused, so it cannot override
.scroll-pane:focused due to lower selector specificity, allowing the white/opaque background to
reappear on focus/click.

jabgui/src/main/java/org/jabref/gui/welcome/WelcomeTab.java[224-231]
jabgui/src/main/resources/org/jabref/gui/jabref-javafx.css[206-215]
jabgui/src/main/resources/org/jabref/gui/jabref-javafx.css[922-924]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`WelcomeTab` no longer sets an inline `-fx-background-color: transparent` on the `ScrollPane` and instead adds the `welcome-scroll-pane` style class. However, `jabref-javafx.css` has a more specific `.scroll-pane:focused` rule that sets a non-transparent background, so the scroll pane can still turn white/opaque when focused (clicked).
### Issue Context
The code comment explicitly notes that a class selector was previously insufficient to prevent the white background on click. The newly added `.welcome-scroll-pane` rule does not cover the focused state and therefore cannot reliably prevent the focused background styling.
### Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/welcome/WelcomeTab.java[224-231]
- jabgui/src/main/resources/org/jabref/gui/jabref-javafx.css[206-215]
- jabgui/src/main/resources/org/jabref/gui/jabref-javafx.css[922-924]
### Suggested fix
In `jabref-javafx.css`, add a focused override with higher/equal specificity, e.g.:

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Hey @SyedAbd110786! 👋

Thank you for contributing to JabRef!

We have automated checks in place, based on which you will soon get feedback if any of them are failing. We also use Qodo for review assistance. It will update your pull request description with a review help and offer suggestions to improve the pull request.

After all automated checks pass, a maintainer will also review your contribution. Once that happens, you can go through their comments in the "Files changed" tab and act on them, or reply to the conversation if you have further inputs. You can read about the whole pull request process in our contribution guide.

Please ensure that your pull request is in line with our AI Usage Policy and make necessary disclosures.

@github-actions github-actions Bot added first contrib good first issue An issue intended for project-newcomers. Varies in difficulty. component: ui labels Jun 9, 2026
@qodo-free-for-open-source-projects

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Replace JavaFX inline setStyle() usage with reusable CSS classes
🐞 Bug fix ✨ Enhancement 🕐 20-40 Minutes

Grey Divider

Walkthroughs

User Description

Related issues and pull requests

Closes #15934

PR Description

Replaced multiple inline setStyle(...) calls across the UI layer with proper CSS style classes, and added corresponding utility classes to jabref-javafx.css. This ensures custom themes such as Atlanta are no longer overridden by hardcoded inline styles.

jabref-contrib-policy:4.2:reviewed:ok

Analogies: Like honey that adapts to any container it is poured into, these style classes flow naturally into any theme. Like chocolate that complements different desserts without overpowering them, the CSS classes work alongside existing stylesheets without forcing their own appearance. Like the moon that reflects light rather than generating its own, these changes defer visual decisions to the theme layer rather than hardcoding them inline.

Steps to test

  1. Open JabRef
  2. Go to Search and perform a fulltext search — verify bold and italic text renders correctly in results
  3. Open any entry with linked files — verify cursor changes to hand on hover
  4. Open Manage Citations dialog — verify bold text appears correct
  5. Open Welcome tab — verify background appears correct
  6. Apply a custom theme from Preferences → Appearance — verify styles are not overridden
Screenshot (47)

AI usage

Claude (claude-sonnet-4-6) was used to assist with identifying affected files and suggesting CSS class replacements. All changes were reviewed, understood, and owned by the contributor.

Checklist

  • I own the copyright of the code submitted and I license it under the MIT license
  • If AI tools were used, I disclosed them in the "AI usage" section and reviewed, understood, and take full ownership of all AI-generated code
  • I manually tested my changes in running JabRef
  • [/] I added JUnit tests for changes (if applicable)
  • [/] I added screenshots in the PR description (if change is visible to the user)
  • I added a screenshot in the PR description showing a library with a single entry with me as author and as title the issue number
  • [/] I described the change in CHANGELOG.md in a way that can be understood by the average user (if change is visible to the user)
  • [/] I checked the user documentation for up to dateness and submitted a pull request to our user documentation repository
AI Description
• Replace inline JavaFX styles with CSS classes to avoid overriding custom themes
• Add reusable utility classes (bold/italic/small-text) to the global stylesheet
• Switch cursor styling to the JavaFX Cursor API for consistent hover behavior
Diagram
graph TD
  A["FileAnnotationTabView"] --> B["FulltextSearchResultsTab"] --> C["ManageCitationsDialogView"] --> D["LinkedFilesEditor"] --> E["WelcomeTab"] --> F("jabref-javafx.css")
  G["FieldValueCell"] --> H["JavaFX Cursor API"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Use component-scoped style classes only (no global utility classes)
  • ➕ Avoids a growing set of generic utility classes
  • ➕ Keeps styling semantics tied to a specific UI component
  • ➕ Reduces accidental cross-UI coupling via shared class names
  • ➖ More CSS duplication for common patterns like bold/italic text
  • ➖ Harder to apply consistent emphasis styling across multiple views
2. Use JavaFX pseudo-classes for emphasis states
  • ➕ More semantic than generic classes (e.g., 'emphasis' state)
  • ➕ Can be toggled programmatically without class list management
  • ➖ More code complexity to define/propagate pseudo-class state
  • ➖ Less straightforward for static styling like fixed small text
3. Adopt theme variables/lookups instead of fixed values (e.g., no 0.75em utility)
  • ➕ Maximizes theme control and accessibility (font scaling/contrast)
  • ➕ Avoids encoding typography decisions into shared utilities
  • ➖ Requires additional theme infrastructure and conventions
  • ➖ May be overkill for simple formatting needs

Recommendation: The chosen approach (moving inline styles into stylesheet classes) is the right fix for theme compatibility and maintainability. Keep utility classes limited to truly reusable typography patterns (bold/italic/bold-italic) and prefer component-specific classes (like linked-files-info, welcome-scroll-pane) when a rule is layout/structure-related to avoid utility-class sprawl.

Grey Divider

File Changes

Bug fix (2)
FieldValueCell.java Use JavaFX cursor API instead of inline cursor CSS +1/-1

Use JavaFX cursor API instead of inline cursor CSS

• Replaces '-fx-cursor: hand' setStyle() with label.setCursor(Cursor.HAND). This keeps hover intent while avoiding inline CSS that can be overridden unpredictably across themes.

jabgui/src/main/java/org/jabref/gui/mergeentries/threewaymerge/cell/FieldValueCell.java


WelcomeTab.java Move WelcomeTab scroll background styling to stylesheet class +1/-1

Move WelcomeTab scroll background styling to stylesheet class

• Replaces an inline transparent background setStyle() with the 'welcome-scroll-pane' style class. This reduces theme conflicts while preserving the intended transparent background behavior.

jabgui/src/main/java/org/jabref/gui/welcome/WelcomeTab.java


Refactor (5)
FileAnnotationTabView.java Replace inline font styling with small-text/bold CSS classes +1/-1

Replace inline font styling with small-text/bold CSS classes

• Removes an inline setStyle() that enforced font size and weight. Applies the equivalent styling via the shared 'small-text' and 'bold' CSS classes to preserve theme control.

jabgui/src/main/java/org/jabref/gui/entryeditor/fileannotationtab/FileAnnotationTabView.java


FulltextSearchResultsTab.java Apply bold/italic styling via CSS classes in search results +3/-3

Apply bold/italic styling via CSS classes in search results

• Replaces inline styles on Text nodes with 'italic', 'bold', and 'bold-italic' classes. This prevents hardcoded typography from overriding theme styling while retaining emphasis in result rendering.

jabgui/src/main/java/org/jabref/gui/entryeditor/fileannotationtab/FulltextSearchResultsTab.java


LinkedFilesEditor.java Move LinkedFilesEditor padding to a named CSS class +1/-1

Move LinkedFilesEditor padding to a named CSS class

• Replaces an inline padding style on the info HBox with the 'linked-files-info' CSS class. Keeps layout alignment behavior while allowing theme overrides.

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


DeleteFileAction.java Remove unused header inline styling in delete-files dialog +1/-3

Remove unused header inline styling in delete-files dialog

• Deletes a multi-line inline style block on the dialog header label. This eliminates hardcoded padding/background that could conflict with themed styling.

jabgui/src/main/java/org/jabref/gui/linkedfile/DeleteFileAction.java


ManageCitationsDialogView.java Replace inline bold styling with shared CSS class +1/-1

Replace inline bold styling with shared CSS class

• Removes an inline bold style on the middle Text segment and applies the 'bold' style class instead. Ensures emphasis remains but does not hardcode appearance against theme rules.

jabgui/src/main/java/org/jabref/gui/openoffice/ManageCitationsDialogView.java


Other (1)
jabref-javafx.css Add reusable typography utilities and component-specific style classes +23/-0

Add reusable typography utilities and component-specific style classes

• Introduces utility classes for bold, italic, small text, and bold-italic, plus component classes for linked file padding and welcome scroll pane background. Centralizes styling so custom themes can override consistently.

jabgui/src/main/resources/org/jabref/gui/jabref-javafx.css


Grey Divider

Qodo Logo

Comment thread jabgui/src/main/resources/org/jabref/gui/jabref-javafx.css Outdated

@Maran23 Maran23 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

inline comment

@github-actions github-actions Bot added the status: changes-required Pull requests that are not yet complete label Jun 9, 2026
@SyedAbd110786

Copy link
Copy Markdown
Author

Thank you for the feedback! I have fixed the following:

  • DeleteFileAction: restored the setStyle with proper CSS class
    delete-file-dialog-header in jabref-theme.css instead of removing it
  • Removed incorrectly added classes from jabref-javafx.css
  • Added .italic class to jabref-theme.css (correct file)
  • Replaced bold-italic with addAll("bold", "italic")
    This is my first contribution to JabRef. I appreciate your patience and detailed feedback.

@Maran23 Maran23 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You again missed some styleClasses - they do not exist anymore.
If you want to make Open Source contributions please check and TEST your changes MANUALLY, or we will close your PR.

@SyedAbd110786 SyedAbd110786 requested a review from Maran23 June 10, 2026 12:00
@github-actions

Copy link
Copy Markdown
Contributor

Do not request reviews if changes are required.
Address the changes first.

@koppor koppor removed the request for review from Maran23 June 10, 2026 12:01
@SyedAbd110786

Copy link
Copy Markdown
Author

I found the missing style classes and added them to the appropriate CSS file. The changes have been committed and pushed in the latest update.

This is my first open-source PR, so I appreciate your patience and guidance. When you have time, could you please take another look and let me know if there is anything else that needs to be adjusted?

@Maran23

Maran23 commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

This is my first open-source PR, so I appreciate your patience and guidance.

This is fine and I'm aware. But no excuse for not testing your code changes - which can lead to servere regressions for users of JabRef.

-fx-font-size: 0.75em;
}

.delete-file-dialog-header {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this one is unused.

@Maran23

Maran23 commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

Closing as per my comments above. Feel free to revisit when you completely understand the details and Open Source.

@Maran23 Maran23 closed this Jun 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component: ui first contrib good first issue An issue intended for project-newcomers. Varies in difficulty. status: changes-required Pull requests that are not yet complete

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Replace all FontIcon.setStyle calls with getStyleClass

2 participants