Fix for group focus after removing the filter (14084)#14883
Fix for group focus after removing the filter (14084)#14883koppor merged 13 commits intoJabRef:mainfrom
Conversation
|
Hey @ZiadAbdElFatah! 👋 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. |
8730280 to
5c020ce
Compare
|
I need help here.
I tried to reformat my code but when typed |
|
This is my first pr with JabRef, i went with the docs step by step, in this step of checkstyle setup When i tried to restore the file to its original state the JavaDoc formatting is disabled. I din't know if this is a bug or what. |
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
||||||||||||||||||||||||||||||||
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
||||||||||||||||||||||||||||
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||
|
Ignore the comments above, i saw this #14860 got merged, so i updated the branch and everything is fine now. |
| final String filterText = searchField.textProperty().getValue(); | ||
| if (filterText == null || filterText.isEmpty()) { | ||
| Platform.runLater(() -> { | ||
| viewModel.selectedGroupsProperty().setAll(previouslySelectedGroup); |
There was a problem hiding this comment.
Does this work even if the current group is not shown? -- Java comment needed
There was a problem hiding this comment.
I just tried it and noticed that if the current group is not shown, it can't be selected.
| final List<GroupNodeViewModel> previouslySelectedGroup = new ArrayList<>(viewModel.selectedGroupsProperty()); | ||
| viewModel.filterTextProperty().setValue(searchField.textProperty().getValue()); | ||
| final String filterText = searchField.textProperty().getValue(); | ||
| if (filterText == null || filterText.isEmpty()) { |
There was a problem hiding this comment.
Can filterText be null in some cases? I know that AI generates this code, but I want to add some confidence.
There was a problem hiding this comment.
I would like to add confidence too. So well, for the code logic it is 100% my logic no any AI logic here. For the code itself, I wrote the code at first and gave it to ChatGPT to review it, it told me to add filterText == null to be more secure so i added it.
I don't have AI agent integrated in intillij so there will not be any code generated with AI only, and be sure that I will not submit a single line of code without fully understand it.
There was a problem hiding this comment.
I would like to add confidence too.
What is the result? Logger used? Breakpoint used?
There was a problem hiding this comment.
I would like to add confidence too.
What is the result? Logger used? Breakpoint used?
There was a problem hiding this comment.
Is this reply on the correct comment?
If it is so, I don't really relate.
| LOGGER.debug("Run group search {}", searchField.getText()); | ||
|
|
||
| final List<GroupNodeViewModel> previouslySelectedGroup = new ArrayList<>(viewModel.selectedGroupsProperty()); | ||
| viewModel.filterTextProperty().setValue(searchField.textProperty().getValue()); |
There was a problem hiding this comment.
Isn't the main reason at the listeners running when setValue is called?
There was a problem hiding this comment.
After implementing the solution in this pr, I spent about two days trying to fix the problem from its core and tried to fix the listeners that interact with the selected groups after changing the filter but I couldn't find it, but I am still trying.
A hint would be appreciated a lot.
There was a problem hiding this comment.
If we keep this solution, a short Java comment should be put above
Maybe
/// Ensure that group selection is kept after clearance of searchThere was a problem hiding this comment.
I like the solution, because it also helps when typing something in the field.
|
I think this solution is better, where the selection stays as long as the user decides to change it. Whether the selected group is shown or not. |
|
@ZiadAbdElFatah Please add a CHANGELOG.md entry. |
koppor
left a comment
There was a problem hiding this comment.
Add CHANGELOG.md entry - and a small java comment, then this is good to go.
| LOGGER.debug("Run group search {}", searchField.getText()); | ||
|
|
||
| final List<GroupNodeViewModel> previouslySelectedGroup = new ArrayList<>(viewModel.selectedGroupsProperty()); | ||
| viewModel.filterTextProperty().setValue(searchField.textProperty().getValue()); |
There was a problem hiding this comment.
If we keep this solution, a short Java comment should be put above
Maybe
/// Ensure that group selection is kept after clearance of search| LOGGER.debug("Run group search {}", searchField.getText()); | ||
|
|
||
| final List<GroupNodeViewModel> previouslySelectedGroup = new ArrayList<>(viewModel.selectedGroupsProperty()); | ||
| viewModel.filterTextProperty().setValue(searchField.textProperty().getValue()); |
There was a problem hiding this comment.
I like the solution, because it also helps when typing something in the field.
Ok, that is nice. I am sorry I will be out today, once I return tomorrow I will make these changes. |
|
Your pull request conflicts with the target branch. Please merge with your code. For a step-by-step guide to resolve merge conflicts, see https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/addressing-merge-conflicts/resolving-a-merge-conflict-using-the-command-line. |
|
@koppor sorry for the mention, but is there anything else remaining to merge this pr? |



User description
Closes 14084
I saved the group selected after applying a filter, and when the filter changes, I made sure that the new filter isn't empty to ensure the case of clearing, if it is empty I assign the group selected before as the current selected one.
Steps to test
1.


2.
3.
Mandatory checks
CHANGELOG.mdin a way that is understandable for the average user (if change is visible to the user)PR Type
Bug fix
Description
Preserves group selection when clearing filter text
Saves selected groups before filter change, restores if filter becomes empty
Uses Platform.runLater to ensure UI update after filter change
Removes unnecessary ENABLE_JAVADOC_FORMATTING configuration option
Diagram Walkthrough
File Walkthrough
GroupTreeView.java
Restore group selection on filter clearjabgui/src/main/java/org/jabref/gui/groups/GroupTreeView.java
Project.xml
Remove javadoc formatting configuration.idea/codeStyles/Project.xml