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 issue 9863 - Change Tab selection order #9907

Merged
merged 14 commits into from
Jun 10, 2023

Conversation

YifeiShi99
Copy link
Contributor

@YifeiShi99 YifeiShi99 commented May 16, 2023

Fixes #9863

Compulsory checks

@@ -55,7 +55,8 @@ public enum KeyBinding {
FILE_LIST_EDITOR_MOVE_ENTRY_DOWN("File list editor, move entry down", Localization.lang("File list editor, move entry down"), "ctrl+DOWN", KeyBindingCategory.VIEW),
FILE_LIST_EDITOR_MOVE_ENTRY_UP("File list editor, move entry up", Localization.lang("File list editor, move entry up"), "ctrl+UP", KeyBindingCategory.VIEW),
FIND_UNLINKED_FILES("Search for unlinked local files", Localization.lang("Search for unlinked local files"), "shift+F7", KeyBindingCategory.QUALITY),
FOCUS_ENTRY_TABLE("Focus entry table", Localization.lang("Focus entry table"), "alt+1", KeyBindingCategory.VIEW),
FOCUS_ENTRY_TABLE("Focus entry table", Localization.lang("Focus entry table"), "TAB", KeyBindingCategory.VIEW),
Copy link
Member

Choose a reason for hiding this comment

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

Why did you change this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is changed to match the Tasks of focusing on entry table after pressing tab from the pull request #9863

Copy link
Member

Choose a reason for hiding this comment

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

Is it still possible to navigate away from the entry table? I would see tab as cycling though different ui components: Main menu entries, groups, main table, entry editor, ...

It should be possible to focus these elements using quick shortcut. Alr+1 was such a shortcut.

Copy link
Member

Choose a reason for hiding this comment

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

I think there is a bit of a misunderstanding of the issue:
Tab and Shift Tab are coming from the operating system and mean:
Tab: Forward navigation between controls
Shift + Tab: Backwards navigation between controls

The guy means: If you have the entry editor open, pressing shift + tab should navigate to the group. And if your focus is on the group; Navigation with tab should go forward to the
But I think this might be difficult. It depends on the order of the Nodes in the javafx scencegraph. Focus model etc.
I would rather add two extra shortcuts (like you did), but with separate shortcuts: like alt+1 or alt+2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi Siedlerchr, we can use alt + 1 to focus on the entry table (like before) and alt + 2 to focus on the group list

Copy link
Member

Choose a reason for hiding this comment

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

I think the reporter @DvP17 at #9863 really meant a proper Tab cycling order. I agree, however, that this is very difficult to implement. A workaround using Alt+1 and Alt+2 is OK for me.

Note that the hotkey system was revised at #1525. Updated at #1605.

Keys - new Keys - old Function
F7 alt + f Automatically set file links
F8 ctrl + shift + F7 Cleanup entries
alt + 1 ctrl + shift + E Focus entry table
alt + 2 ctrl + F9 Toggle entry/preview editor
alt + 3 ctrl + shift + G Toggle groups
alt + 4 F5 web search
alt + 0 - Open Openoffice/LibreOffice connection
ctrl + shift + F7 ctrl + f4 Synchronize file links
ctrl + shift R - Technical report
- alt + P Print entry preview
- ctrl + alt + T Hide/show toolbar
- ctrl+P Edit preamble

Maybe, we should think about propr key binding defaults. Alt+Number is used for both toggle and focus in the implementation of 2016.

Copy link
Member

Choose a reason for hiding this comment

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

Note that we have a blog article on a revised hotkey system in 2016: https://blog.jabref.org/2016/08/15/HotkeyRevision/

Copy link
Member

Choose a reason for hiding this comment

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

alt +1 and alt +2 are no longer used

@ThiloteE ThiloteE changed the title implement request from issue #9863 Fix issue 9863 - Change Tab selection order May 16, 2023
/**
* Focus on GroupTree
*/
public void requestFocusGroupTree() { groupTree.requestFocus(); }
Copy link
Member

Choose a reason for hiding this comment

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

Please fix the checkstyle issue

Copy link
Contributor Author

Choose a reason for hiding this comment

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

checkstyle issue fixed

Comment on lines 77 to 82
for (Node child : getChildren()) {
if (child instanceof GroupTreeView) {
((GroupTreeView) child).requestFocusGroupTree();
break;
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Casting can be omitted by the new pattern matching feature of java

Suggested change
for (Node child : getChildren()) {
if (child instanceof GroupTreeView) {
((GroupTreeView) child).requestFocusGroupTree();
break;
}
}
for (Node child : getChildren()) {
if (child instanceof GroupTreeView groupTreeView) {
groupTreeView.requestFocusGroupTree();
break;
}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

casting has been replaced by pattern matching

@@ -55,7 +55,8 @@ public enum KeyBinding {
FILE_LIST_EDITOR_MOVE_ENTRY_DOWN("File list editor, move entry down", Localization.lang("File list editor, move entry down"), "ctrl+DOWN", KeyBindingCategory.VIEW),
FILE_LIST_EDITOR_MOVE_ENTRY_UP("File list editor, move entry up", Localization.lang("File list editor, move entry up"), "ctrl+UP", KeyBindingCategory.VIEW),
FIND_UNLINKED_FILES("Search for unlinked local files", Localization.lang("Search for unlinked local files"), "shift+F7", KeyBindingCategory.QUALITY),
FOCUS_ENTRY_TABLE("Focus entry table", Localization.lang("Focus entry table"), "alt+1", KeyBindingCategory.VIEW),
FOCUS_ENTRY_TABLE("Focus entry table", Localization.lang("Focus entry table"), "TAB", KeyBindingCategory.VIEW),
Copy link
Member

Choose a reason for hiding this comment

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

Is it still possible to navigate away from the entry table? I would see tab as cycling though different ui components: Main menu entries, groups, main table, entry editor, ...

It should be possible to focus these elements using quick shortcut. Alr+1 was such a shortcut.

@Siedlerchr
Copy link
Member

DevCall: Decision: Remove unused Preamble Store Change key binding
Reuse default keybinding alt + s to focus on the group

@Siedlerchr Siedlerchr added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Jun 10, 2023
@calixtus calixtus merged commit 94f964c into JabRef:main Jun 10, 2023
@calixtus
Copy link
Member

Thank you for your contribution. We would be happy to see more contributions from your side.

@calixtus calixtus added ui and removed status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers labels Jun 10, 2023
Siedlerchr added a commit that referenced this pull request Jan 1, 2024
* upstream/main: (68 commits)
  Fix issue 9863 - Change Tab selection order (#9907)
  New Crowdin updates (#9994)
  Enable editing typo with double clicking on field name in custom entry type (#9977)
  Remove "Field name:" from localization - we already have "Field name" (#9991)
  Changed database to catalog in org.jabref.gui.slr and org.jabref.logic.crawler (#9989)
  Use environment variables for hostname detection (#9910)
  Add cleanup activity for URL field (#9970)
  Fix freezing when fetching IBSN and no results are found (#9987)
  Make Group(Node)TreeViewModel more OO (#9978)
  Fix container for group item count still visible if display count is off  (#9980)
  Fix paste of BibTeX data (#9985)
  Bring back SimplifyBoolean* and UnnecessaryParantheses (and refine guide) (#9981)
  Add new menu entry to remove braces from selection, aka unprotect it. (#9968)
  User specific comment (#9727)
  Bump org.openrewrite.recipe:rewrite-recipe-bom from 1.19.3 to 2.0.0 (#9975)
  Fix typos
  Remove non-needed link
  Fix typos
  Enable RemoveJavaDocAuthorTag (#9973)
  Bump com.fasterxml.jackson.datatype:jackson-datatype-jsr310 (#9974)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change tab selection order
5 participants