-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix column sort order gets overwritten #7573
Conversation
May i eventually push on your branch? I'd prefer immutable preferences objects... |
Fix itself looks right. But there are some code style things I would like to change. |
@calixtus Sure, but I think the root cause was the immutable ColumnPreferences, the sort order was always overwritten even if it did not change. Note that it is also not possible to check for empty, because when you clear the sort order the list is empty and you could not reset the sort order. |
I think it's fine. I just made the ColumnPreferences immutable again, but kept the distinction in the methods between updates of the SortOrder and the Columns, so nothing should be overwritten. It just pulls the old opposite unchanged value directly from the preferences. The logic should be exactly the same, but without altering the existing ColumnPreferences, but creating new ones with the kept value from the preferences. Can you please check it to be sure it works for you? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Codewise LGTM!
Ah I see, will test tomorrow |
* upstream/master: Bump classgraph from 4.8.102 to 4.8.104 (#7587) Bump checkstyle from 8.41 to 8.41.1 (#7586) Bump org.beryx.jlink from 2.23.3 to 2.23.5 (#7588) Revert "Re-add LibericaJDK" Re-add LibericaJDK Update gradle from 6.8 to 6.8.3 (#7583) Fix for issue 7416: font size of the preferences dialog does not update with the rest of the GUI. (#7509) Fix school/instituation is printed twice (#7574)
…TableColSort * 'mainTableColSort' of github.com:JabRef/jabref: Clean up Clean up Clean up
Works, but I found another bug in the SaveOrderConfig.SortCriterion uses Boolean.parseBoolean() and it was passed the enum values "ASCENDING" or "DESCENDING" which obviously will always return false when you have that input for Boolean.parse |
Co-authored-by: Oliver Kopp <olly98@users.sourceforge.net>
DevCall: Merge as hotfix; later: nice preferences (String should be enum) |
* upstream/main: (25 commits) Fix NumberFormatException in BracketedPattern (#7600) Update MAINTAINERS (#7601) Fix CSL update (#7592) Add unit tests for org.jabref.gui classes (#7579) Squashed 'buildres/csl/csl-styles/' changes from 30fb68e..e1acabe Bump tika-core from 1.25 to 1.26 (#7589) Revert "Skip Mac OS build if secret not present (#7591)" (#7594) Optimize saving (#7568) Skip Mac OS build if secret not present (#7591) Rename master to main in dev help Update tests-fetchers.yml Update refresh-journal-lists.yml Update deployment.yml Rename master to main in coding readme Rename master to main in citation style update Rename master to main in gitversion config Fix column sort order gets overwritten (#7573) Add some hints on test tooling (#7585) Add "Update Gradle Wrapper Action" (#7584) Bump classgraph from 4.8.102 to 4.8.104 (#7587) ...
Fixes #7524
The problem was that mainTable.getSortOrder is only filled when there is a change. However, updateColumnPreferences was also called when just the width changed and then getSortOrder was empty and was always overwritten.
CHANGELOG.md
described in a way that is understandable for the average user (if applicable)