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

Implement better scaling of main table showing entries #7181

Closed
wants to merge 37 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
a1a5ccc
Implement better scaling of main table showing entries
koppor Dec 12, 2020
e91bb8d
Fix number
koppor Dec 14, 2020
8f522df
Remove resizeColumnsToFit
koppor Dec 14, 2020
1acd12e
Fix checkstyle
koppor Dec 14, 2020
c8caa13
Merge branch 'master' into fix-967
koppor Dec 17, 2020
f5e2d9f
Add smart resize if content fits into table width
koppor Dec 17, 2020
bcea66c
Address some comments
koppor Dec 18, 2020
6da49b2
WIP
koppor Dec 21, 2020
dfac705
Speedup processResources
koppor Dec 21, 2020
057ae9f
Merge branch 'speedup-process-resources' into fix-967
koppor Dec 21, 2020
8834fb6
Try path without ./ at the beginning
koppor Dec 21, 2020
5b6f0d8
Output java error on console, too
koppor Dec 21, 2020
fd80c30
Use LOGGER instead of System.err
koppor Dec 21, 2020
c64b5e3
Merge branch 'fix-output-with-wrong-jdk-version' into fix-967
koppor Dec 21, 2020
9f56fd5
Merge branch 'speedup-process-resources' into fix-967
koppor Dec 21, 2020
d15e90f
Fix scroll bar still present
koppor Dec 21, 2020
7de21ad
MinWidth to 80
koppor Dec 21, 2020
8c7cd7a
Merge branch 'master' into fix-967
koppor Dec 21, 2020
f7186f3
Merge branch 'fix-967' of github.com:JabRef/jabref into fix-967
koppor Dec 21, 2020
80557f2
WIP (does not work)
koppor Dec 22, 2020
043a0cc
Merge remote-tracking branch 'origin/master' into fix-967
koppor Dec 29, 2020
c3b02e3
Fix ordering in CHANGELOG.md
koppor Dec 29, 2020
fd118a6
WIP
koppor Dec 29, 2020
f0788f6
WIP - "User changed column size in a way that window can contain all …
koppor Dec 29, 2020
a1633a1
Add documentation on implementation idea
koppor Jan 2, 2021
73d5448
Refine description (and change TableColumnBase to TableColumn)
koppor Jan 3, 2021
f75831d
Cases I0 to I3
koppor Jan 3, 2021
c417d7d
First proposal of case names
koppor Jan 3, 2021
496e505
WIP: Reimplement logic according to comments
koppor Jan 6, 2021
466731f
Merge remote-tracking branch 'origin/master' into fix-967
koppor Jan 7, 2021
caf1e23
WIP
koppor Jan 7, 2021
63af8fd
Merge remote-tracking branch 'origin/master' into fix-967
koppor Jan 10, 2021
274e109
Resizing of colomms works
koppor Jan 10, 2021
910c592
Use EPSILON_MARGIN also for "content has fit into table before"
koppor Jan 10, 2021
513c68c
Merge branch 'master' into fix-967
koppor Jan 21, 2021
286e7f9
Merge remote-tracking branch 'upstream/master' into fix-967
calixtus Feb 6, 2021
caf7232
Merge branch 'main' into fix-967
koppor Oct 14, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
- We changed connect timeouts for server requests to 30 seconds in general and 5 seconds for GROBID server (special) and improved user notifications on connection issues. [7026](https://github.com/JabRef/jabref/pull/7026)
- We changed the order of the library tab context menu items. [#7171](https://github.com/JabRef/jabref/issues/7171)
- We changed the way linked files are opened on Linux to use the native openFile method, compatible with confined packages. [7037](https://github.com/JabRef/jabref/pull/7037)
- We refined the entry preview to show the full names of authors and editors, to list the editor only if no author is present, have the year ealier. [#7083](https://github.com/JabRef/jabref/issues/7083)
- We refined the entry preview to show the full names of authors and editors, to list the editor only if no author is present, have the year earlier. [#7083](https://github.com/JabRef/jabref/issues/7083)
- We changed the resize behavior of table columns to change the width of the current column only. [#967](https://github.com/JabRef/jabref/issues/967)

### Fixed

Expand Down
6 changes: 2 additions & 4 deletions src/main/java/org/jabref/gui/maintable/MainTable.java
Original file line number Diff line number Diff line change
Expand Up @@ -137,16 +137,14 @@ public MainTable(MainTableDataModel model,
}
}
*/
mainTablePreferences.getColumnPreferences().getColumnSortOrder().forEach(columnModel ->
mainTablePreferences.getColumnPreferences().getColumnSortOrder().forEach(columnModel ->
this.getColumns().stream()
.map(column -> (MainTableColumn<?>) column)
.filter(column -> column.getModel().equals(columnModel))
.findFirst()
.ifPresent(column -> this.getSortOrder().add(column)));

if (mainTablePreferences.getResizeColumnsToFit()) {
this.setColumnResizePolicy(new SmartConstrainedResizePolicy());
}
this.setColumnResizePolicy(new SmartConstrainedResizePolicy());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the if check should still be there, since otherwise the checkbox in the preferences doesn't do anything, right?

Copy link
Member Author

@koppor koppor Dec 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We removed the preference option completely - since this was a hidden feature. That should have been a visible toggle button and not hidden in the preferences.

Moreover #967 said that there should be a single behavior without the need of a configuration flat. We discussed it in length back then - and the implementors think, this is still a good thing.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We added a guard to really cover both cases

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So you don't want to give users the option for the infinite-column behavior that you were supporting above? I don't see any harm in still having the checkbox. But since I don't want to argue again for keeping code while you want to remove it, removing is also fine with me ;-)


this.getSelectionModel().setSelectionMode(SelectionMode.MULTIPLE);

Expand Down
14 changes: 12 additions & 2 deletions src/main/java/org/jabref/gui/maintable/MainTableColumnFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import org.jabref.model.entry.field.Field;
import org.jabref.model.entry.field.FieldFactory;
import org.jabref.model.entry.field.SpecialField;
import org.jabref.model.entry.field.StandardField;
import org.jabref.model.groups.AbstractGroup;
import org.jabref.model.util.OptionalUtil;
import org.jabref.preferences.PreferencesService;
Expand Down Expand Up @@ -109,7 +110,16 @@ public MainTableColumnFactory(BibDatabaseContext database,
default:
case NORMALFIELD:
if (!column.getQualifier().isBlank()) {
columns.add(createFieldColumn(column));
TableColumn<BibEntryTableViewModel, ?> fieldColumn = createFieldColumn(column);
columns.add(fieldColumn);
if (column.getQualifier().equalsIgnoreCase(StandardField.YEAR.getName())) {
// We adjust the min width and the current width, but not the max width to allow the user to enlarge this field
// 75 is chosen, because of the optimal width of a four digit field
koppor marked this conversation as resolved.
Show resolved Hide resolved
fieldColumn.setMinWidth(60);
fieldColumn.setPrefWidth(60);
} else {
fieldColumn.setMinWidth(ColumnPreferences.DEFAULT_COLUMN_WIDTH);
koppor marked this conversation as resolved.
Show resolved Hide resolved
}
}
break;
}
Expand All @@ -125,7 +135,7 @@ public static void setExactWidth(TableColumn<?, ?> column, double width) {
}

/**
* Creates a column with a continous number
* Creates a column with a continuous number
*/
private TableColumn<BibEntryTableViewModel, String> createIndexColumn(MainTableColumnModel columnModel) {
TableColumn<BibEntryTableViewModel, String> column = new MainTableColumn<>(columnModel);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,9 @@ public Type getType() {
return typeProperty.getValue();
}

/**
* Returns the field name of the column (e.g., year)
*/
public String getQualifier() {
return qualifierProperty.getValue();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import java.util.List;

import javafx.scene.control.ResizeFeaturesBase;
import javafx.scene.control.TableColumn;
import javafx.scene.control.TableColumnBase;
import javafx.scene.control.TableView;
import javafx.util.Callback;
Expand All @@ -25,80 +26,36 @@ public class SmartConstrainedResizePolicy implements Callback<TableView.ResizeFe
@Override
public Boolean call(TableView.ResizeFeatures prop) {
if (prop.getColumn() == null) {
return initColumnSize(prop.getTable());
// table is initialized
// no need to adjust
return false;
} else {
return constrainedResize(prop);
}
}

private Boolean initColumnSize(TableView<?> table) {
double tableWidth = getContentWidth(table);
List<? extends TableColumnBase<?, ?>> visibleLeafColumns = table.getVisibleLeafColumns();
double totalWidth = visibleLeafColumns.stream().mapToDouble(TableColumnBase::getWidth).sum();

if (Math.abs(totalWidth - tableWidth) > 1) {
double totalPrefWidth = visibleLeafColumns.stream().mapToDouble(TableColumnBase::getPrefWidth).sum();
if (totalPrefWidth > 0) {
for (TableColumnBase col : visibleLeafColumns) {
double share = col.getPrefWidth() / totalPrefWidth;
double newSize = tableWidth * share;

// Just to make sure that we are staying under the total table width (due to rounding errors)
newSize -= 2;

resize(col, newSize - col.getWidth());
}
}
}

return false;
}

private void resize(TableColumnBase column, double delta) {
// We have to use reflection since TableUtil is not visible to us
try {
// TODO: reflective access, should be removed
Class<?> clazz = Class.forName("javafx.scene.control.TableUtil");
Method constrainedResize = clazz.getDeclaredMethod("resize", TableColumnBase.class, double.class);
constrainedResize.setAccessible(true);
constrainedResize.invoke(null, column, delta);
} catch (NoSuchMethodException | IllegalAccessException | InvocationTargetException | ClassNotFoundException e) {
LOGGER.error("Could not invoke resize in TableUtil", e);
}
}

private Boolean constrainedResize(TableView.ResizeFeatures<?> prop) {
TableView<?> table = prop.getTable();
List<? extends TableColumnBase<?, ?>> visibleLeafColumns = table.getVisibleLeafColumns();
return constrainedResize(prop,
false,
getContentWidth(table) - 2,
visibleLeafColumns);
}

private Boolean constrainedResize(TableView.ResizeFeatures prop, Boolean isFirstRun, Double contentWidth, List<? extends TableColumnBase<?, ?>> visibleLeafColumns) {
// We have to use reflection since TableUtil is not visible to us
try {
// TODO: reflective access, should be removed
Class<?> clazz = Class.forName("javafx.scene.control.TableUtil");
Method constrainedResize = clazz.getDeclaredMethod("constrainedResize", ResizeFeaturesBase.class, Boolean.TYPE, Double.TYPE, List.class);
constrainedResize.setAccessible(true);
Object returnValue = constrainedResize.invoke(null, prop, isFirstRun, contentWidth, visibleLeafColumns);
return (Boolean) returnValue;
} catch (NoSuchMethodException | IllegalAccessException | InvocationTargetException | ClassNotFoundException e) {
LOGGER.error("Could not invoke constrainedResize in TableUtil", e);
return false;
Double delta = prop.getDelta();
TableColumn<?, ?> userChosenColumnToResize = prop.getColumn();

double oldWidth = userChosenColumnToResize.getWidth();
double newWidth;
if (delta < 0) {
double minWidth = userChosenColumnToResize.getMinWidth();
LOGGER.trace("MinWidth {}", minWidth);
newWidth = Math.max(minWidth, oldWidth + delta);
} else {
double maxWidth = userChosenColumnToResize.getMaxWidth();
LOGGER.trace("MaxWidth {}", maxWidth);
newWidth = Math.min(maxWidth, oldWidth + delta);
}
}

private Double getContentWidth(TableView<?> table) {
try {
// TODO: reflective access, should be removed
Field privateStringField = TableView.class.getDeclaredField("contentWidth");
privateStringField.setAccessible(true);
return (Double) privateStringField.get(table);
} catch (IllegalAccessException | NoSuchFieldException e) {
return 0d;
LOGGER.trace("Size: {} -> {}", oldWidth, newWidth);
if (oldWidth == newWidth) {
return false;
}
userChosenColumnToResize.setPrefWidth(newWidth);
return true;
koppor marked this conversation as resolved.
Show resolved Hide resolved
}
}