refactor: Renders Explore in SPA #20572

michael-s-molina · 2022-06-30T18:59:52Z

SUMMARY

Renders Explore in SPA under /explore. Previously, Explore was displayed under /superset/explore. This will open a number of possibilities for in-context features and also reduce the development complexity associated with keeping multiple React applications.

This PR also replaces SPA-related Explore navigation with React Router and removes the previous entry point files.

Follow-up of #20519

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Screen.Recording.2022-07-01.at.4.00.25.PM.mov

TESTING INSTRUCTIONS

We need to test all features that are associated with Explore, like:

Navigate from multiple places to Explore (welcome page, charts list, add a chart, dashboard, etc)
Refresh the Explore page and check if the content persists
Check permalinks
Check Explore save flow

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 · 2022-07-01T13:06:00Z

Codecov Report

Merging #20572 (28feee5) into master (b30f6a5) will increase coverage by 0.01%.
The diff coverage is 47.45%.

❗ Current head 28feee5 differs from pull request most recent head eaeaa82. Consider uploading reports for the commit eaeaa82 to get more accurate results

@@            Coverage Diff             @@
##           master   #20572      +/-   ##
==========================================
+ Coverage   66.80%   66.81%   +0.01%     
==========================================
  Files        1754     1751       -3     
  Lines       65566    65553      -13     
  Branches     6933     6935       +2     
==========================================
- Hits        43802    43801       -1     
+ Misses      20010    19995      -15     
- Partials     1754     1757       +3

Flag	Coverage Δ
javascript	`51.85% <39.21%> (+0.03%)`	⬆️

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

Impacted Files	Coverage Δ
...t-ui-chart-controls/src/operators/pivotOperator.ts	`100.00% <ø> (ø)`
...controls/src/operators/timeComparePivotOperator.ts	`100.00% <ø> (ø)`
...t-controls/src/sections/echartsTimeSeriesQuery.tsx	`50.00% <ø> (ø)`
...superset-ui-core/src/query/types/PostProcessing.ts	`100.00% <ø> (ø)`
...set-ui-core/src/ui-overrides/UiOverrideRegistry.ts	`100.00% <ø> (ø)`
...egacy-preset-chart-deckgl/src/layers/Grid/Grid.jsx	`0.00% <0.00%> (ø)`
...reset-chart-deckgl/src/layers/Grid/controlPanel.ts	`50.00% <0.00%> (-50.00%)`	⬇️
.../legacy-preset-chart-deckgl/src/layers/Hex/Hex.jsx	`0.00% <0.00%> (ø)`
...preset-chart-deckgl/src/layers/Hex/controlPanel.ts	`50.00% <0.00%> (-50.00%)`	⬇️
...et-chart-deckgl/src/layers/Polygon/controlPanel.ts	`33.33% <0.00%> (-16.67%)`	⬇️
... and 33 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 b30f6a5...eaeaa82. Read the comment docs.

superset-frontend/src/explore/exploreUtils/index.js

EugeneTorap · 2022-07-01T20:35:44Z

@michael-s-molina Thank you very much for this long awaited PR
Do you plan to make switching between Home, Dashboards, Charts, SQL Editor and Data tabs in SPA without reloading the browser page?

kgabryje · 2022-07-04T20:43:42Z

superset/models/slice.py

@@ -286,14 +286,14 @@ def get_query_context(self) -> Optional[QueryContext]:

    def get_explore_url(
        self,
-        base_url: str = "/superset/explore",
+        base_url: str = "/explore",
        overrides: Optional[Dict[str, Any]] = None,
    ) -> str:
        overrides = overrides or {}
        form_data = {"slice_id": self.id}
        form_data.update(overrides)


Not sure if overrides will be recognized by the new explore endpoint, since it doesn't handle form_data param. We might want to look into that later and fix/refactor if needed

Thank you for spotting that! I investigated and it's trickier than it sounds. It's being used by Top N charts cache warm-up. I'll tackle this problem in a separate PR because it's not working anyway currently with form_data_key.

kgabryje · 2022-07-04T20:47:40Z

We need to replace links to Explore from Datasets List - we still use <a> there. However, I'm fine with doing that in a separate PR, since there's an edge case to consider - dataset can link to an external website instead of explore, and for external websites we can't use React Router links. We probably should first create a generic link component, which would detect if url is internal (if so, use React Router link) or external (use HTML anchor)

kgabryje

Left 2 non-blocking comments. The PR looks great!

zhaoyongjie · 2022-07-05T05:45:26Z

/testenv up

github-actions · 2022-07-05T05:47:12Z

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

zhaoyongjie

@michael-s-molina The results data pane seems lost.

kgabryje · 2022-07-05T07:42:12Z

@michael-s-molina The results data pane seems lost.

Works for me 🤔

zhaoyongjie · 2022-07-05T07:47:24Z

@michael-s-molina The results data pane seems lost.

Works for me 🤔

Could you try to close the Results Pane then reopen this Chart from a new tab?

jinghua-qa · 2022-07-05T08:04:42Z

I have tried open some example charts, i also saw the issue that result/samples are missing, however example chart 'Most Dominant Platforms' is working fine

result.and.sample.mov

jinghua-qa · 2022-07-05T08:15:26Z

Found 2 issue when switching chart:
1, when i switching area chart to pie chart and big number, area chart will non-stop flashing before user update the chart

keep.flashing.mov

2, change chart to pie chart and big number chart through viz switchers, the chart will keep zoom out itself

Screen.Recording.2022-07-05.at.12.35.46.AM.mov

kgabryje · 2022-07-05T08:52:14Z

Thanks for spotting that @zhaoyongjie @jinghua-qa ! The problem was that after using the SPA html template, the structure of the page changed a bit and the height of Explore container wasn't calculated correctly. I pushed a fix for that and confirmed that results pane is now displayed correctly when it's closed.

@jinghua-qa I couldn't repro the problem with flashing/zooming, but it looks to me like it's connected to calculating the page height. Hopefully my latest commit fixes this issue as well

kgabryje · 2022-07-05T10:48:00Z

/testenv up

github-actions · 2022-07-05T10:52:28Z

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

michael-s-molina · 2022-07-05T11:46:46Z

@michael-s-molina Thank you very much for this long awaited PR Do you plan to make switching between Home, Dashboards, Charts, SQL Editor and Data tabs in SPA without reloading the browser page?

Hi @EugeneTorap. That's the plan for the whole SPA project. We're tackling it in phases so some of the pages may still be reloading.

michael-s-molina · 2022-07-05T13:13:09Z

Thanks for spotting that @zhaoyongjie @jinghua-qa ! The problem was that after using the SPA html template, the structure of the page changed a bit and the height of Explore container wasn't calculated correctly. I pushed a fix for that and confirmed that results pane is now displayed correctly when it's closed.

@jinghua-qa I couldn't repro the problem with flashing/zooming, but it looks to me like it's connected to calculating the page height. Hopefully my latest commit fixes this issue as well

Thank you for submitting the fix! I'm just waiting for @zhaoyongjie and @jinghua-qa's approval to merge it.

michael-s-molina · 2022-07-05T13:17:25Z

We need to replace links to Explore from Datasets List - we still use <a> there. However, I'm fine with doing that in a separate PR, since there's an edge case to consider - dataset can link to an external website instead of explore, and for external websites we can't use React Router links. We probably should first create a generic link component, which would detect if url is internal (if so, use React Router link) or external (use HTML anchor)

Good point. Let's tackle it in a separate PR 😉

jinghua-qa

Verified fix for result/sample title and auto zoom out issue, also did not see major issues for regression test from QA team. LGTM!

github-actions · 2022-07-05T19:58:31Z

Ephemeral environment shutdown and build artifacts deleted.

* feat: Renders Explore in SPA * Adds permalink support * Replaces navigation from Welcome page * Fix initializing feature flags * Remove redundant import * Adds saveSlice workaround * Fixes paths * Fixes lint error * Fixes tests * Fix url to explore from Datasets view * Fix explore page height Co-authored-by: Kamil Gabryjelski <[email protected]>

pull-request-size bot added the size/XXL label Jun 30, 2022

feat: Renders Explore in SPA

83535e0

michael-s-molina force-pushed the move-explore-entry-point branch from 24e34fe to 83535e0 Compare June 30, 2022 19:24

pull-request-size bot added size/L and removed size/XXL labels Jun 30, 2022

codyml mentioned this pull request Jul 1, 2022

chore(explore): Update chart save to use API endpoints #20498

Merged

9 tasks

michael-s-molina marked this pull request as ready for review July 1, 2022 11:31

michael-s-molina changed the title ~~feat: Renders Explore in SPA~~ [WIP] feat: Renders Explore in SPA Jul 1, 2022

Adds permalink support

ca9b9cd

kgabryje reviewed Jul 1, 2022

View reviewed changes

superset-frontend/src/explore/exploreUtils/index.js Show resolved Hide resolved

michael-s-molina and others added 7 commits July 1, 2022 10:58

Replaces navigation from Welcome page

e4e572b

Fix initializing feature flags

10d9c54

Remove redundant import

f0777f7

Adds saveSlice workaround

6085a0e

Fixes paths

2982864

Fixes lint error

350ddd9

Fixes tests

49fc045

michael-s-molina requested review from geido, jinghua-qa, rusackas and zhaoyongjie July 1, 2022 19:20

michael-s-molina changed the title ~~[WIP] feat: Renders Explore in SPA~~ feat: Renders Explore in SPA Jul 1, 2022

Fix url to explore from Datasets view

77113a5

kgabryje reviewed Jul 4, 2022

View reviewed changes

kgabryje approved these changes Jul 4, 2022

View reviewed changes

zhaoyongjie reviewed Jul 5, 2022

View reviewed changes

Fix explore page height

eaeaa82

kgabryje mentioned this pull request Jul 5, 2022

feat(dashboard): Transition to Explore with React Router #20606

Merged

9 tasks

michael-s-molina requested a review from zhaoyongjie July 5, 2022 13:29

zhaoyongjie approved these changes Jul 5, 2022

View reviewed changes

jinghua-qa approved these changes Jul 5, 2022

View reviewed changes

michael-s-molina merged commit 662bab1 into apache:master Jul 5, 2022

michael-s-molina mentioned this pull request Jul 7, 2022

refactor: Unify shared datasources reducers and actions #20645

Merged

9 tasks

michael-s-molina changed the title ~~feat: Renders Explore in SPA~~ refactor: Renders Explore in SPA Jul 11, 2022

michael-s-molina mentioned this pull request Jul 21, 2022

fix: Redirects old Explore URLs to the new ones #20812

Merged

9 tasks

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: Renders Explore in SPA #20572

refactor: Renders Explore in SPA #20572

michael-s-molina commented Jun 30, 2022 •

edited

Loading

codecov bot commented Jul 1, 2022 •

edited

Loading

EugeneTorap commented Jul 1, 2022 •

edited

Loading

kgabryje Jul 4, 2022 •

edited

Loading

michael-s-molina Jul 5, 2022

kgabryje commented Jul 4, 2022

kgabryje left a comment

zhaoyongjie commented Jul 5, 2022

github-actions bot commented Jul 5, 2022

zhaoyongjie left a comment

kgabryje commented Jul 5, 2022

zhaoyongjie commented Jul 5, 2022 •

edited

Loading

jinghua-qa commented Jul 5, 2022

jinghua-qa commented Jul 5, 2022

kgabryje commented Jul 5, 2022

kgabryje commented Jul 5, 2022

github-actions bot commented Jul 5, 2022

michael-s-molina commented Jul 5, 2022

michael-s-molina commented Jul 5, 2022

michael-s-molina commented Jul 5, 2022

jinghua-qa left a comment

github-actions bot commented Jul 5, 2022

refactor: Renders Explore in SPA #20572

refactor: Renders Explore in SPA #20572

Conversation

michael-s-molina commented Jun 30, 2022 • edited Loading

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

codecov bot commented Jul 1, 2022 • edited Loading

Codecov Report

EugeneTorap commented Jul 1, 2022 • edited Loading

kgabryje Jul 4, 2022 • edited Loading

Choose a reason for hiding this comment

michael-s-molina Jul 5, 2022

Choose a reason for hiding this comment

kgabryje commented Jul 4, 2022

kgabryje left a comment

Choose a reason for hiding this comment

zhaoyongjie commented Jul 5, 2022

github-actions bot commented Jul 5, 2022

zhaoyongjie left a comment

Choose a reason for hiding this comment

kgabryje commented Jul 5, 2022

zhaoyongjie commented Jul 5, 2022 • edited Loading

jinghua-qa commented Jul 5, 2022

jinghua-qa commented Jul 5, 2022

kgabryje commented Jul 5, 2022

kgabryje commented Jul 5, 2022

github-actions bot commented Jul 5, 2022

michael-s-molina commented Jul 5, 2022

michael-s-molina commented Jul 5, 2022

michael-s-molina commented Jul 5, 2022

jinghua-qa left a comment

Choose a reason for hiding this comment

github-actions bot commented Jul 5, 2022

michael-s-molina commented Jun 30, 2022 •

edited

Loading

codecov bot commented Jul 1, 2022 •

edited

Loading

EugeneTorap commented Jul 1, 2022 •

edited

Loading

kgabryje Jul 4, 2022 •

edited

Loading

zhaoyongjie commented Jul 5, 2022 •

edited

Loading