Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

feat: Update ShortKey for stop query running in SqlLab editor #19692

Merged
merged 4 commits into from
Apr 27, 2022
Merged

feat: Update ShortKey for stop query running in SqlLab editor #19692

merged 4 commits into from
Apr 27, 2022

Conversation

codemaster08240328
Copy link
Contributor

SUMMARY

  • Change the shortcut for Stop Query from Control + X to Control + E on PC / Linux
  • Update the tooltip text on the stop query button to reflect the shortcut change
  • Mac shortcut should remain the same as it is today (Control+X)

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

  1. Go to SqlLab
  2. Run any query
  3. Try to press ctrl + e(on PC, Linux) or ctrl + x (on Mac)
  4. Check tool tip when hovering on Stop query button

ADDITIONAL INFORMATION

@codecov
Copy link

codecov bot commented Apr 13, 2022 •
edited
Loading

Codecov Report

Merging #19692 (c8b51d1) into master (5e468f7) will increase coverage by 0.00%.
The diff coverage is 73.33%.

❗ Current head c8b51d1 differs from pull request most recent head debc245. Consider uploading reports for the commit debc245 to get more accurate results

@@           Coverage Diff           @@
##           master   #19692   +/-   ##
=======================================
  Coverage   66.54%   66.54%           
=======================================
  Files        1692     1692           
  Lines       64775    64811   +36     
  Branches     6661     6663    +2     
=======================================
+ Hits        43103    43131   +28     
- Misses      19972    19978    +6     
- Partials     1700     1702    +2
Flag Coverage Δ
javascript 51.24% <58.33%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...omponents/ColumnConfigControl/ColumnConfigItem.tsx 0.00% <ø> (ø)
...in-chart-echarts/src/MixedTimeseries/buildQuery.ts 0.00% <ø> (ø)
...frontend/src/SqlLab/components/SqlEditor/index.jsx 51.07% <0.00%> (-0.28%) ⬇️
.../src/explore/components/controls/SliderControl.tsx 0.00% <0.00%> (ø)
superset/views/base.py 73.77% <25.00%> (-0.74%) ⬇️
...c/SqlLab/components/RunQueryActionButton/index.tsx 53.12% <66.66%> (+1.40%) ⬆️
superset/db_engine_specs/presto.py 88.70% <71.42%> (-0.33%) ⬇️
...CRUD/alert/components/AlertReportCronScheduler.tsx 87.50% <83.33%> (+10.57%) ⬆️
superset/models/sql_types/presto_sql_types.py 80.39% <84.61%> (+0.90%) ⬆️
superset/views/base_api.py 98.38% <100.00%> (+0.06%) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5e468f7...debc245. Read the comment docs.

diegomedina248
diegomedina248 reviewed Apr 13, 2022
View reviewed changes
superset-frontend/src/SqlLab/components/RunQueryActionButton/index.tsx Outdated
@@ -114,7 +117,9 @@ const RunQueryActionButton = ({
tooltip={
(!isDisabled &&
(shouldShowStopBtn
? t('Stop running (Ctrl + x)')
? userOS === 'MacOS'
Copy link
Contributor

Choose a reason for hiding this comment

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

we could move this to a memo, or even a plain function outside of the component called getStopButtonTooltipText or something.. Seems too much inline ifs for my liking here

Copy link
Member

Choose a reason for hiding this comment

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

Willing to take a look at externalizing this, and squeeze it into this PR, @codemaster08240328?

@pull-request-size pull-request-size bot added size/M and removed size/S labels Apr 14, 2022
@rusackas rusackas merged commit 60e06c1 into apache:master Apr 27, 2022
@rusackas
Copy link
Member

Tests for this would be appreciated as a follow-up PR.

hughhhh pushed a commit to hve-labs/superset that referenced this pull request May 11, 2022
@codemaster08240328
feat: Update ShortKey for stop query running in SqlLab editor (apache…
f0d3f91 
…#19692)

* feat: Update shortkey for stop query running in sqllab editor

* resolve comment

* fix invalid import useMemo
philipher29 pushed a commit to ValtechMobility/superset that referenced this pull request Jun 9, 2022
@codemaster08240328 @philipher29
feat: Update ShortKey for stop query running in SqlLab editor (apache…
f620e47 
…#19692)

* feat: Update shortkey for stop query running in sqllab editor

* resolve comment

* fix invalid import useMemo
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 2.0.0 labels Mar 13, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@diegomedina248 diegomedina248 diegomedina248 left review comments

@rusackas rusackas rusackas approved these changes

Assignees
No one assigned
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels Preset-Patch size/M 🚢 2.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@codemaster08240328 @rusackas @diegomedina248 @mistercrunch