You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2022/04/13 14:28:31 UTC
[GitHub] [superset] codemaster08240328 opened a new pull request, #19692: feat: Update ShortKey for stop query running in SqlLab editor
codemaster08240328 opened a new pull request, #19692:
URL: https://github.com/apache/superset/pull/19692
<!---
Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
Example:
fix(dashboard): load charts correctly
-->
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
- 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
<!--- Skip this if not applicable -->
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
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
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [x] Has associated issue: https://github.com/apache/superset/issues/9990
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] rusackas commented on a diff in pull request #19692: feat: Update ShortKey for stop query running in SqlLab editor
Posted by GitBox <gi...@apache.org>.
rusackas commented on code in PR #19692:
URL: https://github.com/apache/superset/pull/19692#discussion_r852488229
##########
superset-frontend/src/SqlLab/components/RunQueryActionButton/index.tsx:
##########
@@ -114,7 +117,9 @@ const RunQueryActionButton = ({
tooltip={
(!isDisabled &&
(shouldShowStopBtn
- ? t('Stop running (Ctrl + x)')
+ ? userOS === 'MacOS'
Review Comment:
Willing to take a look at externalizing this, and squeeze it into this PR, @codemaster08240328?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] rusackas merged pull request #19692: feat: Update ShortKey for stop query running in SqlLab editor
Posted by GitBox <gi...@apache.org>.
rusackas merged PR #19692:
URL: https://github.com/apache/superset/pull/19692
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] commented on pull request #19692: feat: Update ShortKey for stop query running in SqlLab editor
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #19692:
URL: https://github.com/apache/superset/pull/19692#issuecomment-1098168159
# [Codecov](https://codecov.io/gh/apache/superset/pull/19692?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#19692](https://codecov.io/gh/apache/superset/pull/19692?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (110dde9) into [master](https://codecov.io/gh/apache/superset/commit/d693f4e9700e932a29cb51583b26e10793aeab17?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d693f4e) will **decrease** coverage by `0.03%`.
> The diff coverage is `85.97%`.
> :exclamation: Current head 110dde9 differs from pull request most recent head dfebe6b. Consider uploading reports for the commit dfebe6b to get more accurate results
```diff
@@ Coverage Diff @@
## master #19692 +/- ##
==========================================
- Coverage 66.30% 66.27% -0.04%
==========================================
Files 1681 1683 +2
Lines 64408 64520 +112
Branches 6593 6607 +14
==========================================
+ Hits 42704 42758 +54
- Misses 20020 20068 +48
- Partials 1684 1694 +10
```
| Flag | Coverage Δ | |
|---|---|---|
| mysql | `81.95% <97.05%> (+0.05%)` | :arrow_up: |
| postgres | `?` | |
| python | `81.95% <97.05%> (-0.09%)` | :arrow_down: |
| sqlite | `?` | |
| unit | `47.75% <62.74%> (?)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/19692?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...chart-controls/src/shared-controls/dndControls.tsx](https://codecov.io/gh/apache/superset/pull/19692/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL3NoYXJlZC1jb250cm9scy9kbmRDb250cm9scy50c3g=) | `35.89% <ø> (ø)` | |
| [...controls/src/shared-controls/emitFilterControl.tsx](https://codecov.io/gh/apache/superset/pull/19692/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL3NoYXJlZC1jb250cm9scy9lbWl0RmlsdGVyQ29udHJvbC50c3g=) | `50.00% <ø> (ø)` | |
| [...et-ui-chart-controls/src/shared-controls/index.tsx](https://codecov.io/gh/apache/superset/pull/19692/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL3NoYXJlZC1jb250cm9scy9pbmRleC50c3g=) | `36.19% <ø> (ø)` | |
| [...nd/plugins/legacy-preset-chart-deckgl/src/utils.js](https://codecov.io/gh/apache/superset/pull/19692/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9sZWdhY3ktcHJlc2V0LWNoYXJ0LWRlY2tnbC9zcmMvdXRpbHMuanM=) | `0.00% <0.00%> (ø)` | |
| [...gacy-preset-chart-nvd3/src/DistBar/controlPanel.ts](https://codecov.io/gh/apache/superset/pull/19692/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9sZWdhY3ktcHJlc2V0LWNoYXJ0LW52ZDMvc3JjL0Rpc3RCYXIvY29udHJvbFBhbmVsLnRz) | `11.11% <ø> (ø)` | |
| [...s/plugin-chart-echarts/src/BoxPlot/controlPanel.ts](https://codecov.io/gh/apache/superset/pull/19692/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvQm94UGxvdC9jb250cm9sUGFuZWwudHM=) | `100.00% <ø> (ø)` | |
| [...ns/plugin-chart-echarts/src/Gauge/controlPanel.tsx](https://codecov.io/gh/apache/superset/pull/19692/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvR2F1Z2UvY29udHJvbFBhbmVsLnRzeA==) | `100.00% <ø> (ø)` | |
| [...gin-chart-echarts/src/Timeseries/transformProps.ts](https://codecov.io/gh/apache/superset/pull/19692/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvVGltZXNlcmllcy90cmFuc2Zvcm1Qcm9wcy50cw==) | `58.06% <0.00%> (ø)` | |
| [...ugin-chart-table/src/DataTable/hooks/useSticky.tsx](https://codecov.io/gh/apache/superset/pull/19692/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtdGFibGUvc3JjL0RhdGFUYWJsZS9ob29rcy91c2VTdGlja3kudHN4) | `4.21% <0.00%> (-0.05%)` | :arrow_down: |
| [superset-frontend/src/SqlLab/actions/sqlLab.js](https://codecov.io/gh/apache/superset/pull/19692/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9hY3Rpb25zL3NxbExhYi5qcw==) | `60.39% <ø> (ø)` | |
| ... and [51 more](https://codecov.io/gh/apache/superset/pull/19692/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/19692?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/19692?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [d693f4e...dfebe6b](https://codecov.io/gh/apache/superset/pull/19692?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] rusackas commented on pull request #19692: feat: Update ShortKey for stop query running in SqlLab editor
Posted by GitBox <gi...@apache.org>.
rusackas commented on PR #19692:
URL: https://github.com/apache/superset/pull/19692#issuecomment-1110486592
Tests for this would be appreciated as a follow-up PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] diegomedina248 commented on a diff in pull request #19692: feat: Update ShortKey for stop query running in SqlLab editor
Posted by GitBox <gi...@apache.org>.
diegomedina248 commented on code in PR #19692:
URL: https://github.com/apache/superset/pull/19692#discussion_r855283247
##########
superset-frontend/src/SqlLab/components/RunQueryActionButton/index.tsx:
##########
@@ -27,6 +27,7 @@ import {
DropdownButtonProps,
} from 'src/components/DropdownButton';
import { detectOS } from 'src/utils/common';
+import { useMemo } from '@storybook/addons';
Review Comment:
This one should be imported from React
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] rusackas commented on a diff in pull request #19692: feat: Update ShortKey for stop query running in SqlLab editor
Posted by GitBox <gi...@apache.org>.
rusackas commented on code in PR #19692:
URL: https://github.com/apache/superset/pull/19692#discussion_r856568233
##########
superset-frontend/src/SqlLab/components/RunQueryActionButton/index.tsx:
##########
@@ -27,6 +27,7 @@ import {
DropdownButtonProps,
} from 'src/components/DropdownButton';
import { detectOS } from 'src/utils/common';
+import { useMemo } from '@storybook/addons';
Review Comment:
Correct... and we can add this (and the lodash one, which is super dangerous) as an eslint restricted-imports rule.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] diegomedina248 commented on a diff in pull request #19692: feat: Update ShortKey for stop query running in SqlLab editor
Posted by GitBox <gi...@apache.org>.
diegomedina248 commented on code in PR #19692:
URL: https://github.com/apache/superset/pull/19692#discussion_r849619990
##########
superset-frontend/src/SqlLab/components/RunQueryActionButton/index.tsx:
##########
@@ -114,7 +117,9 @@ const RunQueryActionButton = ({
tooltip={
(!isDisabled &&
(shouldShowStopBtn
- ? t('Stop running (Ctrl + x)')
+ ? userOS === 'MacOS'
Review Comment:
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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] rusackas commented on a diff in pull request #19692: feat: Update ShortKey for stop query running in SqlLab editor
Posted by GitBox <gi...@apache.org>.
rusackas commented on code in PR #19692:
URL: https://github.com/apache/superset/pull/19692#discussion_r856568233
##########
superset-frontend/src/SqlLab/components/RunQueryActionButton/index.tsx:
##########
@@ -27,6 +27,7 @@ import {
DropdownButtonProps,
} from 'src/components/DropdownButton';
import { detectOS } from 'src/utils/common';
+import { useMemo } from '@storybook/addons';
Review Comment:
Correct... and we can add this (and the lodash one) as an eslint restricted-imports rule.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org