Change the "download" keyboard shortcut (Ctrl+S) handling, in GENERIC/CHROME builds, to utilize the EventBus (issue 11657); add a new "openfile" keyboard shortcut (Ctrl+O), in GENERIC builds
#11826
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…/CHROME builds, to utilize the `EventBus` (issue 11657) This improves the consistency of the "download" handling, in the default viewer, such that the `Toolbar`/`SecondaryToolbar` buttons *and* the keyboard shortcut are now handled in the same way (using the `EventBus`). Given that the "download" keyboard shortcut handling is limited to GENERIC/CHROME builds and that the issue does raise a valid point about only being able to observe *some* downloads, these changes seem acceptable in this particular case. Finally the pre-processor condition is adjusted to *explicitly*, rather than implicitly, list the affected build targets. *Please note:* This patch should NOT be construed as carte blanche to simply convert all of the code in `webViewerKeyDown`, or elsewhere, to make use of the `EventBus` instead of direct function calls. Any further changes, along the lines in this patch, would need to be evaluated on a case-by-case basis to determine if they are actually wanted, given that many/most existing cases in `webViewerKeyDown` should already be *indirectly* observable through the `EventBus` instance.
Somewhat surprisingly, despite the GENERIC viewer implementing "openfile" support, there's never been a keyboard shortcut available. Similar to the previous patch, this utilizes the `EventBus` for consistency with the `Toolbar`/`SecondaryToolbar` buttons. *Please note:* This patch should NOT be construed as carte blanche to simply convert all of the code in `webViewerKeyDown`, or elsewhere, to make use of the `EventBus` instead of direct function calls. Any further changes, along the lines in this patch, would need to be evaluated on a case-by-case basis to determine if they are actually wanted, given that many/most existing cases in `webViewerKeyDown` should already be *indirectly* observable through the `EventBus` instance.
|
/botio-linux preview |
From: Bot.io (Linux m4)ReceivedCommand cmd_preview from @timvandermeij received. Current queue size: 0 Live output at: http://54.67.70.0:8877/17f1446f236cc6e/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/17f1446f236cc6e/output.txt Total script time: 2.55 mins Published |
timvandermeij
approved these changes
Apr 20, 2020
|
Looks like a good improvement; thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Change the "download" keyboard shortcut (Ctrl+S) handling, in GENERIC/CHROME builds, to utilize the
EventBus(issue 11657)This improves the consistency of the "download" handling, in the default viewer, such that the
Toolbar/SecondaryToolbarbuttons and the keyboard shortcut are now handled in the same way (using theEventBus).Given that the "download" keyboard shortcut handling is limited to GENERIC/CHROME builds and that the issue does raise a valid point about only being able to observe some downloads, these changes seem acceptable in this particular case.
Finally the pre-processor condition is adjusted to explicitly, rather than implicitly, list the affected build targets.
Fixes No event bus used for Ctrl+S #11657
Add a new "openfile" keyboard shortcut (Ctrl+O), in GENERIC builds
Somewhat surprisingly, despite the GENERIC viewer implementing "openfile" support, there's never been a keyboard shortcut available. Similar to the previous patch, this utilizes the
EventBusfor consistency with theToolbar/SecondaryToolbarbuttons.Please note: This patch should NOT be construed as carte blanche to simply convert all of the code in
webViewerKeyDown, or elsewhere, to make use of theEventBusinstead of direct function calls.Any further changes, along the lines in this patch, would need to be evaluated on a case-by-case basis to determine if they are actually wanted, given that many/most existing cases in
webViewerKeyDownshould already be indirectly observable through theEventBusinstance.