SQL Lab editor shortcuts override other common shortcuts

[qyra]

The keyboard shortcuts in the "SQL Editor" view cause issues while editing because they override common text editing and browser shortcut conventions. I've had issues with them in Chromium and Firefox on Linux, and a colleague has had issues in Safari on a mac as well as well.

On Chrome:
Control-X is usually "cut" in a text box, in the editor it has been overridden to stop the query.
Control-F (find) is overridden and I have no idea what it was changed to (I don't see it documented)

Also, the "Keyboard Shortcuts" widget says that Control-T should open a new query tab but it actually opens a new browser tab so whatever override is attempted there doesn't work - but overriding standard browser shortcuts doesn't seem great so I would argue this is a feature, not a bug.

On Safari:
On safari Cmd-X still works for "cut" so presumably the shortcuts treat control and command differently. Cmd-F (find) is still broken.

Just for completeness the following work as expected:
Control-C (copy), Control-V (paste), Control-Z (undo), Control-Y (redo), Control-A (select all), Control-/ (comment code)

Suggested Fix

Any of the following would be good:

• Pick new default keyboard shortcuts that do not override common text and browser shortcuts
• Allow changing individual shortcuts permanently (Either with some superset config, or with a user preference)
• If a keyboard shortcut change menu is too much work for this, being able to just disable individual shortcuts would also work as a stopgap.

Server Environment

Superset 0.36.0
Python 3.6.9
Flask 1.1.2
Werkzeug 1.0.1

Browser

Chromium on Linux, Version 83.0.4103.61

Checklist

• I have checked the superset logs for python stacktraces and included it here as text if there are any.
• I have reproduced the issue with at least the latest released version of superset.
• I have checked the issue tracker for the same issue and I haven't found one similar.

[issue-label-bot]

Issue-Label Bot is automatically applying the label #bug to this issue, with a confidence of 0.81. Please mark this comment with 👍 or 👎 to give our bot feedback!

Links: app homepage, dashboard and code for this bot.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. For admin, please label this issue .pinned to prevent stale bot from closing the issue.

[rumbin]

Do not close, please

[rumbin]

Related: #12429

[john-bodley]

@suddjian or @nytai should we update the SQL Lab shortcuts so they don't conflict with frequently used keyboards shortcuts?

[nytai]

@john-bodley My instinct would be yes, as it can be quite annoying. @betodealmeida @eschutho any thoughts here?

[eschutho]

@yousoph has done some investigation into this and we've made some small changes, but yes, they shouldn't conflict.

[nickbp]

Seeing this on Superset 1.4.2 with both Firefox 98.0.2 and Chromium 100.0.4896.60 on Linux/Gnome. Ctrl+X and Ctrl+F shortcuts are silently ignored.

Repro steps are straightforward:

• Navigate to http://[superset]/superset/sqllab/
• Attempt to cut text in editor box using Ctrl+X, observe nothing happens
  • Attempt to cut text by right-clicking and selecting the Cut option, observe that it works fine (so breakage seems to be specific to the keyboard shortcut rather than the cut operation itself)
  • Attempt to copy text using Ctrl+C, observe that it works fine (so breakage seems specific to the cut shortcut)
• Attempt to bring up text search prompt using Ctrl+F, observe nothing happens

To be honest I wonder if this is only affecting a small subset of users because the lack of a cut shortcut in particular makes the in-Superset SQL editor fairly unusable for me. As such I'd have expected more noise on this issue if it were more widespread.

I've gotten to where I just do my primary editing in a separate text editor, since the ability to cut text comes up pretty often for me when constructing a query. My RSI is bad enough without needing to right-click to cut text.

[nickbp]

It looks like the source of the problem is that Ctrl+X is being overridden to instead be "stop query":

superset/superset-frontend/src/SqlLab/components/SqlEditor/index.jsx

Lines 344 to 347 in 02ef9ca

	name: 'stopQuery',
	key: 'ctrl+x',
	descr: t('Stop query'),
	func: this.stopQuery,

I think it'd make sense to just pick a different shortcut for that so that cutting text isn't broken in the editor. Alt+X is available?

Unrelated to this issue, but there's a similar override for Ctrl+R in the same file that conflicts with the common browser shortcut for reloading the page:

superset/superset-frontend/src/SqlLab/components/SqlEditor/index.jsx

Lines 312 to 319 in 02ef9ca

	name: 'runQuery1',
	key: 'ctrl+r',
	descr: t('Run query'),
	func: () => {
	if (this.state.sql.trim() !== '') {
	this.runQuery();
	}
	},

I don't see the override that's breaking Ctrl+F in there, it might be in a different component.

[nickbp]

Talked with someone else about this issue and they hadn't had any problems because they're using Superset on a Mac. So Cmd+X to cut for example works fine for them, but anyone on Windows or Linux is probably seeing this problem.

[nickbp]

I've figured out a quick hack that fixes ctrl+x at server startup. This method is very ugly but it has the benefit of avoiding the need to rebuild the whole superset frontend/image if you already have an existing environment going.

The example provided here just switches the errant ctrl+x binding for "stop query" referenced above to instead be alt+x:

sed -i s/ctrl+x/alt+x/ /app/superset/static/assets/sqllab.*.js \
&& gunicorn [...]

After restarting the Superset container/process to have the above edit, and then reloading the frontend in my browser (Ctrl+Shift+R to ensure the static assets are redownloaded), cutting/pasting text is no longer broken in the SQL Lab editor on Linux/Windows.

It might be possible to do something similar to unbreak other standard shortcuts as desired. For me specifically, getting cut/paste functionality unbroken quickly was important for my own sanity. I imagine I'm not the only one so hopefully this is helpful for anyone else who's been encountering the issue over the last several months.

[eschutho]

Thank you for the finding this issue. We've done some workarounds for shortcuts in Windows vs Mac, and I agree that this is an important shortcut for Windows users. @yousoph do you think someone can look into this?

[rusackas]

@yousoph / @kasiazjc has anyone looked into the current state of affairs here? It might be nice to either recap the current status on this thread, or perhaps close this issue an start a new one if that's a cleaner way to document the remaining work needed.

[villebro]

@rusackas @yousoph as we're already using proper ⌘ keyboard shortcuts on Mac on the dashboard page (see pic below), wouldn't it be prudent to migrate SQL Lab to the same convention? It feels janky to be doing ctrl + return to run queries on Mac, when it really should be ⌘ + return.

cc: @kgabryje @john-bodley @michael-s-molina

[yousoph]

⌘ + return makes sense to me for run query

If we move Mac shortcuts from Ctrl to ⌘, I think the shortcuts that would conflict are:

• New tab: currently Ctrl + t on Macs, Ctrl + q on PCs
• Stop query: currently Ctrl + x, Ctrl + e on PCs

Would it make sense to add ⌘ shortcuts onto the existing Ctrl ones for Mac? ie for Run query, users could use ⌘ + return or Ctrl + return and for new tab, users could use Ctrl + t or ⌘ + q?

[rumbin]

I'd opt for introducing new shortcuts while keeping the old ones working. As long as the old shortcuts don't conflict in any way, it does not harm keeping them. And users will be happy, as they don't need to change habits and muscle memory.

[nickbp]

Per above, standard shortcuts like ctrl+x to cut text are already being broken under the current scheme. The conflicts are already there.

[sfirke]

I'm testing out the SQL Lab in 3.1.0rc1 and it has a handy list of keyboard shortcuts:

So ctrl + x is no longer a conflict, nor is ctrl + t. IMO it is okay that ctrl + f does a find-in-SQL, if I want a find-in-page I click outside the window and execute that command. The only conflict I see remaining with a common keyboard shortcut is ctrl + R to run a query -- not a problem for me but given that there are other shortcuts available, maybe that one could be dropped?

[geido]

I am closing this for now as it seems that most issues have been resolved. Contributions are welcome to iterate on these controls more if necessary. Thanks all!