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

Fix for issue: right click menu 6601 #9271

Merged
merged 10 commits into from
Dec 5, 2022

Conversation

ethantr
Copy link
Contributor

@ethantr ethantr commented Oct 21, 2022

Fixes #6601 [WIP]

The user is able to right-click on the header of a maintable, to toggle the each column(field) visibility. This is also updated in the Entry Table in preferences. This is intended to improve the efficiency of using the program/UI as it will give users one less step from toggling the visibility of fields, instead of clicking through the preferences menus.

Inspired from #7729, #8762

  • Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for UI changes)
  • Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
  • Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

Demonstration (Screenshots):

  1. When the user right clicks on the Main Table header, a context menu appears with the most commonly used fields based on the default preferences), and the other currently used fields. A tick next to the item shows whether the column is visible in the table. The seperator in the table distinguishes between commonly and currently used fields.
    screenshotA

  2. When an item is clicked, its visibility gets toggled. This process either removes the column from the MainTable, or adds it back into view, which is reflected in the preferences. In this scenario, I have clicked the Author/Editor and the Chapter fields, both are now removed from the MainTable, and their ticks show they are not visible.
    screenShotB

  3. After clicking on "Author/Editor" it is placed back into the table, and its tick is present, showing that is indeed back in the table with its original contents.
    screenShotC

TO-DO:

  • Currently considering adding the Show Preferences button, but thought I would push this draft pull request to get some feedback to make sure this is going in the right direction.
  • Aware that checking the column is present in the current maintable is inefficient, beleive a boolean property within the MainTableColumn could be added to make it constant, and only have to check once.

… non functional concept of how the menu will operate.
…t of the current columns in header, indicating if it is visible or invisible. Clicking on the item will toggle its visibility.

Signed-off-by: ethantr <ethantrossi@gmail.com>
…t of the current columns in header, indicating if it is visible or invisible. Clicking on the item will toggle its visibility.

Signed-off-by: ethantr <ethantrossi@gmail.com>
…removes the columns and adds columns to the table instead of relying on changing visibility

Signed-off-by: ethantr <ethantrossi@gmail.com>
…create individual columns, reduced code duplication. Now removes and adds from the MainTable when item is clicked, instead of removing visibility.

Signed-off-by: ethantr <ethantrossi@gmail.com>
…er of the current fields in MainTable. Messy code reduced.

Signed-off-by: ethantr <ethantrossi@gmail.com>
Signed-off-by: ethantr <ethantrossi@gmail.com>
@ethantr ethantr marked this pull request as ready for review October 21, 2022 03:47
@ThiloteE
Copy link
Member

ThiloteE commented Oct 28, 2022

I have had a look at this pr and tried it in my local workspace with gh pr checkout 9271 and ./gradlew run on my Linux Mint Cinnamon (kernel 5.4) machine.

Contrary to screenshots, explanation and expectations, i was not able to simply rightclick to trigger the menu. I needed to hold right click. During this hold, neither using left- nor right-click would allow me to remove columns from the main table or alternatively add a column to the main table.

This pr needs changes before it can be merged.

@ThiloteE ThiloteE added the status: changes required Pull requests that are not yet complete label Oct 28, 2022
@ethantr
Copy link
Contributor Author

ethantr commented Nov 10, 2022

@ThiloteE I've made some changes and tested it on a Linux machine, should work as intended now.

@ThiloteE
Copy link
Member

Awesome! I can confirm it works now! Thank you!

@ThiloteE ThiloteE added status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers and removed status: changes required Pull requests that are not yet complete labels Nov 10, 2022
Copy link
Member

@tobiasdiez tobiasdiez left a comment

Choose a reason for hiding this comment

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

This is a really nice addition, thanks a lot for your work!

I've only a few smallish suggestions about the order of the menu entries. Otherwise it looks good to me.

private void addColumn(MainTableColumn tableColumn) {
// Do not add duplicate if table column is already within the table.
if (!isInMainTable(tableColumn)) {
mainTable.getColumns().add(tableColumn);
Copy link
Member

Choose a reason for hiding this comment

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

What happens if the user first removes a column, and then immediately readds it (e.g. he realizes removing it was a mistake). In this case, the column order should be the same as it was before. If I read the code correctly, then currently the column would be appended at the end instead of at its original position.

Copy link
Contributor Author

@ethantr ethantr Nov 26, 2022

Choose a reason for hiding this comment

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

I've made some more changes here with my current commit. When the user goes to click to remove the column, it obtains the current index in the MainTable and stores that number inside the Menu Item. If he wants to immediately place it back, it will be placed back in that exact position using that index.
If the items addjacent to it are removed, it will still be placed in the position of the given index - not sure if that will be confusing to the end user or not, whether it should be done in this way, or another. happy to hear your thoughts.

…followed by remaining common columns)

MenuItems construction now contains visibility and index.
When item is removed, it is now replaced in the previous position it was before. Otherwise, it is appended to the end of the table.

Signed-off-by: ethantr <ethantrossi@gmail.com>
@koppor
Copy link
Member

koppor commented Dec 5, 2022

DevCall: We merge as is.

However, eventually, there should be a follow-up PR:

We take Windows Explorer as reference (see #6601 (comment))

We tried it:

grafik

Only columns enabled in the preferences are shown:

grafik

How to enable addition of new columns (as shown at the screenshot at #6601 (comment))?

We think "More..." ("Mehr..." in German) is missing:

grafik

After a click, a new popup opens

grafik

Would be nice if that was added here, too. Would that be possible?

This list should be filled with all fields known to JabRef. In case a user wants to add more fields, they must go to the preferences and add the custom field there.

@koppor koppor merged commit a1ef245 into JabRef:main Dec 5, 2022
Siedlerchr added a commit to MaryJml/jabref that referenced this pull request Dec 13, 2022
* upstream/main: (37 commits)
  Update database context in state manager after loading (JabRef#9450)
  Bump classgraph from 4.8.151 to 4.8.152 (JabRef#9448)
  Bump appleboy/ssh-action from 0.1.5 to 0.1.6 (JabRef#9443)
  Bump Pendect/action-rsyncer from 1.1.0 to 2.0.0 (JabRef#9444)
  Bump jackson-dataformat-yaml from 2.14.0 to 2.14.1 (JabRef#9445)
  Bump unirest-java from 3.14.0 to 3.14.1 (JabRef#9447)
  Bump postgresql from 42.5.0 to 42.5.1 (JabRef#9446)
  New Crowdin updates (JabRef#9435)
  Return absolute path in case an absolute one is given (JabRef#9433)
  New Crowdin updates (JabRef#9434)
  Fix for issue: right click menu 6601 (JabRef#9271)
  Fix modernizer and refactor protected terms (JabRef#9427)
  Observable Preferences (OpenOffice) (JabRef#9422)
  Allow users to review backup changes before restoring them or merge them selectively (JabRef#9311)
  Bump slf4j-api from 2.0.4 to 2.0.5 (JabRef#9428)
  Bump archunit-junit5-api from 1.0.0 to 1.0.1 (JabRef#9429)
  Bump jackson-datatype-jsr310 from 2.14.0 to 2.14.1 (JabRef#9430)
  Bump lucene-highlighter from 9.4.1 to 9.4.2 (JabRef#9431)
  Fix weird checkbox styling (JabRef#9425)
  New translations JabRef_en.properties (Italian) (JabRef#9424)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintable status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add right click menu for main table header
5 participants