[Bug fix] Wrong behavior when clicking table rows on list pages (#3540)

getredash · Mar 7, 2019 · be1bd28 · be1bd28
1 parent 507ea61
commit be1bd28
Show file tree

Hide file tree

Showing 11 changed files with 31 additions and 99 deletions.
diff --git a/client/app/assets/less/redash/redash-newstyle.less b/client/app/assets/less/redash/redash-newstyle.less
@@ -157,7 +157,6 @@ body {
 .table-data {
   tbody > tr > td {
     padding-top: 5px !important;
-    cursor: pointer;
   }
 
   .btn-favourite, .btn-archive {

diff --git a/client/app/components/FavoritesControl.jsx b/client/app/components/FavoritesControl.jsx
@@ -20,9 +20,6 @@ export class FavoritesControl extends React.Component {
   };
 
   toggleItem(event, item, callback) {
-    event.preventDefault();
-    event.stopPropagation();
-
     const action = item.is_favorite ? item.$unfavorite.bind(item) : item.$favorite.bind(item);
     const savedIsFavorite = item.is_favorite;
 

diff --git a/client/app/components/groups/DeleteGroupButton.jsx b/client/app/components/groups/DeleteGroupButton.jsx
@@ -7,10 +7,6 @@ import Tooltip from 'antd/lib/tooltip';
 import { toastr } from '@/services/ng';
 
 function deleteGroup(event, group, onGroupDeleted) {
-  // prevent default click action on table rows
-  event.preventDefault();
-  event.stopPropagation();
-
   Modal.confirm({
     title: 'Delete Group',
     content: 'Are you sure you want to delete this group?',

diff --git a/client/app/lib/utils.js b/client/app/lib/utils.js
@@ -1,5 +1,6 @@
-import { isFunction, each, extend } from 'lodash';
+import { each, extend } from 'lodash';
 
+// eslint-disable-next-line import/prefer-default-export
 export function routesToAngularRoutes(routes, template) {
   const result = {};
   template = extend({}, template); // convert to object
@@ -22,18 +23,3 @@ export function routesToAngularRoutes(routes, template) {
   });
   return result;
 }
-
-function doCancelEvent(event) {
-  event.stopPropagation();
-  event.preventDefault();
-}
-
-export function cancelEvent(handler) {
-  if (isFunction(handler)) {
-    return (event, ...rest) => {
-      doCancelEvent(event);
-      return handler(...rest);
-    };
-  }
-  return doCancelEvent;
-}
diff --git a/client/app/pages/alerts/AlertsList.jsx b/client/app/pages/alerts/AlertsList.jsx
@@ -14,7 +14,6 @@ import LoadingState from '@/components/items-list/components/LoadingState';
 import ItemsTable, { Columns } from '@/components/items-list/components/ItemsTable';
 
 import { Alert } from '@/services/alert';
-import navigateTo from '@/services/navigateTo';
 import { routesToAngularRoutes } from '@/lib/utils';
 
 const STATE_CLASS = {
@@ -57,8 +56,6 @@ class AlertsList extends React.Component {
       width: '1%' }),
   ];
 
-  onTableRowClick = (event, item) => navigateTo('alerts/' + item.id);
-
   render() {
     const { controller } = this.props;
 
@@ -82,7 +79,6 @@ class AlertsList extends React.Component {
                 <ItemsTable
                   items={controller.pageItems}
                   columns={this.listColumns}
-                  onRowClick={this.onTableRowClick}
                   orderByField={controller.orderByField}
                   orderByReverse={controller.orderByReverse}
                   toggleSorting={controller.toggleSorting}

diff --git a/client/app/pages/dashboards/DashboardList.jsx b/client/app/pages/dashboards/DashboardList.jsx
@@ -16,7 +16,6 @@ import ItemsTable, { Columns } from '@/components/items-list/components/ItemsTab
 import Layout from '@/components/layouts/ContentWithSidebar';
 
 import { Dashboard } from '@/services/dashboard';
-import navigateTo from '@/services/navigateTo';
 import { routesToAngularRoutes } from '@/lib/utils';
 
 import DashboardListEmptyState from './DashboardListEmptyState';
@@ -68,8 +67,6 @@ class DashboardList extends React.Component {
     }),
   ];
 
-  onTableRowClick = (event, item) => navigateTo('dashboard/' + item.slug);
-
   render() {
     const { controller } = this.props;
     return (
@@ -107,7 +104,6 @@ class DashboardList extends React.Component {
                   <ItemsTable
                     items={controller.pageItems}
                     columns={this.listColumns}
-                    onRowClick={this.onTableRowClick}
                     orderByField={controller.orderByField}
                     orderByReverse={controller.orderByReverse}
                     toggleSorting={controller.toggleSorting}

diff --git a/client/app/pages/groups/GroupDataSources.jsx b/client/app/pages/groups/GroupDataSources.jsx
@@ -27,7 +27,7 @@ import { currentUser } from '@/services/auth';
 import { Group } from '@/services/group';
 import { DataSource } from '@/services/data-source';
 import navigateTo from '@/services/navigateTo';
-import { routesToAngularRoutes, cancelEvent } from '@/lib/utils';
+import { routesToAngularRoutes } from '@/lib/utils';
 
 class GroupDataSources extends React.Component {
   static propTypes = {
@@ -64,7 +64,7 @@ class GroupDataSources extends React.Component {
       const menu = (
         <Menu
           selectedKeys={[datasource.view_only ? 'viewonly' : 'full']}
-          onClick={item => this.setDataSourcePermissions(item.domEvent, datasource, item.key)}
+          onClick={item => this.setDataSourcePermissions(datasource, item.key)}
         >
           <Menu.Item key="full">Full Access</Menu.Item>
           <Menu.Item key="viewonly">View Only</Menu.Item>
@@ -73,9 +73,7 @@ class GroupDataSources extends React.Component {
 
       return (
         <Dropdown trigger={['click']} overlay={menu}>
-          <Button className="w-100" onClick={cancelEvent()}>
-            {datasource.view_only ? 'View Only' : 'Full Access'} <Icon type="down" />
-          </Button>
+          <Button className="w-100">{datasource.view_only ? 'View Only' : 'Full Access'}<Icon type="down" /></Button>
         </Dropdown>
       );
     }, {
@@ -84,14 +82,21 @@ class GroupDataSources extends React.Component {
       isAvailable: () => currentUser.isAdmin,
     }),
     Columns.custom((text, datasource) => (
-      <Button className="w-100" type="danger" onClick={event => this.removeGroupDataSource(event, datasource)}>Remove</Button>
+      <Button className="w-100" type="danger" onClick={() => this.removeGroupDataSource(datasource)}>Remove</Button>
     ), {
       width: '1%',
       isAvailable: () => currentUser.isAdmin,
     }),
   ];
 
-  removeGroupDataSource = cancelEvent((datasource) => {
+  componentDidMount() {
+    Group.get({ id: this.groupId }).$promise.then((group) => {
+      this.group = group;
+      this.forceUpdate();
+    });
+  }
+
+  removeGroupDataSource = (datasource) => {
     Group.removeDataSource({ id: this.groupId, dataSourceId: datasource.id }).$promise
       .then(() => {
         this.props.controller.updatePagination({ page: 1 });
@@ -100,18 +105,9 @@ class GroupDataSources extends React.Component {
       .catch(() => {
         toastr.error('Failed to remove data source from group.');
       });
-  });
-
-  componentDidMount() {
-    Group.get({ id: this.groupId }).$promise.then((group) => {
-      this.group = group;
-      this.forceUpdate();
-    });
-  }
-
-  onTableRowClick = (event, item) => navigateTo('data_sources/' + item.id);
+  };
 
-  setDataSourcePermissions = cancelEvent((datasource, permission) => {
+  setDataSourcePermissions = (datasource, permission) => {
     const viewOnly = permission !== 'full';
 
     Group.updateDataSource({ id: this.groupId, dataSourceId: datasource.id }, { view_only: viewOnly }).$promise
@@ -122,7 +118,7 @@ class GroupDataSources extends React.Component {
       .catch(() => {
         toastr.error('Failed change data source permissions.');
       });
-  });
+  };
 
   addDataSources = () => {
     const allDataSources = DataSource.query().$promise;
@@ -199,7 +195,6 @@ class GroupDataSources extends React.Component {
                     items={controller.pageItems}
                     columns={this.listColumns}
                     showHeader={false}
-                    onRowClick={this.onTableRowClick}
                     context={this.actions}
                     orderByField={controller.orderByField}
                     orderByReverse={controller.orderByReverse}

diff --git a/client/app/pages/groups/GroupMembers.jsx b/client/app/pages/groups/GroupMembers.jsx
@@ -24,7 +24,7 @@ import { currentUser } from '@/services/auth';
 import { Group } from '@/services/group';
 import { User } from '@/services/user';
 import navigateTo from '@/services/navigateTo';
-import { routesToAngularRoutes, cancelEvent } from '@/lib/utils';
+import { routesToAngularRoutes } from '@/lib/utils';
 
 class GroupMembers extends React.Component {
   static propTypes = {
@@ -73,25 +73,21 @@ class GroupMembers extends React.Component {
     }),
   ];
 
-  removeGroupMember = cancelEvent((user) => {
-    Group.removeMember({ id: this.groupId, userId: user.id }).$promise
-      .then(() => {
-        this.props.controller.updatePagination({ page: 1 });
-        this.props.controller.update();
-      })
-      .catch(() => {
-        toastr.error('Failed to remove member from group.');
-      });
-  });
-
   componentDidMount() {
     Group.get({ id: this.groupId }).$promise.then((group) => {
       this.group = group;
       this.forceUpdate();
     });
   }
 
-  onTableRowClick = (event, item) => navigateTo('users/' + item.id);
+  removeGroupMember = (event, user) => Group.removeMember({ id: this.groupId, userId: user.id }).$promise
+    .then(() => {
+      this.props.controller.updatePagination({ page: 1 });
+      this.props.controller.update();
+    })
+    .catch(() => {
+      toastr.error('Failed to remove member from group.');
+    });
 
   addMembers = () => {
     const alreadyAddedUsers = map(this.props.controller.allItems, u => u.id);
@@ -164,7 +160,6 @@ class GroupMembers extends React.Component {
                     items={controller.pageItems}
                     columns={this.listColumns}
                     showHeader={false}
-                    onRowClick={this.onTableRowClick}
                     context={this.actions}
                     orderByField={controller.orderByField}
                     orderByReverse={controller.orderByReverse}

diff --git a/client/app/pages/groups/GroupsList.jsx b/client/app/pages/groups/GroupsList.jsx
@@ -38,9 +38,9 @@ class GroupsList extends React.Component {
     }),
     Columns.custom((text, group) => (
       <Button.Group>
-        <Button href={`groups/${group.id}`} onClick={e => e.stopPropagation()}>Members</Button>
+        <Button href={`groups/${group.id}`}>Members</Button>
         {currentUser.isAdmin && (
-          <Button href={`groups/${group.id}/data_sources`} onClick={e => e.stopPropagation()}>Data Sources</Button>
+          <Button href={`groups/${group.id}/data_sources`}>Data Sources</Button>
         )}
       </Button.Group>
     ), {
@@ -73,8 +73,6 @@ class GroupsList extends React.Component {
     });
   };
 
-  onTableRowClick = (event, item) => navigateTo('groups/' + item.id);
-
   onGroupDeleted = () => {
     this.props.controller.updatePagination({ page: 1 });
     this.props.controller.update();
@@ -103,7 +101,6 @@ class GroupsList extends React.Component {
                 items={controller.pageItems}
                 columns={this.listColumns}
                 showHeader={false}
-                onRowClick={this.onTableRowClick}
                 context={this.actions}
                 orderByField={controller.orderByField}
                 orderByReverse={controller.orderByReverse}

diff --git a/client/app/pages/queries-list/QueriesList.jsx b/client/app/pages/queries-list/QueriesList.jsx
@@ -18,7 +18,6 @@ import Layout from '@/components/layouts/ContentWithSidebar';
 
 import { Query } from '@/services/query';
 import { currentUser } from '@/services/auth';
-import navigateTo from '@/services/navigateTo';
 import { routesToAngularRoutes } from '@/lib/utils';
 
 import QueriesListEmptyState from './QueriesListEmptyState';
@@ -84,8 +83,6 @@ class QueriesList extends React.Component {
     ),
   ];
 
-  onTableRowClick = (event, item) => navigateTo('queries/' + item.id);
-
   render() {
     const { controller } = this.props;
     return (
@@ -123,7 +120,6 @@ class QueriesList extends React.Component {
                   <ItemsTable
                     items={controller.pageItems}
                     columns={this.listColumns}
-                    onRowClick={this.onTableRowClick}
                     orderByField={controller.orderByField}
                     orderByReverse={controller.orderByReverse}
                     toggleSorting={controller.toggleSorting}

diff --git a/client/app/pages/users/UsersList.jsx b/client/app/pages/users/UsersList.jsx
@@ -161,31 +161,11 @@ class UsersList extends React.Component {
     }
   };
 
-  onTableRowClick = (event, item) => navigateTo('users/' + item.id);
+  enableUser = (event, user) => User.enableUser(user).then(() => this.props.controller.update());
 
-  enableUser = (event, user) => {
-    // prevent default click action on table rows
-    event.preventDefault();
-    event.stopPropagation();
-    return User.enableUser(user)
-      .then(() => this.props.controller.update());
-  };
-
-  disableUser = (event, user) => {
-    // prevent default click action on table rows
-    event.preventDefault();
-    event.stopPropagation();
-    return User.disableUser(user)
-      .then(() => this.props.controller.update());
-  };
+  disableUser = (event, user) => User.disableUser(user).then(() => this.props.controller.update());
 
-  deleteUser = (event, user) => {
-    // prevent default click action on table rows
-    event.preventDefault();
-    event.stopPropagation();
-    return User.deleteUser(user)
-      .then(() => this.props.controller.update());
-  };
+  deleteUser = (event, user) => User.deleteUser(user).then(() => this.props.controller.update());
 
   // eslint-disable-next-line class-methods-use-this
   renderPageHeader() {
@@ -230,7 +210,6 @@ class UsersList extends React.Component {
                   <ItemsTable
                     items={controller.pageItems}
                     columns={this.listColumns}
-                    onRowClick={this.onTableRowClick}
                     context={this.actions}
                     orderByField={controller.orderByField}
                     orderByReverse={controller.orderByReverse}