-
-
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 for issue 6601: Add right click menu for main table header #8762
Conversation
Added Right Click Menu & Need help for Remove Column
TableTab tableTab = new TableTab(); | ||
TableTabViewModel tableTabViewModel = new TableTabViewModel(dialogService, preferencesService); | ||
|
||
// tableTabViewModel.removeColumn(tableTab.getList().getSelectionModel().getSelectedItem()); |
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.
We know how to access the column name but not sure if we can somehow use it in removeColumn( ).
Or is there a different way to remove a particular column from the preferences/entry table and maintable if we know the column name.
Any suggestions would be highly appreciated!
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.
I would operate on the ColumnPreferences here as follows.
In MainTable you have the columns in this.columns and can map them to the MainTableColumnModel. And then you can create a new TablecolumnPreferences
this.getColumns().stream()
.map(column -> (MainTableColumn<?>) column)
e.g. (see PersistenceVisualStateTable)
preferences.storeMainTableColumnPreferences(new ColumnPreferences(
mainTable.getColumns().stream()
.filter(col -> col instanceof MainTableColumn<?>)
.map(column -> ((MainTableColumn<?>) column).getModel())
.collect(Collectors.toList()),
preferences.getColumnPreferences().getColumnSortOrder()));
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.
Thank you! We had a bit of confusion understanding how to create a new TablecolumnPreferences. We found another way to remove column though. Let us know if it works, thanks!
6f4ffba
to
6163d4c
Compare
static MainTable mT = null; | ||
|
||
public void show(MainTable mainTable, LibraryTab libraryTab, DialogService dialogService) { | ||
System.out.println("show()"); |
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.
Please remove these System.out statements...
import org.jabref.logic.l10n.Localization; | ||
|
||
public class MainTableHeaderRightClickMenu extends ContextMenu { | ||
static MainTable mT = null; |
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.
Why is this static? You could simply declare it as normal member variable.
I would pass the MainTable argument in the constructor and assign it there.
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.
Please do not abbreviate variable names.
@@ -147,4 +146,8 @@ public void sortColumnDown() { | |||
public void addColumn() { | |||
viewModel.insertColumnInList(); | |||
} | |||
|
|||
public TableView<MainTableColumnModel> getList() { |
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.
I would give this a meaningful name
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.
Please do not use a getter for the TableView, instead inject the columnslist of the table into your new class RightClickMenu and work on that. We need to untangle all the callbacks, otherwise the classes are untestable.
public ListView<PreferencesTab> getPreferenceTabList() { | ||
return preferenceTabList; | ||
} |
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.
Do not touch the PreferencesDialog. Changes must happen in the Preferences, not in the Dialog
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.
Or perhaps make the method more generic, so a new method could open any specific preferences tab.
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.
Pushed the wrong button
.map(column -> (MainTableColumn<?>) column) | ||
.filter(column -> column.getModel().equals(columnModel)) | ||
.findFirst() | ||
.ifPresent(column -> this.getSortOrder().add(column))); | ||
.map(column -> (MainTableColumn<?>) column) | ||
.filter(column -> column.getModel().equals(columnModel)) | ||
.findFirst() | ||
.ifPresent(column -> this.getSortOrder().add(column))); |
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.
Please undo all your whitespace changes and use our checkstyle config for your jabref workspace.
/** | ||
* Create any single column | ||
*/ | ||
public TableColumn<BibEntryTableViewModel, ?> createColumn(MainTableColumnModel columnModel) { | ||
switch (columnModel.getType()) { | ||
case INDEX: | ||
return createIndexColumn(columnModel); | ||
case GROUPS: | ||
return createGroupColumn(columnModel); | ||
case FILES: | ||
return createFilesColumn(columnModel); | ||
case LINKED_IDENTIFIER: | ||
return createIdentifierColumn(columnModel); | ||
case LIBRARY_NAME: | ||
return createLibraryColumn(columnModel); | ||
case EXTRAFILE: | ||
if (columnModel.getQualifier().isBlank()) { | ||
return createExtraFileColumn(columnModel); | ||
} | ||
break; | ||
case SPECIALFIELD: | ||
if (!columnModel.getQualifier().isBlank()) { | ||
Field field = FieldFactory.parseField(columnModel.getQualifier()); | ||
if (field instanceof SpecialField) { | ||
return createSpecialFieldColumn(columnModel); | ||
} else { | ||
LOGGER.warn("Special field type '{}' is unknown. Using normal column type.", columnModel.getQualifier()); | ||
return createFieldColumn(columnModel); | ||
} | ||
} | ||
break; | ||
default: | ||
case NORMALFIELD: | ||
if (!columnModel.getQualifier().isBlank()) { | ||
return createFieldColumn(columnModel); | ||
} | ||
break; | ||
} | ||
return null; | ||
} |
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.
If you introduce this method, you need to adjust the constructor of the MainTableColumnFactory to reduce code duplication
// Click on the tableColumns | ||
if (!(clickEvent.getTarget() instanceof StackPane)) { | ||
System.out.println("Click on the tableColumns"); | ||
// Create radioMenuItemList from tableColumnList |
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.
Please avoid code-describing comments, instead try to write self-explaining code and put your ratilonale into a comment.
Any update here? |
To ease organizational workflows I have linked the pull-request to the issue with syntax as described in https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue
|
any update here? |
Closing this issue due to inactivity 💤 |
Fixes #6601
We are able to remove the column from the maintable using a right click menu. And it also updates the Entry Table in Preferences.
Inspired from #7729
Screenshots:
2a
2b
CHANGELOG.md
described in a way that is understandable for the average user (if applicable)