fix(dashboard): fixed FullSize charts broken

[toop]

SUMMARY

When you click the “Maximize chart” option of the chart in the dashboard, the chart is incomplete。

BEFORE:

AFTER:

ADDITIONAL INFORMATION

• Has associated issue:
• 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
  @junlincc @zhaoyongjie

[codecov]

Codecov Report

Merging #13986 (095c0fb) into master (a82d72f) will increase coverage by 0.08%.
The diff coverage is 70.00%.

❗ Current head 095c0fb differs from pull request most recent head 1d40d3c. Consider uploading reports for the commit 1d40d3c to get more accurate results

@@            Coverage Diff             @@
##           master   #13986      +/-   ##
==========================================
+ Coverage   79.12%   79.21%   +0.08%     
==========================================
  Files         936      936              
  Lines       47411    47408       -3     
  Branches     5939     5940       +1     
==========================================
+ Hits        37516    37553      +37     
+ Misses       9768     9728      -40     
  Partials      127      127              

Flag	Coverage Δ
cypress	56.04% <62.50%> (+<0.01%)	⬆️

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

Impacted Files	Coverage Δ
...ashboard/components/gridComponents/ChartHolder.jsx	80.48% <0.00%> (ø)
...src/explore/components/controls/VizTypeControl.jsx	92.06% <83.33%> (+1.29%)	⬆️
...tend/src/views/CRUD/annotation/AnnotationModal.tsx	62.96% <100.00%> (-0.28%)	⬇️
superset/models/core.py	89.37% <0.00%> (+0.27%)	⬆️
superset/connectors/sqla/models.py	90.76% <0.00%> (+1.43%)	⬆️
superset/db_engine_specs/presto.py	88.30% <0.00%> (+5.42%)	⬆️

---

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 a82d72f...1d40d3c. Read the comment docs.

[zhaoyongjie]

LGTM, thanks for this fix.

before screen recording

zoom.in.out.in.dashboard.before.mov

after screen recording

zoom.in.out.in.dashboard.after.mov

[toop]

please delete the comments, thanks! 在 2021-04-07 17:54:52，"Yongjie Zhao" ***@***.***> 写道： @zhaoyongjie commented on this pull request. In superset-frontend/src/dashboard/components/gridComponents/ChartHolder.jsx:

@@ -245,8 +245,9 @@ class ChartHolder extends React.Component {

let chartHeight = 0; if (this.state.isFullSize) { - chartWidth = document.body.clientWidth - CHART_MARGIN; - chartHeight = document.body.clientHeight - CHART_MARGIN; + //toop fixed FullSize charts broken #13600 nit, please do not stay irrelated comments, thanks — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

[zhaoyongjie]

LGTM

[etr2460]

since this was a regression, is there any way we could add a test for it? If not, lgtm, but ideally we could prevent this happening again (since this isn't a commonly tested feature)

[junlincc]

@toop please do add test. 🙏

[zhaoyongjie]

Hi there, why we use window.screen.width instead of window.innerWidth(previous implementation for you). The window.screen.width looks like it always gets the whole screen width instead of the browser width.

Thanks again for your contribution!

[toop]

I 'm sorry! It's not thoughtful. I change it back

[zhaoyongjie]

@toop Could you fix the CI? thanks