Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: [explore][mixed time series chart] when user change size of view query window, query B part will disappear #20750

Conversation

diegomedina248
Copy link
Contributor

SUMMARY

The mixed time series has more than one query.
When opening the "View Query" modal and resizing it, the second query dissapears.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before:

Screen.Recording.2022-07-18.at.13.26.45.mov

After:

new.mov

TESTING INSTRUCTIONS

  1. create new chart use vehicle sales data for mix time-series chart
  2. select sum(sales) for metrics in query A, Max(sales) for query B
  3. run query
  4. go to view query, change the view query window to smaller size from bottom, then change it back to origin size

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 Jul 18, 2022

Codecov Report

Merging #20750 (84d4302) into master (af1bddf) will decrease coverage by 0.00%.
The diff coverage is n/a.

❗ Current head 84d4302 differs from pull request most recent head 318df38. Consider uploading reports for the commit 318df38 to get more accurate results

@@            Coverage Diff             @@
##           master   #20750      +/-   ##
==========================================
- Coverage   66.30%   66.30%   -0.01%     
==========================================
  Files        1758     1756       -2     
  Lines       66802    66734      -68     
  Branches     7056     7049       -7     
==========================================
- Hits        44294    44248      -46     
+ Misses      20710    20689      -21     
+ Partials     1798     1797       -1     
Flag Coverage Δ
hive 53.27% <0.00%> (+0.03%) ⬆️
javascript 51.93% <0.00%> (-0.05%) ⬇️
mysql 81.04% <0.00%> (+0.05%) ⬆️
postgres 81.11% <0.00%> (+0.05%) ⬆️
presto 53.13% <0.00%> (+0.03%) ⬆️
python 81.55% <0.00%> (+0.05%) ⬆️
sqlite 79.60% <0.00%> (+0.05%) ⬆️
unit 50.27% <0.00%> (+0.03%) ⬆️

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

Impacted Files Coverage Δ
...ges/superset-ui-core/src/query/types/Datasource.ts 66.66% <0.00%> (-33.34%) ⬇️
...set-frontend/src/hooks/useElementOnScreen/index.ts 57.14% <0.00%> (-28.58%) ⬇️
...ckages/superset-ui-core/src/query/DatasourceKey.ts 83.33% <0.00%> (-16.67%) ⬇️
...ols/DndColumnSelectControl/utils/optionSelector.ts 24.13% <0.00%> (-13.80%) ⬇️
...set-frontend/src/explore/actions/exploreActions.ts 47.72% <0.00%> (-10.61%) ⬇️
.../src/dashboard/components/menu/WithPopoverMenu.tsx 61.53% <0.00%> (-5.13%) ⬇️
...d/components/DashboardBuilder/DashboardBuilder.tsx 70.10% <0.00%> (-4.90%) ⬇️
...src/dashboard/components/DashboardBuilder/state.ts 70.00% <0.00%> (-3.34%) ⬇️
.../controls/DndColumnSelectControl/OptionWrapper.tsx 63.41% <0.00%> (-2.44%) ⬇️
...-frontend/src/dashboard/reducers/dashboardState.js 72.34% <0.00%> (-2.13%) ⬇️
... and 40 more

Help us with your feedback. Take ten seconds to tell us how you rate us.

Copy link
Member

@stephenLYZ stephenLYZ left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for the fix.

@diegomedina248 diegomedina248 force-pushed the fix/query-window-size-for-mixed-series branch 2 times, most recently from 4627ac7 to 22b03cd Compare July 19, 2022 17:43
@diegomedina248 diegomedina248 force-pushed the fix/query-window-size-for-mixed-series branch from 22b03cd to 318df38 Compare July 22, 2022 01:22
@zhaoyongjie zhaoyongjie merged commit 6e0ddcf into apache:master Jul 22, 2022
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 2.1.0 and removed 🚢 2.1.3 labels Mar 13, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/S 🚢 2.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants