Browse Source

ref(ui): Improve buttons for dashboards edit (#33025)

Evan Purkhiser 2 years ago
parent
commit
3cc4d672b8

+ 22 - 23
static/app/views/dashboardsV2/dashboard.tsx

@@ -20,7 +20,9 @@ import {fetchMetricsFields, fetchMetricsTags} from 'sentry/actionCreators/metric
 import {openAddDashboardWidgetModal} from 'sentry/actionCreators/modal';
 import {loadOrganizationTags} from 'sentry/actionCreators/tags';
 import {Client} from 'sentry/api';
+import Button from 'sentry/components/button';
 import {IconResize} from 'sentry/icons';
+import {t} from 'sentry/locale';
 import space from 'sentry/styles/space';
 import {Organization, PageFilters} from 'sentry/types';
 import trackAdvancedAnalyticsEvent from 'sentry/utils/analytics/trackAdvancedAnalyticsEvent';
@@ -49,6 +51,7 @@ import SortableWidget from './sortableWidget';
 import {DashboardDetails, DashboardWidgetSource, Widget, WidgetType} from './types';
 
 export const DRAG_HANDLE_CLASS = 'widget-drag';
+const DRAG_RESIZE_CLASS = 'widget-resize';
 const DESKTOP = 'desktop';
 const MOBILE = 'mobile';
 export const NUM_DESKTOP_COLS = 6;
@@ -428,7 +431,7 @@ class Dashboard extends Component<Props, State> {
       const key = constructGridItemKey(widget);
       const dragId = key;
       return (
-        <GridItem key={key} data-grid={widget.layout}>
+        <div key={key} data-grid={widget.layout}>
           <SortableWidget
             {...widgetProps}
             dragId={dragId}
@@ -436,7 +439,7 @@ class Dashboard extends Component<Props, State> {
             windowWidth={windowWidth}
             index={String(index)}
           />
-        </GridItem>
+        </div>
       );
     }
 
@@ -570,6 +573,7 @@ class Dashboard extends Component<Props, State> {
         rowHeight={ROW_HEIGHT}
         margin={WIDGET_MARGINS}
         draggableHandle={`.${DRAG_HANDLE_CLASS}`}
+        draggableCancel={`.${DRAG_RESIZE_CLASS}`}
         layouts={layouts}
         onLayoutChange={this.handleLayoutChange}
         onBreakpointChange={this.handleBreakpointChange}
@@ -577,11 +581,13 @@ class Dashboard extends Component<Props, State> {
         isResizable={canModifyLayout}
         resizeHandle={
           <ResizeHandle
-            className="react-resizable-handle"
+            aria-label={t('Resize Widget')}
             data-test-id="custom-resize-handle"
-          >
-            <IconResize />
-          </ResizeHandle>
+            className={DRAG_RESIZE_CLASS}
+            size="xsmall"
+            borderless
+            icon={<IconResize size="xs" />}
+          />
         }
         useCSSTransforms={false}
         isBounded
@@ -679,31 +685,24 @@ const AddWidgetWrapper = styled('div')`
   background-color: ${p => p.theme.background};
 `;
 
-const GridItem = styled('div')`
-  .react-resizable-handle {
-    z-index: 2;
-  }
-`;
-
 const GridLayout = styled(WidthProvider(Responsive))`
   margin: -${space(2)};
 
-  .react-resizable-handle {
-    background-image: none;
-  }
-
-  .react-grid-item > .react-resizable-handle::after {
-    border: none;
-  }
-
   .react-grid-item.react-grid-placeholder {
     background: ${p => p.theme.purple200};
+    border-radius: ${p => p.theme.borderRadius};
   }
 `;
 
-const ResizeHandle = styled('div')`
+const ResizeHandle = styled(Button)`
   position: absolute;
-  bottom: 2px;
-  right: 2px;
+  z-index: 2;
+  bottom: ${space(0.5)};
+  right: ${space(0.5)};
+  color: ${p => p.theme.subText};
   cursor: nwse-resize;
+
+  .react-resizable-hide & {
+    display: none;
+  }
 `;

+ 37 - 30
static/app/views/dashboardsV2/widgetCard/index.tsx

@@ -6,6 +6,7 @@ import styled from '@emotion/styled';
 import {Location} from 'history';
 
 import {Client} from 'sentry/api';
+import Button from 'sentry/components/button';
 import {HeaderTitle} from 'sentry/components/charts/styles';
 import ErrorBoundary from 'sentry/components/errorBoundary';
 import {Panel} from 'sentry/components/panels';
@@ -74,24 +75,39 @@ class WidgetCard extends React.Component<Props> {
       <ToolbarPanel>
         <IconContainer style={{visibility: hideToolbar ? 'hidden' : 'visible'}}>
           {!isMobile && (
-            <IconClick>
-              <StyledIconGrabbable
-                color="textColor"
-                className={DRAG_HANDLE_CLASS}
-                {...draggableProps?.listeners}
-                {...draggableProps?.attributes}
-              />
-            </IconClick>
+            <GrabbableButton
+              size="xsmall"
+              aria-label={t('Drag Widget')}
+              icon={<IconGrabbable />}
+              borderless
+              className={DRAG_HANDLE_CLASS}
+              {...draggableProps?.listeners}
+              {...draggableProps?.attributes}
+            />
           )}
-          <IconClick data-test-id="widget-edit" onClick={onEdit}>
-            <IconEdit color="textColor" />
-          </IconClick>
-          <IconClick aria-label={t('Duplicate Widget')} onClick={onDuplicate}>
-            <IconCopy color="textColor" />
-          </IconClick>
-          <IconClick data-test-id="widget-delete" onClick={onDelete}>
-            <IconDelete color="textColor" />
-          </IconClick>
+          <Button
+            data-test-id="widget-edit"
+            aria-label={t('Edit Widget')}
+            size="xsmall"
+            borderless
+            onClick={onEdit}
+            icon={<IconEdit />}
+          />
+          <Button
+            aria-label={t('Duplicate Widget')}
+            size="xsmall"
+            borderless
+            onClick={onDuplicate}
+            icon={<IconCopy />}
+          />
+          <Button
+            data-test-id="widget-delete"
+            aria-label={t('Delete Widget')}
+            borderless
+            size="xsmall"
+            onClick={onDelete}
+            icon={<IconDelete />}
+          />
         </IconContainer>
       </ToolbarPanel>
     );
@@ -239,22 +255,12 @@ const ToolbarPanel = styled('div')`
 
 const IconContainer = styled('div')`
   display: flex;
-  margin: 10px ${space(2)};
+  margin: ${space(1)};
   touch-action: none;
 `;
 
-const IconClick = styled('div')`
-  padding: ${space(1)};
-
-  &:hover {
-    cursor: pointer;
-  }
-`;
-
-const StyledIconGrabbable = styled(IconGrabbable)`
-  &:hover {
-    cursor: grab;
-  }
+const GrabbableButton = styled(Button)`
+  cursor: grab;
 `;
 
 const WidgetTitle = styled(HeaderTitle)`
@@ -264,6 +270,7 @@ const WidgetTitle = styled(HeaderTitle)`
 
 const WidgetHeader = styled('div')`
   padding: ${space(2)} ${space(1)} 0 ${space(3)};
+  min-height: 36px;
   width: 100%;
   display: flex;
   align-items: center;

+ 1 - 1
tests/acceptance/page_objects/dashboard_detail.py

@@ -2,7 +2,7 @@ from .base import BasePage
 
 EDIT_WIDGET_BUTTON = '[data-test-id="widget-edit"]'
 WIDGET_DRAG_HANDLE = ".widget-drag"
-WIDGET_RESIZE_HANDLE = ".react-resizable-handle"
+WIDGET_RESIZE_HANDLE = ".widget-resize"
 WIDGET_TITLE_FIELD = 'input[data-test-id="widget-title-input"]'
 
 

+ 1 - 1
tests/js/spec/views/dashboardsV2/dashboard.spec.tsx

@@ -142,6 +142,6 @@ describe('Dashboards > Dashboard', () => {
       />,
       initialData.routerContext
     );
-    expect(wrapper.find('StyledIconGrabbable')).toHaveLength(1);
+    expect(wrapper.find('GrabbableButton')).toHaveLength(1);
   });
 });

+ 3 - 3
tests/js/spec/views/dashboardsV2/detail.spec.jsx

@@ -354,13 +354,13 @@ describe('Dashboards > Detail', function () {
       wrapper
         .find('WidgetCard')
         .at(1)
-        .find('IconClick[data-test-id="widget-delete"]')
+        .find('Button[data-test-id="widget-delete"]')
         .simulate('click');
 
       wrapper
         .find('WidgetCard')
         .at(1)
-        .find('IconClick[data-test-id="widget-delete"]')
+        .find('Button[data-test-id="widget-delete"]')
         .simulate('click');
 
       // Save changes
@@ -406,7 +406,7 @@ describe('Dashboards > Detail', function () {
       wrapper
         .find('WidgetCard')
         .first()
-        .find('IconClick[data-test-id="widget-edit"]')
+        .find('Button[data-test-id="widget-edit"]')
         .simulate('click');
 
       expect(openEditModal).toHaveBeenCalledTimes(1);

+ 3 - 3
tests/js/spec/views/dashboardsV2/gridLayout/detail.spec.jsx

@@ -308,13 +308,13 @@ describe('Dashboards > Detail', function () {
       wrapper
         .find('WidgetCard')
         .at(1)
-        .find('IconClick[data-test-id="widget-delete"]')
+        .find('Button[data-test-id="widget-delete"]')
         .simulate('click');
 
       wrapper
         .find('WidgetCard')
         .at(1)
-        .find('IconClick[data-test-id="widget-delete"]')
+        .find('Button[data-test-id="widget-delete"]')
         .simulate('click');
 
       // Save changes
@@ -360,7 +360,7 @@ describe('Dashboards > Detail', function () {
       wrapper
         .find('WidgetCard')
         .first()
-        .find('IconClick[data-test-id="widget-edit"]')
+        .find('Button[data-test-id="widget-edit"]')
         .simulate('click');
 
       expect(openEditModal).toHaveBeenCalledTimes(1);