Skip to content

Conversation

@betodealmeida
Copy link
Member

SUMMARY

When switching away from a SQL tab with results and back again, the results don't show up because the height of the component is computed incorrectly.

I fixed it by simply reusing the height passed as props.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

How to repro:

  1. Go to SQL Lab and run a query with results
  2. Switch to another tab
  3. Switch back
  4. No results are displayed

With this PR, it works as expected:

results.mov

TEST PLAN

See above.

ADDITIONAL INFORMATION

  • Has associated issue:
  • Changes UI
  • Requires DB Migration.
  • Confirm DB Migration upgrade and downgrade tested.
  • Introduces new feature or API
  • Removes existing feature or API

@codecov-io
Copy link

codecov-io commented Jan 12, 2021

Codecov Report

Merging #12472 (1907dc2) into master (ddbcf5b) will increase coverage by 1.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #12472      +/-   ##
==========================================
+ Coverage   62.53%   63.55%   +1.01%     
==========================================
  Files        1014      486     -528     
  Lines       49509    29921   -19588     
  Branches     5078        0    -5078     
==========================================
- Hits        30962    19017   -11945     
+ Misses      18337    10904    -7433     
+ Partials      210        0     -210     
Flag Coverage Δ
javascript ?
python 63.55% <ø> (-0.18%) ⬇️

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

Impacted Files Coverage Δ
superset/db_engine_specs/presto.py 70.56% <0.00%> (-11.48%) ⬇️
superset/examples/world_bank.py 97.10% <0.00%> (-2.90%) ⬇️
superset/views/database/mixins.py 80.70% <0.00%> (-1.76%) ⬇️
superset/utils/core.py 88.56% <0.00%> (-1.31%) ⬇️
superset/models/core.py 88.04% <0.00%> (-0.55%) ⬇️
superset/commands/importers/v1/utils.py 86.04% <0.00%> (-0.07%) ⬇️
superset/viz_sip38.py 0.00% <0.00%> (ø)
superset/css_templates/api.py 95.74% <0.00%> (ø)
superset/annotation_layers/api.py 83.19% <0.00%> (ø)
superset/datasets/commands/importers/v1/utils.py 55.55% <0.00%> (ø)
... and 536 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 ddbcf5b...1907dc2. Read the comment docs.

? this.southPaneRef.current.clientHeight
: 0;
UNSAFE_componentWillReceiveProps(props) {
this.setState({ height: props.height });
Copy link
Member

Choose a reason for hiding this comment

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

since state is just a representation of props at this point, can we remove the state and just use the props?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, yeah — good point! :-P

@adam-stasiak
Copy link
Contributor

Tested manually - works fine

Copy link
Member

@eschutho eschutho left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM

@villebro
Copy link
Member

Merging for inclusion in 1.0.0rc2

@villebro villebro merged commit 086b3a6 into apache:master Jan 13, 2021
@villebro villebro added the v1.0 label Jan 13, 2021
villebro pushed a commit that referenced this pull request Jan 13, 2021
* fix (SQL Lab): disappearing results on tab switch

* Remove state

* Fix test
amitmiran137 pushed a commit to nielsen-oss/superset that referenced this pull request Jan 14, 2021
* fix (SQL Lab): disappearing results on tab switch

* Remove state

* Fix test
villebro pushed a commit to preset-io/superset that referenced this pull request Jan 15, 2021
* fix (SQL Lab): disappearing results on tab switch

* Remove state

* Fix test
etr2460 pushed a commit that referenced this pull request Jan 25, 2021
* release: bump to 1.0.0 and CHANGELOG

* fix(explore): long metric name display (#12387)

* fix(explore): long metric name display

* add tooltip to control

* chore: Show datasets when search input is empty (#12391)

* chore: Fix typo “Rest” to “Reset” (#12392)

* chore: upgrade eslint, babel, and prettier (#12393)

* feat(explore): add tooltip to timepicker label (#12401)

* chore: change Datasource to Dataset in Explore ui (#12402)

* chore(explore):change dataset to datasource in ui

* modal

* Add space

* Changing it back🤦🏾‍♀️

* Chargeback

* fix: Refresh Interval Modal dropdown (#12406)

* fix(native-filters): incorrect queriesData state (#12409)

* refactor: from superset.utils.core break down date_parser (#12408)

* Fixes control panel fields styling (#12236) (#12326)

* feat: Resizable dataset and controls panels on Explore view (#12411)

* Implement resizable panels on explore view

* Optimize chart rendering while resizing

* Make dataset column narrower

Co-authored-by: Evan Rusackas <evan@preset.io>

* fix(dashboard): artefacts shown while drag and dropping deck.gl charts (#12418)

* [12181] Fix artifacts while drag and dropping deck.gl charts.

* Run prettier

* bump superset-ui packages for rolling window change (#12426)

* chore: bump superset-ui deckgl plugin (#12466)

* fix: do not show vertical scrollbar for charts in dashboard (#12478)

* fix: do not show vertical scrollbar for charts in dashboard

* Proper fix for #11419

Co-authored-by: Jesse Yang <jesse.yang@airbnb.com>

* fix(dashboard): use datasource id from slice metadata (#12483)

* fix(timepicker): make pyparsing thread safe (#12489)

* fix: make pyparsing thread safe

* remove parenthesis for decorator

* fix (SQL Lab): disappearing results on tab switch (#12472)

* fix (SQL Lab): disappearing results on tab switch

* Remove state

* Fix test

* fix: import ZIP files that have been modified (#12425)

* fix: import ZIP files that have been modified

* Add unit test

* update changelog with rc2 entries

* fix: impose dataset ownership check on old API (#12491)

* fix: impose dataset ownership check on old API

* update UPDATING.md

* partially protect the old MVC also

* prevent metric and column add and update

* ci: remove refs/tags from docker tags on a release (#12518)

* ci: remove refs/tags from docker tags on a release

* wider head

* fix: lowercase all columns in examples (#12530)

* fix(explore): time table control panel (#12532)

* fix(explore): Add Time section back to FilterBox (#12537)

* Fixing Pinot queries for time granularities: WEEKS/MONTHS/QUARTERS/YEARS (#12536)

* fix: Select options overflowing Save chart modal on Explore view (#12522)

* Fix select options overflowing modal

* fix unit test

Co-authored-by: Ville Brofeldt <ville.v.brofeldt@gmail.com>

* Fix list filters vertical alignment (#12497)

* feat(db-engine): Add support for Apache Solr (#12403)

* [db engine] Add support for Apache Solr

* Fixing typo

* chore: rename docker image in build_docker_image.sh, docker-compose.yml and helm values.yaml (#12337)

* add rc3 changelog entries

* fix: Popover closes on change of dropdowns values (#12410)

* fix: Add MAX_SQL_ROW value to LIMIT_DROPDOWN (#12555)

* fix(viz): missing groupby and broken adhoc metrics for boxplot (#12556)

* fix: height on grid results (#12558)

* fix: case expression should not have double quotes (#12562)

* Fix 500 error when loading dashboards with slice having deleted dataset (#12535)

* add rc4 changelog entries

* Fixed typo on line 348

* Added files

Co-authored-by: Daniel Gaspar <danielvazgaspar@gmail.com>
Co-authored-by: Yongjie Zhao <yongjie.zhao@gmail.com>
Co-authored-by: Geido <60598000+geido@users.noreply.github.com>
Co-authored-by: Junlin Chen <junlin@preset.io>
Co-authored-by: Jesse Yang <jesse.yang@airbnb.com>
Co-authored-by: Agata Stawarz <47450693+agatapst@users.noreply.github.com>
Co-authored-by: Ville Brofeldt <33317356+villebro@users.noreply.github.com>
Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>
Co-authored-by: Kamil Gabryjelski <kamil.gabryjelski@gmail.com>
Co-authored-by: Evan Rusackas <evan@preset.io>
Co-authored-by: Kasia Kucharczyk <2536609+kkucharc@users.noreply.github.com>
Co-authored-by: Phillip Kelley-Dotson <pkelleydotson@yahoo.com>
Co-authored-by: Grace Guo <grace.guo@airbnb.com>
Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>
Co-authored-by: Ville Brofeldt <ville.v.brofeldt@gmail.com>
Co-authored-by: Xiang Fu <fx19880617@gmail.com>
Co-authored-by: Ahmed Adel <github@aadel.io>
Co-authored-by: Amit Miran <47772523+amitmiran137@users.noreply.github.com>
Co-authored-by: Hugh A. Miles II <hughmil3s@gmail.com>
Co-authored-by: Shuyao Bi <shuyaob@andrew.cmu.edu>
Co-authored-by: Lyndsi Kay Williams <lyndsikaywilliams@Lyndsis-MacBook-Pro.local>
@mistercrunch mistercrunch added 🍒 1.0.0 Cherry-picked to 1.0.0 🍒 1.0.1 Cherry-picked to 1.0.1 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.2.0 First shipped in 1.2.0 labels Mar 12, 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/S v1.0 🍒 1.0.0 Cherry-picked to 1.0.0 🍒 1.0.1 Cherry-picked to 1.0.1 🚢 1.2.0 First shipped in 1.2.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants