Fix: Refresh groupTree after removing group (fixes #11487)#12815
Conversation
|
Does this also coincidentally fix #8012 ? |
I don't think so, but the problem may be solved in a similar way, because only the edit command had a groupTree.refresh(); after. EDIT: |
|
@almada39 There is an option to "reopen" pull requests at the same place where you closed it from. I reopened it. |
| case GROUP_REMOVE -> { | ||
| viewModel.removeGroupNoSubgroups(group); | ||
| case GROUP_REMOVE_KEEP_SUBGROUPS -> | ||
| groupTree.refresh(); |
There was a problem hiding this comment.
The refresh() call is duplicated across multiple case blocks. This violates DRY principle and should be moved after the switch statement to avoid code duplication.
There was a problem hiding this comment.
BTW, what if you really add groupTree.refresh() every time there is an action?
Will that introduce more issues? I think it shouldn't.
Performance problems? I think no.
There was a problem hiding this comment.
I don't think it would be a huge problem either, I didn't do it before because i thought it wasn't there because of performance problems. This said would you like me to make a new commit with all the commands refreshing the groupTree after executing?
In that case should I also add the change in the changelog file? I didn't do it because I wasn't sure if the small change in the code was enough to write it in the file.
There was a problem hiding this comment.
In case all things will do a referewsh, please collect it at the end.
Regarding CHANGELOG.md: Independent of the amount of statements needed to fix something, it should be listed there; because the behavior of JabRef changed.
|
Note that your PR will not be accepted/reviewed until you have gone through the mandatory checks in the description and mark them as one of |
|
@almada39 Please add a changelog entry. |
|
Your pull request conflicts with the target branch. Please merge |
|
@trag-bot didn't find any issues in the code! ✅✨ |
…Ref#12815) * Fix JabRef#11487: Refresh groupTree after removing group * Refactor: Refresh groupTree after any group command --------- Co-authored-by: Subhramit Basu <subhramit.bb@live.in>
…Ref#12815) * Fix JabRef#11487: Refresh groupTree after removing group * Refactor: Refresh groupTree after any group command --------- Co-authored-by: Subhramit Basu <subhramit.bb@live.in>
…Ref#12815) * Fix JabRef#11487: Refresh groupTree after removing group * Refactor: Refresh groupTree after any group command --------- Co-authored-by: Subhramit Basu <subhramit.bb@live.in>
closes #8012 closes #12815 closes #8217 Not sure, what commit exactly fixed all these issues, but since they have been very annoying to users, I think it's something worth mentioning in the changelog that they seem to be gone. Successful commits could be found via [Git-bisect](https://git-scm.com/docs/git-bisect) JabRef has become better and the 6.0 release is something to look forward to!
* Add fixed IndexOutOfBoundsException to CHANGELOG.md closes #8012 closes #12815 closes #8217 Not sure, what commit exactly fixed all these issues, but since they have been very annoying to users, I think it's something worth mentioning in the changelog that they seem to be gone. Successful commits could be found via [Git-bisect](https://git-scm.com/docs/git-bisect) JabRef has become better and the 6.0 release is something to look forward to! * Apply review comment Co-authored-by: Subhramit Basu <subhramit.bb@live.in> * Add #8281 --------- Co-authored-by: Subhramit Basu <subhramit.bb@live.in>
* Add fixed IndexOutOfBoundsException to CHANGELOG.md closes JabRef#8012 closes JabRef#12815 closes JabRef#8217 Not sure, what commit exactly fixed all these issues, but since they have been very annoying to users, I think it's something worth mentioning in the changelog that they seem to be gone. Successful commits could be found via [Git-bisect](https://git-scm.com/docs/git-bisect) JabRef has become better and the 6.0 release is something to look forward to! * Apply review comment Co-authored-by: Subhramit Basu <subhramit.bb@live.in> * Add JabRef#8281 --------- Co-authored-by: Subhramit Basu <subhramit.bb@live.in>
Closes #11487
Problem:
Previously, when a group was removed, it would still appear on hover, and users could interact with it via the right-click context menu. Additionally, for groups without a description, the description of the previously removed group could incorrectly appear due to this bug. This was caused by the UI not refreshing immediately after the group was deleted. (before_changes.mp4)
before_changes.mp4
Solution:
Added groupTree.refresh(); after group removal to ensure the interface updates correctly, preventing ghost groups from lingering in the UI and avoiding incorrect description display for newly created groups. (after_changes.mp4)
after_changes.mp4
Mandatory checks
CHANGELOG.mddescribed in a way that is understandable for the average user (if change is visible to the user)