test(maximize-chart): Add tests to maximize chart action #14371

simcha90 · 2021-04-27T09:34:25Z

SUMMARY

This is continue PR of:
#13986 <- need merge it first

It add tests to this PR
And add Exit FullScreen button

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Screen.Recording.2021-04-27.at.12.30.31.mov

TEST PLAN

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

� Conflicts: � superset-frontend/src/dashboard/util/getPermissions.ts

codecov · 2021-04-27T10:07:04Z

Codecov Report

Merging #14371 (057f1f2) into master (7ff35df) will increase coverage by 0.04%.
The diff coverage is 66.66%.

@@            Coverage Diff             @@
##           master   #14371      +/-   ##
==========================================
+ Coverage   77.18%   77.22%   +0.04%     
==========================================
  Files         954      954              
  Lines       48134    48198      +64     
  Branches     5985     6004      +19     
==========================================
+ Hits        37151    37220      +69     
+ Misses      10786    10781       -5     
  Partials      197      197

Flag	Coverage Δ
javascript	`72.20% <66.66%> (+0.10%)`	⬆️

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

Impacted Files	Coverage Δ
...dashboard/components/SliceHeaderControls/index.jsx	`73.26% <50.00%> (+0.07%)`	⬆️
...ashboard/components/gridComponents/ChartHolder.jsx	`76.82% <100.00%> (+7.31%)`	⬆️
...d/src/dashboard/components/FiltersBadge/Styles.tsx	`54.05% <0.00%> (-43.25%)`	⬇️
superset-frontend/src/dataMask/actions.ts	`72.72% <0.00%> (-9.10%)`	⬇️
superset-frontend/src/dataMask/reducer.ts	`91.30% <0.00%> (-8.70%)`	⬇️
...nd/src/dashboard/util/activeAllDashboardFilters.ts	`92.85% <0.00%> (-7.15%)`	⬇️
...onfigModal/FiltersConfigForm/FiltersConfigForm.tsx	`90.43% <0.00%> (-6.12%)`	⬇️
...et-frontend/src/dashboard/components/SaveModal.tsx	`45.45% <0.00%> (-1.72%)`	⬇️
...src/dashboard/components/FiltersBadge/selectors.ts	`76.92% <0.00%> (-1.65%)`	⬇️
...erset-frontend/src/SqlLab/components/SqlEditor.jsx	`54.62% <0.00%> (ø)`
... and 25 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 7ff35df...057f1f2. Read the comment docs.

superset-frontend/src/dashboard/components/SliceHeaderControls/index.jsx

superset-frontend/src/dashboard/components/gridComponents/ChartHolder.test.tsx

michael-s-molina · 2021-04-27T12:01:31Z

superset-frontend/src/dashboard/components/gridComponents/ChartHolder.test.tsx

+  it('toggle full size', async () => {
+    renderWrapper();
+    // @ts-ignore
+    let chart = screen.getByTestId('slice-container')?.firstChild?.style;


You can use the ! (exclamation) to indicate to typescript that chart will not be null. So you don't need to add the ? (question mark) every time chart is involved.

let chart = screen.getByTestId('slice-container')?.firstChild?.style!; <-- exclamation here

I think it would also be nice to rename chart to chartStyle.

converted it to HTMLElement to get style property for TS

michael-s-molina

LGTM

junlincc · 2021-04-27T15:36:06Z

@simcha90 thank you for adding the test, we can merge as soon as #13986 pass CI

* master: (38 commits) refactor(native-filters): allow cascading only for filter_select (apache#14441) test(maximize-chart): Add tests to maximize chart action (apache#14371) fix: fixing mysql error message (apache#14416) feat: Logic added to limiting factor column in Query model (apache#13521) change relationship (apache#14435) fix: bootstrap data permissions (apache#14348) fix: parse simple string error message values (apache#14360) chore: add stack trace to all calls of logger.error (apache#14382) update README with new docs and recordings (apache#14432) Renamed impyla from implya in impala.mdx and Renamed PIP package impyla from impala in index.mdx (apache#14425) fix(native-filters): fix filter scope error (apache#14426) feat: Adding limiting_factor column to Query model (apache#14234) feat: Add etag caching to dashboard APIs (apache#14357) chore: Moves Card to the components folder (apache#14139) feat: Dynamic imports for the Icons component (apache#14318) feat: Support env vars configuration for WebSocket server (apache#14398) fix: SQLLab role permissions (apache#14372) fix(native-filters): always show filters without dataset (apache#14409) fix error getting partitionQuery from table.partition (apache#14369) refactor: Boostrap to AntD - Tabs (apache#14048) ...

* fixed FullSize charts broken #13600 * Update ChartHolder.jsx * fix:fix get permission function * test: adding tests to full screen * lint: fix lint * fix: fix CR notes * fix: fix CR notes * fix: fix CR notes Co-authored-by: toop <mytoop@163.com> (cherry picked from commit 1f8de1d)

* fixed FullSize charts broken #13600 * Update ChartHolder.jsx * fix:fix get permission function * test: adding tests to full screen * lint: fix lint * fix: fix CR notes * fix: fix CR notes * fix: fix CR notes Co-authored-by: toop <mytoop@163.com>

* fixed FullSize charts broken apache#13600 * Update ChartHolder.jsx * fix:fix get permission function * test: adding tests to full screen * lint: fix lint * fix: fix CR notes * fix: fix CR notes * fix: fix CR notes Co-authored-by: toop <mytoop@163.com> (cherry picked from commit 1f8de1d)

* fixed FullSize charts broken apache#13600 * Update ChartHolder.jsx * fix:fix get permission function * test: adding tests to full screen * lint: fix lint * fix: fix CR notes * fix: fix CR notes * fix: fix CR notes Co-authored-by: toop <mytoop@163.com>

toop and others added 9 commits April 7, 2021 15:47

fixed FullSize charts broken apache#13600

eb9af28

Update ChartHolder.jsx

bd4b793

fix:fix get permission function

6eb04b1

Merge branch 'master' of github.com:apache/superset

5e9a1d8

� Conflicts: � superset-frontend/src/dashboard/util/getPermissions.ts

Merge branch 'master' of github.com:apache/superset

2c5800a

Merge branch 'master' of github.com:apache/superset

ea9aa8b

Merge branch 'toop' of github.com:toop/superset into fix_maximize

eecb9a6

Merge branch 'master' of github.com:apache/superset into fix_maximize

d819567

test: adding tests to full screen

bd1e03d

pull-request-size bot added the size/L label Apr 27, 2021

lint: fix lint

73090a1

michael-s-molina reviewed Apr 27, 2021

View reviewed changes

amitmiran137 changed the title ~~test(native-filters): Add tests to maximize chart action~~ test(maximize-chart): Add tests to maximize chart action Apr 27, 2021

simcha90 added 3 commits April 27, 2021 16:12

fix: fix CR notes

5380f90

fix: fix CR notes

32fe58e

fix: fix CR notes

057f1f2

michael-s-molina approved these changes Apr 27, 2021

View reviewed changes

junlincc requested a review from zhaoyongjie April 27, 2021 15:36

junlincc added the dashboard Namespace | Anything related to the Dashboard label Apr 27, 2021

amitmiran137 added the v1.2 label May 2, 2021

amitmiran137 approved these changes May 2, 2021

View reviewed changes

amitmiran137 merged commit 1f8de1d into apache:master May 2, 2021

amitmiran137 mentioned this pull request May 6, 2021

[chart]multiple charts broken on maximizing #13600

Closed

mistercrunch added 🍒 1.2.0 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.3.0 labels Mar 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test(maximize-chart): Add tests to maximize chart action #14371

test(maximize-chart): Add tests to maximize chart action #14371

simcha90 commented Apr 27, 2021 •

edited

Loading

codecov bot commented Apr 27, 2021 •

edited

Loading

michael-s-molina Apr 27, 2021

simcha90 Apr 27, 2021

michael-s-molina left a comment

junlincc commented Apr 27, 2021

test(maximize-chart): Add tests to maximize chart action #14371

test(maximize-chart): Add tests to maximize chart action #14371

Conversation

simcha90 commented Apr 27, 2021 • edited Loading

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TEST PLAN

ADDITIONAL INFORMATION

codecov bot commented Apr 27, 2021 • edited Loading

Codecov Report

michael-s-molina Apr 27, 2021

Choose a reason for hiding this comment

simcha90 Apr 27, 2021

Choose a reason for hiding this comment

michael-s-molina left a comment

Choose a reason for hiding this comment

junlincc commented Apr 27, 2021

simcha90 commented Apr 27, 2021 •

edited

Loading

codecov bot commented Apr 27, 2021 •

edited

Loading