Skip to content

Conversation

rusackas
Copy link
Member

@rusackas rusackas commented Oct 7, 2021

SUMMARY

The SqlEditor had two small issues with some CSS that this PR corrects:

  1. It was directly using the supersetTheme instead of pulling it in via the ThemeProvider as is preferred.
  2. It randomly had a LESS var in there, which couldn't possibly be working... now it too uses the theme.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

No difference.

TESTING INSTRUCTIONS

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 Oct 7, 2021

Codecov Report

Merging #16999 (d5c4302) into master (cde4cdc) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #16999   +/-   ##
=======================================
  Coverage   76.93%   76.93%           
=======================================
  Files        1030     1030           
  Lines       55031    55033    +2     
  Branches     7465     7465           
=======================================
+ Hits        42336    42340    +4     
+ Misses      12441    12439    -2     
  Partials      254      254           
Flag Coverage Δ
hive 81.45% <ø> (ø)
javascript 70.90% <100.00%> (+<0.01%) ⬆️
mysql 81.90% <ø> (ø)
postgres 81.91% <ø> (ø)
presto 81.78% <ø> (+<0.01%) ⬆️
python 82.42% <ø> (+<0.01%) ⬆️
sqlite 81.58% <ø> (ø)

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

Impacted Files Coverage Δ
...frontend/src/SqlLab/components/SqlEditor/index.jsx 54.14% <100.00%> (+0.40%) ⬆️
superset/db_engine_specs/presto.py 90.37% <0.00%> (+0.41%) ⬆️

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 cde4cdc...d5c4302. Read the comment docs.

Copy link
Member

@villebro villebro left a comment

Choose a reason for hiding this comment

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

LGTM

@kgabryje kgabryje merged commit ac50961 into master Oct 7, 2021
@villebro villebro deleted the no-supersetTheme-direct-use-in-SqlEditor branch October 7, 2021 08:21
opus-42 pushed a commit to opus-42/incubator-superset that referenced this pull request Nov 14, 2021
…apache#16999)

* chore: no direct use of supersetTheme (or bad LESS vars) in SqlEditor

* putting the background back in, but using the theme.

* should be no need for this import anymore.
QAlexBall pushed a commit to QAlexBall/superset that referenced this pull request Dec 28, 2021
…apache#16999)

* chore: no direct use of supersetTheme (or bad LESS vars) in SqlEditor

* putting the background back in, but using the theme.

* should be no need for this import anymore.
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.5.0 First shipped in 1.5.0 labels Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels preset-io size/XS 🚢 1.5.0 First shipped in 1.5.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants