You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2022/07/05 11:04:49 UTC

[GitHub] [superset] kgabryje opened a new pull request, #20606: Feat/react router from dashboard

kgabryje opened a new pull request, #20606:
URL: https://github.com/apache/superset/pull/20606

   ### SUMMARY
   BLOCKED BY https://github.com/apache/superset/pull/20572
   
   Currently, when user clicks on chart title or "Edit chart" button in Dashboard view, we first make a POST request to get form_data_key and then open Explore in a new window. In order to fully benefit from Explore SPA project, we need to stay in the SPA context when transitioning from Dashboard to Explore - which means that we should open Explore in the same tab.
   
   This PR changes the default behaviour to open Explore in the same tab using the React Router. If user wants to open Explore in separate tab, they can do cmd+click (on Mac) or ctrl+click (on Linux/Windows). The tooltip was adjusted to make the feature easier to discover. 
   
   Why can't we just make the chart title a link and let user "right click -> open in a new tab"? When user clicks "Edit chart", we first need to send an API request to get the form_data_key, which is required to correctly pass dashboard context to Explore.   For this reason we can't use standard HTML <a> element and instead we use a button with `onClick` callback.
   In the future, we'll work on improving this behaviour, so that we can use a link to transition to Explore
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   
   https://user-images.githubusercontent.com/15073128/177313748-4b63608e-d761-4ef1-904c-bb4733d73648.mov
   
   
   ### TESTING INSTRUCTIONS
   
   1. Go to dashboard
   2. Click on chart title or "Edit chart"
   3. Verify that Explore opens in the same tab
   4. Go back to Dashboard
   5. Click on chart title while holding CMD
   6. Verify that Explore opens in new tab
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [ ] Changes UI
   - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] 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
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] kgabryje commented on a diff in pull request #20606: feat(dashboard): Transition to Explore with React Router

Posted by GitBox <gi...@apache.org>.
kgabryje commented on code in PR #20606:
URL: https://github.com/apache/superset/pull/20606#discussion_r915097079


##########
superset/config.py:
##########
@@ -429,6 +429,7 @@ def _try_json_readsha(filepath: str, length: int) -> Optional[str]:
     "GENERIC_CHART_AXES": False,
     "ALLOW_ADHOC_SUBQUERY": False,
     "USE_ANALAGOUS_COLORS": True,
+    "DASHBOARD_EDIT_CHART_IN_TAB": True,

Review Comment:
   And I understood that as `edit in SAME tab` 😆 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] rusackas commented on a diff in pull request #20606: feat(dashboard): Transition to Explore with React Router

Posted by GitBox <gi...@apache.org>.
rusackas commented on code in PR #20606:
URL: https://github.com/apache/superset/pull/20606#discussion_r914919817


##########
superset-frontend/src/dashboard/components/SliceHeader/index.tsx:
##########
@@ -54,6 +62,28 @@ const CrossFilterIcon = styled(Icons.CursorTarget)`
   width: 22px;
 `;
 
+export const getSliceHeaderTooltip = (sliceName: string | undefined) => {
+  if (!isFeatureEnabled(FeatureFlag.DASHBOARD_TO_EXPLORE_SPA)) {
+    return sliceName
+      ? t('Click to edit %s in ', sliceName)

Review Comment:
   ```suggestion
         ? t('Click to edit %s in ', sliceName)
   ```
   Is this missing a word? Edit {sliceName} in...?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] kgabryje merged pull request #20606: feat(dashboard): Transition to Explore with React Router

Posted by GitBox <gi...@apache.org>.
kgabryje merged PR #20606:
URL: https://github.com/apache/superset/pull/20606


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] codecov[bot] commented on pull request #20606: feat(dashboard): Transition to Explore with React Router

Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #20606:
URL: https://github.com/apache/superset/pull/20606#issuecomment-1176103373

   # [Codecov](https://codecov.io/gh/apache/superset/pull/20606?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#20606](https://codecov.io/gh/apache/superset/pull/20606?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d60f0bd) into [master](https://codecov.io/gh/apache/superset/commit/f38dd1d42d2bb1da563367e4d054fe7eaa99eb04?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f38dd1d) will **decrease** coverage by `11.98%`.
   > The diff coverage is `100.00%`.
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #20606       +/-   ##
   ===========================================
   - Coverage   66.82%   54.83%   -11.99%     
   ===========================================
     Files        1752     1752               
     Lines       65616    65616               
     Branches     6938     6938               
   ===========================================
   - Hits        43849    35983     -7866     
   - Misses      20007    27873     +7866     
     Partials     1760     1760               
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | hive | `?` | |
   | mysql | `?` | |
   | postgres | `?` | |
   | presto | `53.68% <ø> (ø)` | |
   | python | `58.04% <ø> (-24.85%)` | :arrow_down: |
   | sqlite | `?` | |
   | unit | `50.67% <ø> (ø)` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/20606?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ackages/superset-ui-core/src/utils/featureFlags.ts](https://codecov.io/gh/apache/superset/pull/20606/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY29yZS9zcmMvdXRpbHMvZmVhdHVyZUZsYWdzLnRz) | `100.00% <ø> (ø)` | |
   | [...dashboard/components/SliceHeaderControls/index.tsx](https://codecov.io/gh/apache/superset/pull/20606/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1NsaWNlSGVhZGVyQ29udHJvbHMvaW5kZXgudHN4) | `64.28% <ø> (ø)` | |
   | [superset/config.py](https://codecov.io/gh/apache/superset/pull/20606/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29uZmlnLnB5) | `91.14% <ø> (-0.33%)` | :arrow_down: |
   | [...end/src/dashboard/components/SliceHeader/index.tsx](https://codecov.io/gh/apache/superset/pull/20606/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1NsaWNlSGVhZGVyL2luZGV4LnRzeA==) | `86.27% <100.00%> (ø)` | |
   | [.../src/dashboard/components/gridComponents/Chart.jsx](https://codecov.io/gh/apache/superset/pull/20606/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0NoYXJ0LmpzeA==) | `58.82% <100.00%> (ø)` | |
   | [superset/utils/dashboard\_import\_export.py](https://codecov.io/gh/apache/superset/pull/20606/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdXRpbHMvZGFzaGJvYXJkX2ltcG9ydF9leHBvcnQucHk=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [superset/key\_value/commands/upsert.py](https://codecov.io/gh/apache/superset/pull/20606/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL3Vwc2VydC5weQ==) | `0.00% <0.00%> (-89.59%)` | :arrow_down: |
   | [superset/key\_value/commands/update.py](https://codecov.io/gh/apache/superset/pull/20606/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `0.00% <0.00%> (-89.37%)` | :arrow_down: |
   | [superset/key\_value/commands/delete.py](https://codecov.io/gh/apache/superset/pull/20606/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL2RlbGV0ZS5weQ==) | `0.00% <0.00%> (-85.30%)` | :arrow_down: |
   | [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/20606/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.19%)` | :arrow_down: |
   | ... and [285 more](https://codecov.io/gh/apache/superset/pull/20606/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/20606?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/20606?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [f38dd1d...d60f0bd](https://codecov.io/gh/apache/superset/pull/20606?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] kgabryje commented on pull request #20606: feat(dashboard): Transition to Explore with React Router

Posted by GitBox <gi...@apache.org>.
kgabryje commented on PR #20606:
URL: https://github.com/apache/superset/pull/20606#issuecomment-1176292597

   > I think `DASHBOARD_EDIT_CHART_IN_TAB` or some version of it is more descriptive than `DASHBOARD_TO_EXPLORE_SPA`. Especially because in the future it will be a single application and SPA references won't make any sense (native filters? 😆)
   
   Makes sense! 
   
   > When opening in the same tab, we can pass the `form_data` using React Router state and avoid the extra GET in Explore. We should still execute the POST to support Explore refreshes.
   
   I intend to make that change while implementing `Reuse data from dashboard if available`. This PR is a prerequisite.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] ktmud commented on pull request #20606: feat(dashboard): Transition to Explore with React Router

Posted by GitBox <gi...@apache.org>.
ktmud commented on PR #20606:
URL: https://github.com/apache/superset/pull/20606#issuecomment-1175233358

   Flagging https://github.com/apache/superset/issues/20381 as this is related. I agree we should use a regular link for transition. The form data retrieval (and subsequent redirects) should happen on the Explore page, not dashboard page.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] michael-s-molina commented on pull request #20606: feat(dashboard): Transition to Explore with React Router

Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on PR #20606:
URL: https://github.com/apache/superset/pull/20606#issuecomment-1176254293

   The difference in loading time is better than we expected! Some first-pass comments:
   
   I think `DASHBOARD_EDIT_CHART_IN_TAB` or some version of it is more descriptive than `DASHBOARD_TO_EXPLORE_SPA`. Especially because in the future it will be a single application and SPA references won't make any sense (native filters? 😆)
   
   When opening in the same tab, we can pass the `form_data` using React Router state and avoid the extra GET in Explore. We should still execute the POST to support Explore refreshes.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] michael-s-molina commented on a diff in pull request #20606: feat(dashboard): Transition to Explore with React Router

Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on code in PR #20606:
URL: https://github.com/apache/superset/pull/20606#discussion_r915043562


##########
superset/config.py:
##########
@@ -429,6 +429,7 @@ def _try_json_readsha(filepath: str, length: int) -> Optional[str]:
     "GENERIC_CHART_AXES": False,
     "ALLOW_ADHOC_SUBQUERY": False,
     "USE_ANALAGOUS_COLORS": True,
+    "DASHBOARD_EDIT_CHART_IN_TAB": True,

Review Comment:
   I can understand the confusion here 😆. I meant `DASHBOARD_EDIT_CHART_IN_NEW_TAB` (added NEW) and this should be `false` by default.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] rusackas commented on a diff in pull request #20606: feat(dashboard): Transition to Explore with React Router

Posted by GitBox <gi...@apache.org>.
rusackas commented on code in PR #20606:
URL: https://github.com/apache/superset/pull/20606#discussion_r914919817


##########
superset-frontend/src/dashboard/components/SliceHeader/index.tsx:
##########
@@ -54,6 +62,28 @@ const CrossFilterIcon = styled(Icons.CursorTarget)`
   width: 22px;
 `;
 
+export const getSliceHeaderTooltip = (sliceName: string | undefined) => {
+  if (!isFeatureEnabled(FeatureFlag.DASHBOARD_TO_EXPLORE_SPA)) {
+    return sliceName
+      ? t('Click to edit %s in ', sliceName)

Review Comment:
   ```js
         ? t('Click to edit %s in ', sliceName)
   ```
   Is this missing a word? Edit {sliceName} in...?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] michael-s-molina commented on pull request #20606: feat(dashboard): Transition to Explore with React Router

Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on PR #20606:
URL: https://github.com/apache/superset/pull/20606#issuecomment-1176538159

   Can you also add something to `UPDATING.md`?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] kgabryje commented on pull request #20606: feat(dashboard): Transition to Explore with React Router

Posted by GitBox <gi...@apache.org>.
kgabryje commented on PR #20606:
URL: https://github.com/apache/superset/pull/20606#issuecomment-1176074521

   > Flagging #20381 as this is related. The form data retrieval (and subsequent redirects) should happen on the Explore page, not dashboard page.
   
   I think #20381 is related to permissions, this PR is part of the Explore SPA project.
   
   > The form data retrieval (and subsequent redirects) should happen on the Explore page, not dashboard page.
   
   Form data retrieval does happen on Explore. What we do on dashboard is post form data with native filters, color scheme and other data related to dashboard and retrieve a form_data_key. We need to figure out a way to skip that step in order to be able to use a link.
   
   Since this change in flow (open Explore in the same tab by default) might be disruptive, I put it behind a new feature flag `DASHBOARD_TO_EXPLORE_SPA`, which is enabled by default.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] kgabryje commented on a diff in pull request #20606: feat(dashboard): Transition to Explore with React Router

Posted by GitBox <gi...@apache.org>.
kgabryje commented on code in PR #20606:
URL: https://github.com/apache/superset/pull/20606#discussion_r915112329


##########
superset/config.py:
##########
@@ -429,6 +429,7 @@ def _try_json_readsha(filepath: str, length: int) -> Optional[str]:
     "GENERIC_CHART_AXES": False,
     "ALLOW_ADHOC_SUBQUERY": False,
     "USE_ANALAGOUS_COLORS": True,
+    "DASHBOARD_EDIT_CHART_IN_TAB": True,

Review Comment:
   Updated, also updated PR description



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] michael-s-molina commented on a diff in pull request #20606: feat(dashboard): Transition to Explore with React Router

Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on code in PR #20606:
URL: https://github.com/apache/superset/pull/20606#discussion_r915098349


##########
superset/config.py:
##########
@@ -429,6 +429,7 @@ def _try_json_readsha(filepath: str, length: int) -> Optional[str]:
     "GENERIC_CHART_AXES": False,
     "ALLOW_ADHOC_SUBQUERY": False,
     "USE_ANALAGOUS_COLORS": True,
+    "DASHBOARD_EDIT_CHART_IN_TAB": True,

Review Comment:
   My bad 😆 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] kgabryje commented on a diff in pull request #20606: feat(dashboard): Transition to Explore with React Router

Posted by GitBox <gi...@apache.org>.
kgabryje commented on code in PR #20606:
URL: https://github.com/apache/superset/pull/20606#discussion_r914931978


##########
superset-frontend/src/dashboard/components/SliceHeader/index.tsx:
##########
@@ -54,6 +62,28 @@ const CrossFilterIcon = styled(Icons.CursorTarget)`
   width: 22px;
 `;
 
+export const getSliceHeaderTooltip = (sliceName: string | undefined) => {
+  if (!isFeatureEnabled(FeatureFlag.DASHBOARD_TO_EXPLORE_SPA)) {
+    return sliceName
+      ? t('Click to edit %s in ', sliceName)

Review Comment:
   ...in a new tab. Thanks for spotting!



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] kgabryje commented on pull request #20606: feat(dashboard): Transition to Explore with React Router

Posted by GitBox <gi...@apache.org>.
kgabryje commented on PR #20606:
URL: https://github.com/apache/superset/pull/20606#issuecomment-1176327260

   > I misspoke. What I meant is the form data key should be created on the Explore page (and in the Explore page's route if using SPA navigation). Context in the dashboard page should be carried over to Explore via URL params by default and only very large payload will use localStorage if needed (although I doubt it will happen very often).
   > 
   > It's not just permissions, but how making a link depends on a post request has also made it less accessible. The post request adds a sometimes significant delay between users clicking the link to the page opening, which often confuses the users.
   
   Thanks for clarifying. Getting rid of that POST request is the next step for us 👍 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org