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

F4 opens selected file in current JTable context #2626

Merged
merged 2 commits into from
Mar 9, 2017
Merged

Conversation

stefan-kolb
Copy link
Member

Resolves #2355

@stefan-kolb stefan-kolb requested a review from koppor March 8, 2017 14:19
@stefan-kolb stefan-kolb added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Mar 8, 2017
@lenhard
Copy link
Member

lenhard commented Mar 9, 2017

I just tested this locally and it works if I have selected another file in the entry editor.

However, the following seems a little counter-intuitive to me:

  1. I have the second file of an entry selected in the entry editor, everything works as expected.
  2. Next, I click the same entry in the main table. Both the entry in the main table and the second file in the entry editor are still highlighted.
  3. When I push F4 now, the first attached file is opened and not the one that is still highlighted in the entry editor. This seems counter-intuitive.

This is just a minor thing, and if in doubt the PR can be merged as is (code-wise it is fine). However, it would be nice if the feature were more consistent.

@stefan-kolb
Copy link
Member Author

I understand this, but this is a focus problem then. I'm not sure how to fix this. Initially, I also didn't want to implement this feature here, but Olly wanted JabRef to behave like this. For me F4 opening the first link regardless of the current focus was ok.

@lenhard
Copy link
Member

lenhard commented Mar 9, 2017

@stefan-kolb Sure, I understand. This is no blocker and from my point of view, the PR can also be merged as is. I just guess that at some point a user will stumble upon this and report it as an issue.

@stefan-kolb stefan-kolb merged commit fd36288 into master Mar 9, 2017
@stefan-kolb stefan-kolb deleted the f4-key branch March 9, 2017 12:18
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