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.

Sign up for GitHub

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

Jump to bottom

fix "Alt + keyboard shortcuts do not work" #7379

Merged
merged 7 commits into from
Jan 31, 2021
Merged

fix "Alt + keyboard shortcuts do not work" #7379

merged 7 commits into from
Jan 31, 2021
+13 −16

Conversation

Fu188
Copy link
Contributor

@Fu188 Fu188 commented Jan 24, 2021 •
edited
Loading

User can use CheckMenuItem shortcut only when they click the tab. That is because the event will not be created until user click the menu in original version. Such that, I create the event when we intial the GUI. In this case, the issue is fixed. Fixes #6994

  • Change in CHANGELOG.md described (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 documentation: Is the information available and up to date? If not created an issue at https://github.com/JabRef/user-documentation/issues or, even better, submitted a pull request to the documentation repository.

@Fu188 Fu188 closed this Jan 24, 2021
@Fu188 Fu188 reopened this Jan 24, 2021
Siedlerchr
Siedlerchr previously approved these changes Jan 24, 2021
View reviewed changes
Copy link
Member

@Siedlerchr Siedlerchr left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution! Looks good to me.

@Siedlerchr Siedlerchr added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Jan 24, 2021
calixtus
calixtus approved these changes Jan 25, 2021
View reviewed changes
Copy link
Member

@calixtus calixtus left a comment

Choose a reason for hiding this comment

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

The reason for the just-in-time creation of the menu is not clear to me, it was introduced in #4707 by @davidemdot , but I have found nothing in the discussion about it. Think it's okay to remove it.

calixtus
calixtus requested changes Jan 30, 2021
View reviewed changes
Copy link
Member

@calixtus calixtus left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Hi @Fu188 , I tried to fix the remaining issue I mentioned quickly by myself, but it seems, that you did not grant write access to your PR branch. So please fix the remaining issue, then we can merge.
Also Changelog.md needs some conflict resolution.

@Fu188
Copy link
Contributor Author

Fu188 commented Jan 31, 2021

Hi @Fu188 , I tried to fix the remaining issue I mentioned quickly by myself, but it seems, that you did not grant write access to your PR branch. So please fix the remaining issue, then we can merge.
Also Changelog.md needs some conflict resolution.

I have fix the remaining issue. Thanks for your merge!

@Fu188 Fu188 requested a review from calixtus January 31, 2021 02:49
@calixtus
Copy link
Member

Now you have introduced a lot of indentation changes in JabRefFrame (https://github.com/JabRef/jabref/pull/7379/files). Please undo them.

@Fu188
Copy link
Contributor Author

Fu188 commented Jan 31, 2021

Now you have introduced a lot of indentation changes in JabRefFrame (https://github.com/JabRef/jabref/pull/7379/files). Please undo them.

I have undo them and it's ok now.

calixtus
calixtus approved these changes Jan 31, 2021
View reviewed changes
Copy link
Member

@calixtus calixtus left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks!

tobiasdiez
tobiasdiez approved these changes Jan 31, 2021
View reviewed changes
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! Thanks

@tobiasdiez tobiasdiez merged commit dec8ff1 into JabRef:master Jan 31, 2021
@Fu188 Fu188 deleted the fix-for-issue-6994 branch February 1, 2021 02:32
@Fu188 Fu188 restored the fix-for-issue-6994 branch February 1, 2021 02:32
Siedlerchr added a commit that referenced this pull request Feb 1, 2021
@Siedlerchr
Merge remote-tracking branch 'upstream/master' into fixlayoutauthors
a44f589 
* upstream/master:
  Bump archunit-junit5-api from 0.15.0 to 0.16.0 (#7407)
  Bump classgraph from 4.8.98 to 4.8.102 (#7401)
  Bump archunit-junit5-engine from 0.15.0 to 0.16.0 (#7402)
  Bump mariadb-java-client from 2.7.1 to 2.7.2 (#7406)
  Bump org.beryx.jlink from 2.23.2 to 2.23.3 (#7400)
  Bump checkstyle from 8.39 to 8.40 (#7404)
  Ignore codecov status for automerge
  Fixes issue of Changing font size makes font size field too small (#7398)
  fix "Alt + keyboard shortcuts do not work" (#7379)
  Fixed invisible file path in the dark theme (#7396)
  Fix File Filter and some layout issues (#7385)
  Feature/implement complex queries (#7350)
  Change format for study definition to yaml (#7126)
  Fix handling of URL in file field (#7347)
  Fix expansion of bracketed expressions in RegExpBasedFileFinder (#7338)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@calixtus calixtus calixtus approved these changes

@tobiasdiez tobiasdiez tobiasdiez approved these changes

@Siedlerchr Siedlerchr Siedlerchr left review comments

Assignees
No one assigned
Labels
status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Menu View: Alt+ keyboard shortcuts do not work.
4 participants
@Fu188 @calixtus @Siedlerchr @tobiasdiez