Skip to content

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

Merged
tobiasdiez merged 7 commits into
JabRef:masterfrom
cs304-spring2021:fix-for-issue-6994
Jan 31, 2021
Merged

fix "Alt + keyboard shortcuts do not work"#7379
tobiasdiez merged 7 commits into
JabRef:masterfrom
cs304-spring2021:fix-for-issue-6994

Conversation

@Fu188

@Fu188 Fu188 commented Jan 24, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

@Siedlerchr Siedlerchr left a comment

Copy link
Copy Markdown
Member

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

@calixtus calixtus left a comment

Copy link
Copy Markdown
Member

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 reviewed Jan 25, 2021
View reviewed changes
Comment thread src/main/java/org/jabref/gui/JabRefFrame.java Outdated
calixtus
calixtus requested changes Jan 30, 2021
View reviewed changes

@calixtus calixtus left a comment •
edited
Loading

Copy link
Copy Markdown
Member

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

Fu188 commented Jan 31, 2021

Copy link
Copy Markdown
Contributor Author

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

calixtus commented Jan 31, 2021

Copy link
Copy Markdown
Member

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

@Fu188

Fu188 commented Jan 31, 2021

Copy link
Copy Markdown
Contributor Author

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

@calixtus calixtus left a comment

Copy link
Copy Markdown
Member

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

@tobiasdiez tobiasdiez left a comment

Copy link
Copy Markdown
Member

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

@tobiasdiez tobiasdiez tobiasdiez approved these changes

@calixtus calixtus calixtus 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