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

Reenable closing of entry preview by pressing Esc #3883

Merged
merged 1 commit into from
Mar 25, 2018

Conversation

tobiasdiez
Copy link
Member

Problem was that we had a few key-bindings that were bound to Esc, but only the first binding was returned (e.g. user pressed Esc, was recognized as DialogClose but preview only reacted on EntryPreviewClose).


  • Change in CHANGELOG.md described
  • Tests created for changes
  • Manually tested changed features in running JabRef
  • Screenshots added in PR description (for bigger UI changes)
  • Ensured that the git commit message is a good one
  • Check documentation status (Issue created for outdated help page at help.jabref.org?)

@tobiasdiez tobiasdiez added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Mar 23, 2018
@stefan-kolb stefan-kolb mentioned this pull request Mar 23, 2018
35 tasks
@Siedlerchr
Copy link
Member

Still leaves the problem with clear search and close both using esc

@tobiasdiez
Copy link
Member Author

@Siedlerchr
Copy link
Member

But I think it will now have the effect that if you clear the search the entry editor will be closed as well.

@tobiasdiez
Copy link
Member Author

No, it does not (just tried it out to make sure). event.consume() prevents the event from from bubbling up the tree and thus the control with focus gets closed and the other ones stay open.

@@ -143,7 +143,7 @@ public void actionPerformed(ActionEvent e) {
updateOpenCurrentResultsTooltip(globalSearch.isSelected());
focus();
event.consume();
} else if (keyBinding.get().equals(KeyBinding.CLEAR_SEARCH)) {
} else if (keyBinding.get().equals(KeyBinding.CLOSE)) {
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer a switch case here like in the other dialogs to have it consistent

Copy link
Member Author

Choose a reason for hiding this comment

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

Me too, but I'm too lazy right now to change it 😄

@Siedlerchr
Copy link
Member

If you fix the checkstyle issuie it's a go for me

@tobiasdiez tobiasdiez merged commit 0bf39f5 into maintable-beta Mar 25, 2018
@tobiasdiez tobiasdiez deleted the fixClosePreview branch March 25, 2018 17:19
Siedlerchr added a commit that referenced this pull request Mar 26, 2018
…esDlg

* upstream/maintable-beta:
  Reenable closing of entry preview by pressing Esc (#3883)

# Conflicts:
#	src/main/java/org/jabref/gui/openoffice/StyleSelectDialog.java
Siedlerchr added a commit that referenced this pull request Mar 30, 2018
…drop

* upstream/maintable-beta: (48 commits)
  Clean unused imports
  Fix missing icons and wrong package for custom icons
  Consistent FX color scheme for JabRef (#3839)
  javafx replacement for file dialog (#3005)
  Reenable closing of entry preview by pressing Esc (#3883)
  Load all field editors using ViewLoader
  Use JabRef icons in FXML
  Move icon stuff to new package gui.icon
  Load EntryEditor using new ViewLoader
  Improve tooltip tests
  Fix import thread problem
  Don't use null as parameter in DialogService
  Make it easier to create FXML dialogs (#3880)
  update slf4j from 1.8.0-beta1 -> 1.8.0-beta2
  New translations JabRef_en.properties (Tagalog)
  New translations JabRef_en.properties (Italian)
  New translations Menu_en.properties (Italian)
  New translations JabRef_en.properties (Indonesian)
  New translations JabRef_en.properties (Greek)
  New translations Menu_en.properties (Greek)
  ...

# Conflicts:
#	src/main/java/org/jabref/gui/actions/CleanupAction.java
#	src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java
#	src/main/java/org/jabref/gui/groups/GroupTreeController.java
#	src/main/java/org/jabref/gui/maintable/MainTable.java
#	src/main/java/org/jabref/logic/cleanup/MoveFilesCleanup.java
#	src/main/java/org/jabref/logic/cleanup/RenamePdfCleanup.java
#	src/test/java/org/jabref/logic/cleanup/MoveFilesCleanupTest.java
#	src/test/java/org/jabref/logic/cleanup/RenamePdfCleanupTest.java
Siedlerchr added a commit that referenced this pull request Apr 1, 2018
…gsjavafx

* upstream/maintable-beta: (188 commits)
  Fix checkstyle
  Exchange Ignore by Disabled (#3912)
  Remove all @author comments and empty method/class comments
  Clean unused imports
  Fix missing icons and wrong package for custom icons
  Consistent FX color scheme for JabRef (#3839)
  javafx replacement for file dialog (#3005)
  Reenable closing of entry preview by pressing Esc (#3883)
  Load all field editors using ViewLoader
  Use JabRef icons in FXML
  Move icon stuff to new package gui.icon
  Load EntryEditor using new ViewLoader
  Improve tooltip tests
  Fix import thread problem
  Don't use null as parameter in DialogService
  Make it easier to create FXML dialogs (#3880)
  update slf4j from 1.8.0-beta1 -> 1.8.0-beta2
  New translations JabRef_en.properties (Tagalog)
  New translations JabRef_en.properties (Italian)
  New translations Menu_en.properties (Italian)
  ...

# Conflicts:
#	src/main/java/org/jabref/gui/JabRefFrame.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants