feat(explore): Move timer, row counter and cached pills to chart container

[kgabryje]

SUMMARY

This PR is a part of chart header redesign project.
Moves the timer, row counter and cached pills from chart header to chart container.
Final design available in discussion: #19099

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

Verify that timer, row counter and cache pills work like before.

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]

Codecov Report

Merging #19458 (21641ca) into master (602afba) will decrease coverage by 0.01%.
The diff coverage is 58.33%.

❗ Current head 21641ca differs from pull request most recent head c34130e. Consider uploading reports for the commit c34130e to get more accurate results

@@            Coverage Diff             @@
##           master   #19458      +/-   ##
==========================================
- Coverage   66.60%   66.58%   -0.02%     
==========================================
  Files        1678     1678              
  Lines       64246    64242       -4     
  Branches     6539     6539              
==========================================
- Hits        42788    42773      -15     
- Misses      19763    19769       +6     
- Partials     1695     1700       +5     

Flag	Coverage Δ
javascript	51.31% <58.33%> (-0.04%)	⬇️
unit	?

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

Impacted Files	Coverage Δ
...et-frontend/src/components/RowCountLabel/index.tsx	88.88% <ø> (ø)
.../src/explore/components/DataTableControl/index.tsx	71.62% <ø> (ø)
...ntend/src/explore/components/ExploreChartPanel.jsx	72.00% <ø> (ø)
superset-frontend/src/components/Chart/Chart.jsx	52.38% <50.00%> (-0.08%)	⬇️
...erset-frontend/src/components/Chart/ChartPills.tsx	55.55% <55.55%> (ø)
...rc/explore/components/ExploreChartHeader/index.jsx	45.90% <100.00%> (-4.10%)	⬇️
...rontend/src/views/CRUD/dashboard/DashboardList.tsx	55.14% <0.00%> (-2.96%)	⬇️
...rset-frontend/src/components/ListView/ListView.tsx	96.25% <0.00%> (-2.50%)	⬇️
superset/views/alerts.py	77.77% <0.00%> (-2.23%)	⬇️
...rontend/src/views/CRUD/dashboard/DashboardCard.tsx	56.52% <0.00%> (-1.82%)	⬇️
... and 8 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 602afba...c34130e. Read the comment docs.

[zhaoyongjie]

Thanks for the improvement!

I think we shouldn't move those indicators to the Chart Container because the reportor and the chart as image need to look for this DOM element. When we moved those indicators, the cache/rows/timer will display in the screenshot and saved image.

After

move.indicator.to.right-top.mov

Before save as image

[kgabryje]

Good point @zhaoyongjie ! I'll fix that by taking it out of the container that's being grabbed for screenshot 👍

[zhaoyongjie]

Many thanks

On Fri, Apr 1, 2022 at 21:25 Kamil Gabryjelski ***@***.***> wrote: Good point @zhaoyongjie <https://github.com/zhaoyongjie> ! I'll fix that by taking it out of the container that's being grabbed for screenshot 👍 — Reply to this email directly, view it on GitHub <#19458 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAPMKUQP3IGDPQMBESP4U5DVC32LHANCNFSM5SE5456Q> . You are receiving this because you were mentioned.Message ID: ***@***.***>

-- Best regards, Yongjie

[kgabryje]

@zhaoyongjie Fixed!

[zhaoyongjie]

@zhaoyongjie Fixed!

Thanks! I will test it in time.

[zhaoyongjie]

@kgabryje Thanks for follow-up. The codes make sense to me.

@kasiazjc I have one more question do we have necessary to add a new break line after the title of the chart? The new blank lines will make the chart less space, is that what we expect?

This PR

Master Branch

[kgabryje]

@zhaoyongjie We're in progress of implementing redesign for the chart header - that's not the final look. Here you can check out how it's going to look soon: #19099.

[kasiazjc]

@zhaoyongjie We're in progress of implementing redesign for the chart header - that's not the final look. Here you can check out how it's going to look soon: https:/ /github.com//discussions/19099.

@zhaoyongjie ➕ to what Kamil said. We broke it down into smaller tasks, so it may look awkward in some PRs before we get the final designs.

[villebro]

FYI - we're coordinating to ensure that the temporary problem that @zhaoyongjie brought up above will not be released in official releases of Superset (these related cherries will be excluded from 1.5 and 2.0 and will only become available once 2.1 is released)

[zhaoyongjie]

@villebro @kasiazjc @kgabryje Thanks for the explanation! We can discuss this topic on global header board.

[kgabryje]

Updated the PR summary with a link to the final design. I'll include that in upcoming PRs too for context

[kgabryje]

I'm going to hold off merging this PR until a PR that moves the header to the top of the page is ready, so that this change won't appear out-of-context on master

[villebro]

LGTM