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 closing entry editor with ESC #2957

Merged
merged 1 commit into from
Jul 3, 2017
Merged

Conversation

Siedlerchr
Copy link
Member

@Siedlerchr Siedlerchr commented Jul 2, 2017

Rename Escape to ESC for javafx
Renamed Enum variables as key for the constant was amibiuous

First part of #2949
Internal change only, from the new entry editor.

Edit// You may need to reset the keybinding for CLOSE_ENTRY_EDITOR in the Manage keybinding dialog, as the old stored value from the prefs no longer works for javafx.

  • Change in CHANGELOG.md described
  • Tests created for changes
  • Screenshots added (for bigger UI changes)
  • Manually tested changed features in running JabRef
  • Check documentation status (Issue created for outdated help page at help.jabref.org?)
  • If you changed the localization: Did you run gradle localizationUpdate?

Rename Escape to ESC for javafx
Renamed Enum variables as key for the constant was amibiuous
@Siedlerchr Siedlerchr requested review from koppor and tobiasdiez July 2, 2017 21:49
@Siedlerchr Siedlerchr added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Jul 2, 2017
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.

LGTM. Can we just remove the constant thing?

* This method returns the enum constant value
* @return
*/
public String getConstant() {
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need this "constant/key" as a unique identifier if we already have an enum?

Copy link
Member Author

Choose a reason for hiding this comment

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

This constant is just the name of the enum value. It is used in several cases to compare bindings and to map between the settings and the enum values dynamically. It is used in KeyBindingsRepositoriy
I just renamed it, because "key" is ambigous in this content and always confused me.

@Siedlerchr Siedlerchr merged commit 6897b10 into master Jul 3, 2017
@Siedlerchr Siedlerchr deleted the keybindingentryeditor branch July 3, 2017 13:39
Siedlerchr added a commit that referenced this pull request Jul 4, 2017
* upstream/master:
  Fix closing entry editor with ESC (#2957)
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