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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
