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

fix(sqllab): Remove invalid hotkey config #21969

Closed

Conversation

justinpark
Copy link
Member

SUMMARY

There's a bug that throws an error when hit ctrl + p shortcut out of the editor textbox because it contains the editor specific command is included in the global keyboard shortcut handlers.

This commit removes the editor specific command from the global shortcut list.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before:

hotkey-errors.mov

After:

No errors thrown

TESTING INSTRUCTIONS

  1. Go to sqllab
  2. Focus out of the sql editor input
  3. Hit Ctrl + p and then check console log

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@codecov
Copy link

codecov bot commented Nov 2, 2022

Codecov Report

Merging #21969 (8c36db1) into master (7f563cf) will decrease coverage by 1.18%.
The diff coverage is 69.36%.

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

@@            Coverage Diff             @@
##           master   #21969      +/-   ##
==========================================
- Coverage   66.96%   65.77%   -1.19%     
==========================================
  Files        1807     1813       +6     
  Lines       69221    69425     +204     
  Branches     7402     7448      +46     
==========================================
- Hits        46352    45664     -688     
- Misses      20964    21841     +877     
- Partials     1905     1920      +15     
Flag Coverage Δ
javascript 53.50% <63.15%> (+0.10%) ⬆️
unit ?

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

Impacted Files Coverage Δ
...ackages/superset-ui-chart-controls/src/fixtures.ts 100.00% <ø> (ø)
...d/packages/superset-ui-chart-controls/src/index.ts 100.00% <ø> (ø)
...perset-ui-chart-controls/src/sections/sections.tsx 71.42% <0.00%> (-16.08%) ⬇️
...chart-controls/src/shared-controls/dndControls.tsx 58.33% <ø> (ø)
...d/packages/superset-ui-chart-controls/src/types.ts 100.00% <ø> (ø)
...kages/superset-ui-core/src/query/types/Operator.ts 100.00% <ø> (ø)
...packages/superset-ui-core/src/query/types/Query.ts 100.00% <ø> (ø)
...lugin-chart-handlebars/src/plugin/controlPanel.tsx 50.00% <ø> (ø)
...d/src/SqlLab/components/SaveDatasetModal/index.tsx 52.27% <0.00%> (-0.61%) ⬇️
...perset-frontend/src/addSlice/AddSliceContainer.tsx 59.61% <0.00%> (ø)
... and 142 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@stephenLYZ
Copy link
Member

/testenv up

@github-actions
Copy link
Contributor

github-actions bot commented Nov 4, 2022

@stephenLYZ Ephemeral environment spinning up at http://35.86.128.244:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

@justinpark
Copy link
Member Author

@stephenLYZ do you have any updates from test?

@justinpark
Copy link
Member Author

will be covered by #25228

@justinpark justinpark closed this Sep 11, 2023
@justinpark justinpark deleted the fix--sqllab-invalid-editor-hotkeys branch September 11, 2023 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants