Use a Tag Field for Resolve strings and Do not wrap by Zurtar · Pull Request #12876 · JabRef/jabref

Zurtar · 2025-04-01T21:28:27Z

I've updated the EntryTab and EntryTabViewModel classes, to support a list of TagFields instead of deliminated strings, matching the Preferences -> Entry tab to the UI in the Preferences -> Auto Completion.

I tried to follow the implmentation from AutoCompletionTab, I was hoping for some feedback on the naming of the functions and the solution as a whole.

In AutoCompletionTab they use these two functions in order to build the Tags:

    private void setupTagsFiled() {
        autoCompleteFields.setCellFactory(new ViewModelListCellFactory<Field>().withText(Field::getDisplayName));
        autoCompleteFields.setSuggestionProvider(request -> viewModel.getSuggestions(request.getUserText()));
        autoCompleteFields.tagsProperty().bindBidirectional(viewModel.autoCompleteFieldsProperty());
        autoCompleteFields.setConverter(viewModel.getFieldStringConverter());
        autoCompleteFields.setTagViewFactory(this::createTag);
        autoCompleteFields.setShowSearchIcon(false);
        autoCompleteFields.setOnMouseClicked(event -> autoCompleteFields.getEditor().requestFocus());
        autoCompleteFields.getEditor().getStyleClass().clear();
        autoCompleteFields.getEditor().getStyleClass().add("tags-field-editor");
        autoCompleteFields.getEditor().focusedProperty().addListener((observable, oldValue, newValue) -> autoCompleteFields.pseudoClassStateChanged(FOCUSED, newValue));
    }

    private Node createTag(Field field) {
        Label tagLabel = new Label();
        tagLabel.setText(field.getDisplayName());
        tagLabel.setGraphic(IconTheme.JabRefIcons.REMOVE_TAGS.getGraphicNode());
        tagLabel.getGraphic().setOnMouseClicked(event -> autoCompleteFields.removeTags(field));
        tagLabel.setContentDisplay(ContentDisplay.RIGHT);
        return tagLabel;
    }

I think setupTagsFiled is a typo, but I used these methods and created similar ones in EntryTab, but I had them take an argument for TagsField, to let it be used for both "Resolve Strings" and Do not Wrap/Unwrappable

    private void setupResolveTagsForFields() {
            resolvableTagsForFields.setCellFactory(new ViewModelListCellFactory<Field>().withText(Field::getDisplayName));
        resolvableTagsForFields.setSuggestionProvider(request -> viewModel.getSuggestions(request.getUserText()));
        resolvableTagsForFields.tagsProperty().bindBidirectional(viewModel.resolvableTagsForFieldsProperty());
        setupTagsForField(resolvableTagsForFields);
    }

    private void setupNonWrappableFields() {
        nonWrappableFields.setCellFactory(new ViewModelListCellFactory<Field>().withText(Field::getDisplayName));
        nonWrappableFields.setSuggestionProvider(request -> viewModel.getSuggestions(request.getUserText()));
        nonWrappableFields.tagsProperty().bindBidirectional(viewModel.nonWrappableFieldsProperty());
        setupTagsForField(nonWrappableFields);
    }

    private void setupTagsForField(TagsField<Field> tagsField) {
        tagsField.setConverter(viewModel.getFieldStringConverter());
        tagsField.setTagViewFactory(field -> createTag(tagsField, field));
        tagsField.setShowSearchIcon(false);
        tagsField.setOnMouseClicked(event -> tagsField.getEditor().requestFocus());
        tagsField.getEditor().getStyleClass().clear();
        tagsField.getEditor().getStyleClass().add("tags-field-editor");
        tagsField.getEditor().focusedProperty().addListener((observable, oldValue, newValue) -> tagsField.pseudoClassStateChanged(FOCUSED, newValue));
    }

    private Node createTag(TagsField<Field> tagsField, Field field) {
        Label tagLabel = new Label();
        tagLabel.setText(field.getDisplayName());
        tagLabel.setGraphic(IconTheme.JabRefIcons.REMOVE_TAGS.getGraphicNode());
        tagLabel.getGraphic().setOnMouseClicked(event -> tagsField.removeTags(field));
        tagLabel.setContentDisplay(ContentDisplay.RIGHT);
        return tagLabel;
    }

Any feedback would be greatly appreciated, especially with the naming I tried to match existing names as much as possible.

UI Prior to Change:

UI After Change:

Mandatory checks

I own the copyright of the code submitted and I license it under the MIT license
Change in CHANGELOG.md described in a way that is understandable for the average user (if change is visible to the user)
[/] Tests created for changes (if applicable)
Manually tested changed features in running JabRef (always required)
Screenshots added in PR description (if change is visible to the user)
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.

trag-bot · 2025-04-01T21:29:40Z

+        setupTagsForField(nonWrappableFields);
+    }
+
+    private void setupTagsForField(TagsField<Field> tagsField) {


The method setupTagsForField is complex and lacks JavaDoc. Adding JavaDoc would improve code readability and maintainability.

I added that to remove the duplicated code in both setup functions, I wasn't sure if the duplication would be better. It would look like this with the duplicate code:

private void setupResolveTagsForFields() { resolvableTagsForFields.setCellFactory(new ViewModelListCellFactory<Field>().withText(Field::getDisplayName)); resolvableTagsForFields.setSuggestionProvider(request -> viewModel.getSuggestions(request.getUserText())); resolvableTagsForFields.tagsProperty().bindBidirectional(viewModel.resolvableTagsForFieldsProperty()); resolvableTagsForFields.setConverter(viewModel.getFieldStringConverter()); resolvableTagsForFields.setTagViewFactory(field -> createTag(resolvableTagsForFields, field)); resolvableTagsForFields.setShowSearchIcon(false); resolvableTagsForFields.setOnMouseClicked(event -> resolvableTagsForFields.getEditor().requestFocus()); resolvableTagsForFields.getEditor().getStyleClass().clear(); resolvableTagsForFields.getEditor().getStyleClass().add("tags-field-editor"); resolvableTagsForFields.getEditor().focusedProperty().addListener((observable, oldValue, newValue) -> resolvableTagsForFields.pseudoClassStateChanged(FOCUSED, newValue)); } private void setupNonWrappableFields() { nonWrappableFields.setCellFactory(new ViewModelListCellFactory<Field>().withText(Field::getDisplayName)); nonWrappableFields.setSuggestionProvider(request -> viewModel.getSuggestions(request.getUserText())); nonWrappableFields.tagsProperty().bindBidirectional(viewModel.nonWrappableFieldsProperty()); nonWrappableFields.setConverter(viewModel.getFieldStringConverter()); nonWrappableFields.setTagViewFactory(field -> createTag(nonWrappableFields, field)); nonWrappableFields.setShowSearchIcon(false); nonWrappableFields.setOnMouseClicked(event -> nonWrappableFields.getEditor().requestFocus()); nonWrappableFields.getEditor().getStyleClass().clear(); nonWrappableFields.getEditor().getStyleClass().add("tags-field-editor"); nonWrappableFields.getEditor().focusedProperty().addListener((observable, oldValue, newValue) -> nonWrappableFields.pseudoClassStateChanged(FOCUSED, newValue)); } ```

Better to avoid duplicate code. The bot is sometimes just offering suggestions that should be taken with a grain of salt

Zurtar · 2025-04-01T22:37:58Z

After some further testing I realized users can not add custom tags, I also reviewed the other pull request made by the previous assignee and realized I have some similar issues. I'll fix those and push a commit.

I had accidently duplicated a line in the change log, and had left commented out code.

I used the KeywordEditor classes implementation for custom tags, and renamed the fields to match the naming convention I see elsewhere in the project.

…ssue-12550

Siedlerchr · 2025-04-06T16:39:28Z

Tested it works fine. Code lgtm.

Just one UI thing (I have mac os 15 and Preferences -> Override default font settings set to 10) and now the UI text before is displayed with ellipsis
Maybe add some line wrapping/Auto wrapping? If you don't have time for this, I would move it to a new issue.

Added wrapping for "Affected Fields" & "Do not wrap when saving" lables within the Entry tab of preferences.

…ssue-12550

trag-bot · 2025-04-07T14:52:40Z

@trag-bot didn't find any issues in the code! ✅✨

trag-bot · 2025-04-07T14:52:59Z

@trag-bot didn't find any issues in the code! ✅✨

Siedlerchr · 2025-04-07T20:06:19Z

Looks better

Siedlerchr · 2025-04-07T20:21:33Z

Thanks for the contribution and the quick follow up!

* Use a Tag Field for Resolve strings and Do not wrap * Updating ChangeLog.md * Added the ability to use custom tags * Fixing a leftover comment & a mistake in change llog I had accidently duplicated a line in the change log, and had left commented out code. * Fixing line breaks * Custom tags now always show up in suggestions * Added Custom Tag Support I used the KeywordEditor classes implementation for custom tags, and renamed the fields to match the naming convention I see elsewhere in the project. * Added Line Wrapping Added wrapping for "Affected Fields" & "Do not wrap when saving" lables within the Entry tab of preferences.

Zurtar added 2 commits April 1, 2025 16:31

Use a Tag Field for Resolve strings and Do not wrap

f58c079

Updating ChangeLog.md

66fcfb9

trag-bot Bot reviewed Apr 1, 2025

View reviewed changes

Merge branch 'main' into issue-12550

be09642

Added the ability to use custom tags

654de60

trag-bot Bot reviewed Apr 2, 2025

View reviewed changes

Comment thread src/main/java/org/jabref/gui/preferences/entry/EntryTab.java Outdated

Zurtar added 2 commits April 2, 2025 16:16

Fixing a leftover comment & a mistake in change llog

8d142df

I had accidently duplicated a line in the change log, and had left commented out code.

Merge branch 'main' into issue-12550

ce37059

Zurtar requested a review from Siedlerchr April 2, 2025 20:19

Zurtar added 6 commits April 2, 2025 16:23

Fixing line breaks

93f94b2

Custom tags now always show up in suggestions

ad6ca4a

Merge branch 'main' into issue-12550

4d7d537

Added Custom Tag Support

c57bea4

I used the KeywordEditor classes implementation for custom tags, and renamed the fields to match the naming convention I see elsewhere in the project.

Merge branch 'issue-12550' of https://github.com/Zurtar/jabref into i…

51ad19d

…ssue-12550

Merge branch 'main' into issue-12550

961a871

Zurtar marked this pull request as ready for review April 3, 2025 14:26

Merge branch 'main' into issue-12550

763c3a1

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

Siedlerchr previously approved these changes Apr 6, 2025

View reviewed changes

Zurtar added 2 commits April 7, 2025 10:50

Added Line Wrapping

21d09ea

Added wrapping for "Affected Fields" & "Do not wrap when saving" lables within the Entry tab of preferences.

Merge branch 'issue-12550' of https://github.com/Zurtar/jabref into i…

db72f06

…ssue-12550

Zurtar dismissed Siedlerchr’s stale review via db72f06 April 7, 2025 14:50

Merge branch 'main' into issue-12550

5a0b199

Zurtar requested a review from Siedlerchr April 7, 2025 17:09

Siedlerchr approved these changes Apr 7, 2025

View reviewed changes

Siedlerchr added this pull request to the merge queue Apr 7, 2025

Merged via the queue into JabRef:main with commit 0dc9d7d Apr 7, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Use a Tag Field for Resolve strings and Do not wrap#12876

Use a Tag Field for Resolve strings and Do not wrap#12876
Siedlerchr merged 16 commits into
JabRef:mainfrom
Zurtar:issue-12550

Zurtar commented Apr 1, 2025

Uh oh!

trag-bot Bot Apr 1, 2025

Uh oh!

Zurtar Apr 1, 2025 •

edited

Loading

Uh oh!

Siedlerchr Apr 2, 2025 •

edited

Loading

Uh oh!

Zurtar commented Apr 1, 2025

Uh oh!

Uh oh!

Siedlerchr commented Apr 6, 2025

Uh oh!

trag-bot Bot commented Apr 7, 2025

Uh oh!

trag-bot Bot commented Apr 7, 2025

Uh oh!

Siedlerchr commented Apr 7, 2025

Uh oh!

Siedlerchr commented Apr 7, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

Zurtar commented Apr 1, 2025

Mandatory checks

Uh oh!

trag-bot Bot Apr 1, 2025

Choose a reason for hiding this comment

Uh oh!

Zurtar Apr 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Siedlerchr Apr 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Zurtar commented Apr 1, 2025

Uh oh!

Uh oh!

Siedlerchr commented Apr 6, 2025

Uh oh!

trag-bot Bot commented Apr 7, 2025

Uh oh!

trag-bot Bot commented Apr 7, 2025

Uh oh!

Siedlerchr commented Apr 7, 2025

Uh oh!

Siedlerchr commented Apr 7, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Zurtar Apr 1, 2025 •

edited

Loading

Siedlerchr Apr 2, 2025 •

edited

Loading