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

User configurable shortcuts #11

Open
letsfindaway opened this issue Feb 28, 2021 · 11 comments
Open

User configurable shortcuts #11

letsfindaway opened this issue Feb 28, 2021 · 11 comments
Assignees
Labels
1.8.0 enhancement New feature or request

Comments

@letsfindaway
Copy link
Owner

letsfindaway commented Feb 28, 2021

The ability to configure the shortcuts to trigger actions is one of the often requested features for OpenBoard. In this issue I will collect some of the ideas for implementing this.

An example of a shortcut configuration dialog can be seen in QtCreator. Here a screenshot of the relevant part:
image
The dialog consists of a table of all actions, grouped in topics. For each action, the name, a description and the currently assigned shortcut is listed. Above the table there is a text field to filter the shortcuts by name. When a row is selected, the shortcut is shown in a text input field below the table. Here it can be literally edited, or the key combination can be recorded. There is also a button to reset the shortcut to the application default. Below the table, the dialog also offers the option to import or export shortcut definitions.

Not shown on the screenshot: When a shortcut was modified, it is shown in bold italic. During entry, it is checked for correctness and a warning message is shown below the input field. Here also a message is shown when an entry collides with other shortcut definitions.

Special ideas for OpenBoard:

  • Limit the shortcuts to a single key. No key sequences with more than one key.
  • Checkbox to enable activation of actions without Ctrl key, i.e. E switches to eraser, not Ctrl+E.
  • Additional column to assign special mouse or stylus buttons to the actions.
    • However note that the QAction shortcut is defined by a QKeySequence, and this can only represent keyboard keys! We could however send mouse events and tablet events first to the shortcut manager, which filters them and intercepts those used for a shortcut.
    • We could also offer to record such an event.

Questions coming into my mind:

  • How do we group the actions?
    • Many actions are grouped in UBActionPalette objects. This could be a very useful and natural grouping for those actions.
    • Unfortunately the palettes are created in various places and have no object names or tool tips. Seems that we have to touch all these places.
    • Others are grouped in the three toolbars for board, document and web. This grouping is reflected in their parent/child relationship, and the toolbars have an object name and a translated windowTitle.
    • Some of the actions are however present in several toolbars. Here we have to decide where to put them. Probably in a "Common" group?
  • Should the actions be scoped? I.e. for document mode, board mode etc? Note that the shortcut context won't help here, as all actions are assigned to the main window. We would have to move the actions to distinct widgets.
  • Shall the shortcuts be saved as part of the normal user configuration file? Or should there be a special shortcut configuration file, which also resembles the import/export format?
  • Could we generate a printable list of effective shortcuts?

@kaamui: feel free to comment! Too complicated? Other features needed? Your ideas?!

@letsfindaway
Copy link
Owner Author

letsfindaway commented Mar 8, 2021

Some more implementation considerations:

  • The UI handling is implemented in the UBPreferencesController.
  • The UBPreferencesController allows to record a key combination, a mouse button (not the left and right) and a stylus button to an action.
  • There is a new UBShortcutManager which keeps a list of actions and manages the associated shortcuts. This class also assigns mouse and stylus buttons to the actions and checks for duplicate assignments.
    • The UBShortcutManager is also used to group the actions, so that there are nice headings to easier find an action.
  • The UBShortcutManager also handles persistence of the shortcut settings using UBSettings.
  • Information beyond the active keyboard shortcuts, e.g. the original default shortcut and the mouse and stylus button assignments are kept with the actions using dynamic properties.
  • The UBShortcutManager exposes its information as a QAbstractTableModel, which is used by the UBPreferencesController.
  • Keyboard shortcuts are handled via the standard means of the QAction.
  • A checkbox in the UI can enable automatic addition of secondary key sequences which do not require the Ctrl button to be pressed.
  • Mouse and stylus button events are intercepted by the UBApplication. If the UBPreferencesController is in recording mode, then these events are recorded. Else if the UBShortcutManager has an associated action, that action is triggered. In other cases, the default handling of the event is invoked.

Open points

  • It seems that there are actions which cannot be invoked with a shortcut. E.g. "Add page" does not work, while "Next page" and "Previous page" do. The UBBoardPaletteManager does some weird things with this action to enable the pages menu with a long button press. We probably have to reconsider the way we collect the actions for the shortcuts, so that actionNewPage is inserted instead of actionPages.

@letsfindaway
Copy link
Owner Author

letsfindaway commented Mar 9, 2021

As this is not a new topic, I want to collect what I can find in existing (open and closed) issues and pull requests:

There are much more, mainly closed as duplicates. Let me categorize them as far as I understand them:

  1. Most requested is the ability to configure keyboard shortcuts for all actions.
  2. Also very common is the request to use special mouse buttons or stylus buttons to trigger actions.
  3. Here often also new actions are proposed like cycling through colors.

In this issue I want to concentrate on the first two: make the shortcuts configurable and also use mouse and stylus buttons to trigger actions. Number three requires new actions, which will not be the primary purpose of this issue.

@letsfindaway
Copy link
Owner Author

Here is the initial draft of a user interface for configurable shortcuts:

image

  • The table lists actions sorted by groups.
  • There is a line edit field to filter the list. The entered text is searched in the first two columns. Only matching actions along with their group titles will be shown.
  • Each action may have a key sequence, a mouse button and a stylus button specified. They are listed in colums 3 to 5.
  • A checkbox below the table allows to activate a shortcut without pressing the Ctrl key. So e.g. I is sufficient to select the pen instead of Ctrl+I.
  • The bottom part is an edit area for the selected table entry. Shortcuts are "recorded", i.e. you just activate the proper keyboard sequence, mouse button or stylus button, and these actions are recorded and will then be applied as shortcuts to the action. This prevents entering typos like Crtl+X or button specifiers which simply do not exist on the user's hardware. Note that the "recording area" will most probably not be present in the final implementation, as it might not be necessary.
  • An error message will be shown in a status line at the bottom, when a shortcut is duplicated. The table will then automatically be filtered to show the offending entry.
  • There is the option to revert a single action or the complete table to defaults.

@kaamui: I would really appreciate your feedback - and that of any other user, too!

@kaamui
Copy link

kaamui commented Mar 10, 2021

I'll try to look at it asap. It looks great ! Thank you a lot for the help you provide to this project.

@letsfindaway
Copy link
Owner Author

I'll try to look at it asap. It looks great ! Thank you a lot for the help you provide to this project.

Thanks for your feedback. Currently the code is not complete, but I want to check that I'm going in the right direction. I will inform you when I have pushed something working.

I'm currently also thinking whether I could also include a feature to configure keys and buttons which modify tool behavior when pressed, as this was also often requested (e.g. panning, switching to eraser, etc). I will for sure not implement those features, but at least a framework to configure the keys and buttons used for such things if they will come in the future.

@letsfindaway letsfindaway added the enhancement New feature or request label Mar 10, 2021
@letsfindaway
Copy link
Owner Author

letsfindaway commented Mar 13, 2021

Further TODOs:

  • Add all relevant actions to the UBShortcutManager and group them. For all actions being part of UBMainWindow the UBShortcutManager will do this grouping. Additional actions can be contributed by other components if necessary.
  • Implement filtering. All columns of the table are considered when looking for a match, so that you can also use filtering to search which action is e.g. performed by Ctrl+H.
  • Collision detection and warning. Will get active as soon as the final key of a key sequence is released during recording.
  • Persistence using the UBSettings. Use the action name as key and a QStringList as value.
  • Shortcuts without Ctrl including collision detection. The checkbox is disabled when activating would lead to a collision.
  • In a UBActionPalette, the actions should be grouped instead of the buttons. This enables the exclusive behavior even in the case that an action is triggered by a shortcut and not only when a button is pressed.
  • Some keyboard shortcuts are hard-coded in UBBoardView::keyPressEvent. This causes that they do not show up here and are not configurable. However some actions are assigned to multiple keys, and that is something the UBShortcutController currently does not handle. Even if we don't change this, we should avoid to collide with the shortcuts defined there.
  • Shortcuts should also work when the associated palette is not visible.

@letsfindaway
Copy link
Owner Author

letsfindaway commented Mar 13, 2021

@kaamui The current status at https://github.com/letsfindaway/OpenBoard/tree/shortcut-configuration are ready to give you an impression of the current status of my work. What is missing are is the one unchecked box in the comment above.

I just opened a draft pull request OpenBoard-org#460, as I would like to have intensive testing on other platforms by other users.

Further points:

  • Implement "Reset to default" for complete Shortcut tab.
  • Fix and improve conflict detection for "Ignore Ctrl" mode.
  • Add list of conflicting hard-coded shortcuts as disabled actions, probably shown in grey and/or italic.
  • Refactor catching of mouse/tablet events. Use || to chain.
  • Add a UBModifierButton class which can be linked to keys, mouse and tablet buttons. Add methods to add objects of that class. (deferrd to a point in time when this is necessary)
  • Show also secondary shortcuts in table.
  • Add more actions and action groups, e.g. Podcast, addItem actions, ...
  • Remove mouse/tablet actions from action map when Reset is pressed.
  • Improve includes for compilation with Qt 5.15 (used on Tumbleweed)
  • Update language files and German translation

@kaamui
Copy link

kaamui commented Mar 16, 2021

Fix and improve conflict detection for "Ignore Ctrl" mode.

About this particular point (no need for ctrl), where the focus needs to be for the shortcut to be triggered ? Is it automatically handled by Qt that when focus is on an editable QGraphicsTextItem it should not be triggered ? Are the events "attached" to a global handler that is listening or does it need to be attached to every view ?

@letsfindaway
Copy link
Owner Author

letsfindaway commented Mar 16, 2021

tl;dr: it works!

Longer explanation:

I'm not the Qt event handling expert, but what I understood from the documentation and all my tests is the following:

  • The key event is created by Qt and delivered to the QApplication.
  • Here you could filter the event, but that's not what I'm doing for keyboard events.
  • The event then reaches the event() function of the QApplication. Here it is decided, to whom the event is offered.
  • First the utmost active leaf in the widget tree is selected.
  • If the event is not handled here, it bubbles upward towards the root.
  • At each level, shortcut handlers visible from that level are also considered. Shortcuts attached to invisible widgets are ignored.
  • If nobody handles the event, it is dropped.

Currently most actions are attached to buttons. Therefore shortcuts only work, when the buttons are visible. I'm additionally attaching the actions to the main window. Actions can be attached to multiple widgets, which makes them accessible when at least one of those widgets is visible and enabled. So the shortcuts are also working when the associated button is not visible, because visibility of the main window is sufficient.

The option "Ignore Ctrl" adds secondary shortcuts to the actions. So an action with shortcut "Ctrl+B" gets a secondary shortcut "B". What happens when the QGraphicsTextItem is active? Just check the event handling list above: The key press is first offered to the QGraphicsTextItem before it is evaluated as a possible shortcut. As the widget handles the event, it will not trigger the shortcut action. With OpenBoard, I think the event is first offered to UBBoardView where it is forwarded to the active scene which delivers it to the QGraphicsTextItem, but that's just another indirection and does not change the general processing.

I think the important fact is, that the shortcut check for actions attached to the main window is only executed when the key event was not handled by an active child widget before.

Please correct me if I'm wrong, but this is what I have understood.

@kaamui
Copy link

kaamui commented Mar 22, 2021

Hi,

Thanks for your work, and the time you take to answer. I'll try to look at your work as soon as I can.

@letsfindaway
Copy link
Owner Author

Instead of the UBModifierButton mentioned above I will implement a "switch tool while key is pressed" feature. So if you're e.g. using the pen tool and are pressing and holding down Ctrl+E (default shortcut for eraser tool), then the tool will switch to "Eraser" as long as you keep the keys pressed. When the E is released, the previous tool (here: pen tool) is selected again.

This will work with all shortcuts of the tool palette and in addition to the function where pressing a shortcut key combination switches permanently to a new tool. The distinguishing of the behavior is based on time: When the keys are pressed longer than the keyboard autorepeat interval, then the temporary switching will be activated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.8.0 enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants