Skip to content
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

Column sort order not properly stored #7524

Closed
Siedlerchr opened this issue Mar 13, 2021 · 8 comments · Fixed by #7573
Closed

Column sort order not properly stored #7524

Siedlerchr opened this issue Mar 13, 2021 · 8 comments · Fixed by #7573
Labels
bug Confirmed bugs or reports that are very likely to be bugs preferences

Comments

@Siedlerchr
Copy link
Member

Siedlerchr commented Mar 13, 2021

There is somehow a bug in storeColumnPreferences

Even when I change the sort oder of one column

From debugging I noticed it's always empty and this is the root issue why the export does not take into account the current table order

putStringList(COLUMN_SORT_ORDER, columnPreferences
.getColumnSortOrder().stream()
.map(MainTableColumnModel::getName)
.collect(Collectors.toList()));

@Siedlerchr Siedlerchr added bug Confirmed bugs or reports that are very likely to be bugs preferences labels Mar 13, 2021
@Siedlerchr Siedlerchr reopened this Mar 13, 2021
@Siedlerchr Siedlerchr changed the title Column sort order not proberly stored Column sort order not properly stored Mar 13, 2021
@Siedlerchr
Copy link
Member Author

Debugged a bit more, it's odd: Sometimes the mainTable.getSortOrder() returns an empty list.
I could reproduce it as follows. Sort by year DESC, e.g. click on the table header for the YEAR column.

private void updateColumnPreferences() {
preferences.storeColumnPreferences(new ColumnPreferences(
mainTable.getColumns().stream()
.map(column -> ((MainTableColumn<?>) column).getModel())
.collect(Collectors.toList()),
mainTable.getSortOrder().stream()

@calixtus I remember that you worked on this. Any idea?

@calixtus
Copy link
Member

I took a quick look into it. I honestly have no idea why. mainTable.getSortOrder refs directly to the javafx-implementation of a TableView.

@Mehdi-Ayadi
Copy link

Hi, i'm a software engineering student and I'm really looking forward to solve this issue if possible. It will be really appreciated to get some guidance or help since i'm new to the project. Thanks!

@Mehdi-Ayadi
Copy link

@Siedlerchr i can propose the idea of adding a change listener to the mainTable.getSortOrder that will basically detect whenever there's a change in the sorting order we want ASC or DESC and in a case of DESC order the event will be handled. If it's a good solution i can manage to solve it directly. Thanks in advance !

@Siedlerchr
Copy link
Member Author

Thanks for your interest, but the underlying problem is a bit more complicated and we already have listeners in place if you look closely in the PersistenceVisualStateTable

https://stackoverflow.com/questions/52255126/javafx-tableview-why-does-the-sortorder-listener-does-not-fire-when-i-sort-desc

The question is rather: Why is the getSortOrder list sometimes empty and sometimes not when a listener is triggered?

@calixtus
Copy link
Member

Hello @Mehdi-Ayadi nice to hear, that you took interest in JabRef.
To get started, I would suggest you take a look at our developer documentation and start by solving some "good first issues" in our issue tracker to become familiar with the code base of JabRef.
Please note that you have to distinguish in JavaFX between to column sort order and the sort type of a column. Both are stored in JavaFX in separate variables. Right now we are using a changelistener (in PersistentVisualState), but somehow the JavaFX framework does not provide any sort order at all in certain cases. A first step towards a solution would be to debug the ChangeListener and to get an understanding, in which situations it does not work and in which it does.

@Mehdi-Ayadi
Copy link

@Siedlerchr @calixtus thank you for your fast replies and for the clarifications ! it's really appreciated. I will take a look a bit more into the details and do my best to come up with a solution.

@Siedlerchr
Copy link
Member Author

Idea for debugging: Split each listener to call a separate method so it's easier to see what listener is when called

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed bugs or reports that are very likely to be bugs preferences
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants