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 2020/12/08 02:18:30 UTC

[GitHub] [incubator-superset] ktmud commented on a change in pull request #11687: feat(dashboard): change dashboard edit action

ktmud commented on a change in pull request #11687:
URL: https://github.com/apache/incubator-superset/pull/11687#discussion_r537981196



##########
File path: superset-frontend/src/views/CRUD/dashboard/DashboardCard.tsx
##########
@@ -67,13 +67,11 @@ function DashboardCard({
 
   const menu = (
     <Menu>
-      {canEdit && openDashboardEditModal && (
+      {canEdit && handleDashboardEdit && (
         <Menu.Item
           role="button"
           tabIndex={0}
-          onClick={() =>
-            openDashboardEditModal && openDashboardEditModal(dashboard)
-          }
+          onClick={() => handleDashboardEdit && handleDashboardEdit(dashboard)}

Review comment:
       Opening in new tab is often helpful in context switching actions so users can return to previous page without losing context. It has been proven to be helpful for conversion time and time again on e-commerce websites.
   
   While Superset is a different product, there are places where opening a new window makes sense, too. We should ask ourselves:
   
   1.  Are users likely to go back to the previous page after finishing actions on the page they will open?
   2. Does the previous page load slowly?
   3. Is retaining scrolling position  on previous page important?
   
   If the answer to any of these questions is "Yes", then we should open the link in a new 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.

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