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(dashboard): Empty states overflowing small chart containers #19095

Merged
merged 2 commits into from
Mar 10, 2022

Conversation

kgabryje
Copy link
Member

SUMMARY

When query returned no results, we were showing a large empty state component in chart container on the dashboard, regardless of container's size. If the container was small, the empty state was overflowing. This PR fixes that behaviour by replacing large empty state with a small one if chart's width is smaller than 300px or if chart's height is smaller than 220px.
Also, I removed chart empty state's description if it's displayed on the dashboard - the description will be displayed only on Explore page.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before:

image

After:

image

TESTING INSTRUCTIONS

  1. Create a dashboard with large and small charts
  2. Add a filter that filters out all results
  3. Verify that the empty state is not overflowing in any chart
  4. Verify that the empty state has only image and title
  5. Click "View chart in Explore"
  6. Verify that empty state has an image, title and description

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

CC @kasiazjc

Copy link
Member

@geido geido left a comment

Choose a reason for hiding this comment

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

Code LGTM. Waiting for CI to pass for a test env

@kgabryje
Copy link
Member Author

/testenv up

@kgabryje
Copy link
Member Author

Code LGTM. Waiting for CI to pass for a test env

I started a test env. I need to correct a test, but that won't change the functionality in any way.

@github-actions
Copy link
Contributor

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

@codecov
Copy link

codecov bot commented Mar 10, 2022

Codecov Report

Merging #19095 (4e9f37f) into master (79a7a5d) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #19095   +/-   ##
=======================================
  Coverage   66.51%   66.51%           
=======================================
  Files        1643     1645    +2     
  Lines       63462    63502   +40     
  Branches     6449     6462   +13     
=======================================
+ Hits        42210    42238   +28     
- Misses      19582    19590    +8     
- Partials     1670     1674    +4     
Flag Coverage Δ
javascript 51.26% <100.00%> (+0.02%) ⬆️

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

Impacted Files Coverage Δ
superset-frontend/src/components/Chart/Chart.jsx 54.09% <100.00%> (+0.76%) ⬆️
...et-frontend/src/components/Chart/ChartRenderer.jsx 52.63% <100.00%> (+7.73%) ⬆️
...s/legacy-preset-chart-nvd3/src/Bar/controlPanel.ts 16.66% <0.00%> (-83.34%) ⬇️
...ns/plugin-chart-echarts/src/Radar/controlPanel.tsx 30.00% <0.00%> (-3.34%) ⬇️
...src/dashboard/components/FiltersBadge/selectors.ts 68.46% <0.00%> (-2.18%) ⬇️
...eFilters/FiltersConfigModal/FiltersConfigModal.tsx 63.04% <0.00%> (-1.64%) ⬇️
...nd/plugins/plugin-chart-table/src/controlPanel.tsx 16.17% <0.00%> (-0.50%) ⬇️
...frontend/src/SqlLab/components/ResultSet/index.tsx 50.49% <0.00%> (-0.25%) ⬇️
...erset-frontend/src/components/EmptyState/index.tsx 61.53% <0.00%> (ø)
...et-frontend/src/components/TableView/TableView.tsx 84.31% <0.00%> (ø)
... and 16 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 79a7a5d...4e9f37f. Read the comment docs.

@kgabryje kgabryje merged commit 70081a6 into apache:master Mar 10, 2022
@github-actions
Copy link
Contributor

Ephemeral environment shutdown and build artifacts deleted.

villebro pushed a commit that referenced this pull request Apr 3, 2022
* fix(dashboard): Empty states overflowing small chart containers

* Fix test

(cherry picked from commit 70081a6)
@mistercrunch mistercrunch added 🍒 1.5.0 🍒 1.5.1 🍒 1.5.2 🍒 1.5.3 🏷️ 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
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 lts-v1 size/M 🍒 1.5.0 🍒 1.5.1 🍒 1.5.2 🍒 1.5.3 🚢 2.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants