Automatic field editor: Add Clear content tab + viewmodel #13824
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Siedlerchr
requested changes
Sep 8, 2025
| Field chosen = fieldCombo.getValue(); // keep ComboBox<Field> | ||
| if (chosen != null) { | ||
| viewModel.clearField(chosen); // VM builds/publishes the undo compound | ||
| System.out.println("Cleared field: " + chosen.getName()); |
There was a problem hiding this comment.
No System.out and remove the comments
|
|
||
| List<BibEntry> selected = stateManager.getSelectedEntries(); | ||
| for (BibEntry entry : selected) { | ||
| // clearField returns Optional<FieldChange> |
| List<BibEntry> selected = stateManager.getSelectedEntries(); | ||
| Set<Field> setFields = new LinkedHashSet<>(); | ||
|
|
||
| for (BibEntry entry : selected) { |
There was a problem hiding this comment.
can be refactored into a stream
| private final ClearContentViewModel viewModel; | ||
|
|
||
| public ClearContentTabView(StateManager stateManager) { | ||
| super(); |
There was a problem hiding this comment.
Is there a reasony why you did not creae an fxml? I guess you used AI..
There was a problem hiding this comment.
Yes, I initially used AI to generate the layout in Java so I could focus on getting the logic working first but forgot to move it into a proper FXML file. I've made the changes now.
Siedlerchr
added
the
status: changes-required
|
Sorry, misunderstood PR. Nevermind. |
![trag-bot[bot]](https://avatars.githubusercontent.com/in/856808?s=60&v=4){kind=small}
Comment on lines
+28
to
+33
| public Set<Field> getSetFieldsOnly() { | ||
| return stateManager.getSelectedEntries().stream() | ||
| .flatMap(entry -> entry.getFields().stream() | ||
| .filter(f -> entry.getField(f).isPresent() && !entry.getField(f).get().isBlank())) | ||
| .collect(Collectors.toCollection(LinkedHashSet::new)); | ||
| } |
There was a problem hiding this comment.
The method uses Optional.isPresent() followed by get() which is an anti-pattern. Should use Optional.map() or filter() instead to handle the Optional value in a more functional way.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #13780
This PR introduces a dedicated "Clear content" tab in the Automatic Field Editor:
Steps to test
author,journal).author) and click Clear field content: the field value should be removed from all selected entries.Mandatory checks
CHANGELOG.mdin a way that is understandable for the average user (if change is visible to the user)