Browse Source

feat(widget-library): Default to custom widget tab (#30888)

Default the Add Widget button to open the modal in
the CustomWidget tab so users see what they expect.
Puts the Widget Library in the secondary tab and
adds a beta badge to it.
Shruthi 3 years ago
parent
commit
b0fe5a6461

+ 31 - 27
static/app/components/modals/dashboardWidgetLibraryModal/tabsButtonBar.tsx

@@ -5,6 +5,7 @@ import {
   openAddDashboardWidgetModal,
   openDashboardWidgetLibraryModal,
 } from 'sentry/actionCreators/modal';
+import FeatureBadge from 'sentry/components/featureBadge';
 import {t} from 'sentry/locale';
 import space from 'sentry/styles/space';
 import {Organization} from 'sentry/types';
@@ -44,8 +45,31 @@ export function TabsButtonBar({
 }: Props) {
   return (
     <StyledButtonBar active={activeTab}>
+      <CustomButton
+        barId={TAB.Custom}
+        onClick={() => {
+          if (activeTab === TAB.Custom) {
+            return;
+          }
+          trackAdvancedAnalyticsEvent('dashboards_views.widget_library.switch_tab', {
+            organization,
+            to: TAB.Custom,
+          });
+          openAddDashboardWidgetModal({
+            organization,
+            dashboard,
+            selectedWidgets,
+            widget: customWidget,
+            source: DashboardWidgetSource.LIBRARY,
+            onAddLibraryWidget: onAddWidget,
+          });
+        }}
+      >
+        {t('Custom Widget')}
+      </CustomButton>
       <LibraryButton
         barId={TAB.Library}
+        data-test-id="library-tab"
         onClick={() => {
           if (activeTab === TAB.Library) {
             return;
@@ -66,44 +90,24 @@ export function TabsButtonBar({
         }}
       >
         {t('Widget Library')}
+        <FeatureBadge type="beta" />
       </LibraryButton>
-      <CustomButton
-        barId={TAB.Custom}
-        onClick={() => {
-          if (activeTab === TAB.Custom) {
-            return;
-          }
-          trackAdvancedAnalyticsEvent('dashboards_views.widget_library.switch_tab', {
-            organization,
-            to: TAB.Custom,
-          });
-          openAddDashboardWidgetModal({
-            organization,
-            dashboard,
-            selectedWidgets,
-            widget: customWidget,
-            source: DashboardWidgetSource.LIBRARY,
-            onAddLibraryWidget: onAddWidget,
-          });
-        }}
-      >
-        {t('Custom Widget')}
-      </CustomButton>
     </StyledButtonBar>
   );
 }
 
 const StyledButtonBar = styled(ButtonBar)`
-  display: inline;
+  display: inline-flex;
   margin-bottom: ${space(2)};
 `;
 
 const LibraryButton = styled(Button)`
-  border-top-right-radius: 0;
-  border-bottom-right-radius: 0;
+  border-top-left-radius: 0;
+  border-bottom-left-radius: 0;
 `;
 
 const CustomButton = styled(Button)`
-  border-top-left-radius: 0;
-  border-bottom-left-radius: 0;
+  border-top-right-radius: 0;
+  border-bottom-right-radius: 0;
+  line-height: 17px;
 `;

+ 19 - 19
static/app/views/dashboardsV2/dashboard.tsx

@@ -15,10 +15,7 @@ import zip from 'lodash/zip';
 
 import {validateWidget} from 'sentry/actionCreators/dashboards';
 import {addErrorMessage} from 'sentry/actionCreators/indicator';
-import {
-  openAddDashboardWidgetModal,
-  openDashboardWidgetLibraryModal,
-} from 'sentry/actionCreators/modal';
+import {openAddDashboardWidgetModal} from 'sentry/actionCreators/modal';
 import {loadOrganizationTags} from 'sentry/actionCreators/tags';
 import {Client} from 'sentry/api';
 import space from 'sentry/styles/space';
@@ -69,6 +66,7 @@ type Props = {
   onUpdate: (widgets: Widget[]) => void;
   onSetWidgetToBeUpdated: (widget: Widget) => void;
   handleAddLibraryWidgets: (widgets: Widget[]) => void;
+  handleAddCustomWidget: (widget: Widget) => void;
   layout: Layout[];
   onLayoutChange: (layout: Layout[]) => void;
   paramDashboardId?: string;
@@ -132,11 +130,11 @@ class Dashboard extends Component<Props, State> {
   }
 
   async addNewWidget() {
-    const {api, organization, newWidget} = this.props;
+    const {api, organization, newWidget, handleAddCustomWidget} = this.props;
     if (newWidget) {
       try {
         await validateWidget(api, organization.slug, newWidget);
-        this.handleAddComplete(newWidget);
+        handleAddCustomWidget(newWidget);
       } catch (error) {
         // Don't do anything, widget isn't valid
         addErrorMessage(error);
@@ -150,7 +148,13 @@ class Dashboard extends Component<Props, State> {
   }
 
   handleStartAdd = () => {
-    const {organization, dashboard, selection, handleAddLibraryWidgets} = this.props;
+    const {
+      organization,
+      dashboard,
+      selection,
+      handleAddLibraryWidgets,
+      handleAddCustomWidget,
+    } = this.props;
     trackAdvancedAnalyticsEvent('dashboards_views.add_widget_modal.opened', {
       organization,
     });
@@ -159,10 +163,13 @@ class Dashboard extends Component<Props, State> {
       trackAdvancedAnalyticsEvent('dashboards_views.widget_library.opened', {
         organization,
       });
-      openDashboardWidgetLibraryModal({
+      openAddDashboardWidgetModal({
         organization,
         dashboard,
-        onAddWidget: (widgets: Widget[]) => handleAddLibraryWidgets(widgets),
+        selection,
+        onAddWidget: handleAddCustomWidget,
+        onAddLibraryWidget: (widgets: Widget[]) => handleAddLibraryWidgets(widgets),
+        source: DashboardWidgetSource.LIBRARY,
       });
       return;
     }
@@ -170,7 +177,7 @@ class Dashboard extends Component<Props, State> {
       organization,
       dashboard,
       selection,
-      onAddWidget: this.handleAddComplete,
+      onAddWidget: handleAddCustomWidget,
       source: DashboardWidgetSource.DASHBOARDS,
     });
   };
@@ -196,14 +203,6 @@ class Dashboard extends Component<Props, State> {
     });
   };
 
-  handleAddComplete = (widget: Widget) => {
-    let newWidget = widget;
-    if (this.props.organization.features.includes('dashboard-grid-layout')) {
-      newWidget = assignTempId(widget);
-    }
-    this.props.onUpdate([...this.props.dashboard.widgets, newWidget]);
-  };
-
   handleUpdateComplete = (prevWidget: Widget) => (nextWidget: Widget) => {
     const nextList = [...this.props.dashboard.widgets];
     const updateIndex = nextList.indexOf(prevWidget);
@@ -248,6 +247,7 @@ class Dashboard extends Component<Props, State> {
       location,
       paramDashboardId,
       onSetWidgetToBeUpdated,
+      handleAddCustomWidget,
     } = this.props;
 
     if (organization.features.includes('metrics')) {
@@ -279,7 +279,7 @@ class Dashboard extends Component<Props, State> {
       organization,
       widget,
       selection,
-      onAddWidget: this.handleAddComplete,
+      onAddWidget: handleAddCustomWidget,
       onUpdateWidget: this.handleUpdateComplete(widget),
     };
     openAddDashboardWidgetModal({

+ 16 - 3
static/app/views/dashboardsV2/detail.tsx

@@ -10,7 +10,7 @@ import {
   updateDashboard,
 } from 'sentry/actionCreators/dashboards';
 import {addSuccessMessage} from 'sentry/actionCreators/indicator';
-import {openDashboardWidgetLibraryModal} from 'sentry/actionCreators/modal';
+import {openAddDashboardWidgetModal} from 'sentry/actionCreators/modal';
 import {Client} from 'sentry/api';
 import Breadcrumbs from 'sentry/components/breadcrumbs';
 import HookOrDefault from 'sentry/components/hookOrDefault';
@@ -34,6 +34,7 @@ import {
   DashboardDetails,
   DashboardListItem,
   DashboardState,
+  DashboardWidgetSource,
   MAX_WIDGETS,
   Widget,
 } from './types';
@@ -300,15 +301,25 @@ class DashboardDetail extends Component<Props, State> {
     );
   };
 
+  handleAddCustomWidget = (widget: Widget) => {
+    let newWidget = widget;
+    if (this.props.organization.features.includes('dashboard-grid-layout')) {
+      newWidget = assignTempId(widget);
+    }
+    this.onUpdateWidget([...this.props.dashboard.widgets, newWidget]);
+  };
+
   onAddWidget = () => {
     const {organization, dashboard} = this.props;
     this.setState({
       modifiedDashboard: cloneDashboard(dashboard),
     });
-    openDashboardWidgetLibraryModal({
+    openAddDashboardWidgetModal({
       organization,
       dashboard,
-      onAddWidget: (widgets: Widget[]) => this.handleAddLibraryWidgets(widgets),
+      onAddWidget: (widget: Widget) => this.handleAddCustomWidget(widget),
+      onAddLibraryWidget: (widgets: Widget[]) => this.handleAddLibraryWidgets(widgets),
+      source: DashboardWidgetSource.LIBRARY,
     });
   };
 
@@ -536,6 +547,7 @@ class DashboardDetail extends Component<Props, State> {
               onUpdate={this.onUpdateWidget}
               onSetWidgetToBeUpdated={this.onSetWidgetToBeUpdated}
               handleAddLibraryWidgets={this.handleAddLibraryWidgets}
+              handleAddCustomWidget={this.handleAddCustomWidget}
               router={router}
               location={location}
               layout={layout}
@@ -618,6 +630,7 @@ class DashboardDetail extends Component<Props, State> {
                   widgetLimitReached={widgetLimitReached}
                   onUpdate={this.onUpdateWidget}
                   handleAddLibraryWidgets={this.handleAddLibraryWidgets}
+                  handleAddCustomWidget={this.handleAddCustomWidget}
                   onSetWidgetToBeUpdated={this.onSetWidgetToBeUpdated}
                   router={router}
                   location={location}

+ 2 - 0
tests/acceptance/test_organization_dashboards.py

@@ -89,6 +89,8 @@ class OrganizationDashboardsAcceptanceTest(AcceptanceTestCase):
             button = self.browser.element('[data-test-id="add-widget-library"]')
             button.click()
 
+            self.browser.element('[data-test-id="library-tab"]').click()
+
             # Edit the first widget.
             self.browser.element('[data-test-id="widget-library-card-0"]').click()
             self.browser.element('[data-test-id="widget-library-card-2"]').click()

+ 1 - 1
tests/js/spec/components/modals/dashboardWidgetLibraryModal.spec.jsx

@@ -65,7 +65,7 @@ describe('Modals -> DashboardWidgetLibraryModal', function () {
     expect(screen.queryByText('Users Affected by Errors')).toBeInTheDocument();
 
     expect(
-      screen.getByRole('button', {name: 'Widget Library', current: true})
+      screen.getByRole('button', {name: 'Widget Library beta', current: true})
     ).toBeInTheDocument();
     expect(
       screen.getByRole('button', {name: 'Custom Widget', current: false})

+ 12 - 9
tests/js/spec/views/dashboardsV2/dashboard.spec.tsx

@@ -40,15 +40,16 @@ describe('Dashboards > Dashboard', () => {
     });
   });
   it('dashboard adds new widget if component is mounted with newWidget prop', async () => {
-    const mock = jest.fn();
+    const mockHandleAddCustomWidget = jest.fn();
     const wrapper = mountWithTheme(
       <Dashboard
         paramDashboardId="1"
         dashboard={mockDashboard}
         organization={initialData.organization}
         isEditing={false}
-        onUpdate={mock}
-        handleAddLibraryWidgets={mock}
+        onUpdate={() => undefined}
+        handleAddLibraryWidgets={() => undefined}
+        handleAddCustomWidget={mockHandleAddCustomWidget}
         onSetWidgetToBeUpdated={() => undefined}
         router={initialData.router}
         location={initialData.location}
@@ -61,19 +62,20 @@ describe('Dashboards > Dashboard', () => {
     );
     await tick();
     wrapper.update();
-    expect(mock).toHaveBeenCalled();
+    expect(mockHandleAddCustomWidget).toHaveBeenCalled();
   });
 
   it('dashboard adds new widget if component updated with newWidget prop', async () => {
-    const mock = jest.fn();
+    const mockHandleAddCustomWidget = jest.fn();
     const wrapper = mountWithTheme(
       <Dashboard
         paramDashboardId="1"
         dashboard={mockDashboard}
         organization={initialData.organization}
         isEditing={false}
-        onUpdate={mock}
-        handleAddLibraryWidgets={mock}
+        onUpdate={() => undefined}
+        handleAddLibraryWidgets={() => undefined}
+        handleAddCustomWidget={mockHandleAddCustomWidget}
         onSetWidgetToBeUpdated={() => undefined}
         router={initialData.router}
         location={initialData.location}
@@ -83,11 +85,11 @@ describe('Dashboards > Dashboard', () => {
       />,
       initialData.routerContext
     );
-    expect(mock).not.toHaveBeenCalled();
+    expect(mockHandleAddCustomWidget).not.toHaveBeenCalled();
     wrapper.setProps({newWidget});
     await tick();
     wrapper.update();
-    expect(mock).toHaveBeenCalled();
+    expect(mockHandleAddCustomWidget).toHaveBeenCalled();
   });
 
   it('displays widgets with drag handle when in edit mode', () => {
@@ -100,6 +102,7 @@ describe('Dashboards > Dashboard', () => {
         onUpdate={() => undefined}
         onSetWidgetToBeUpdated={() => undefined}
         handleAddLibraryWidgets={() => undefined}
+        handleAddCustomWidget={() => undefined}
         router={initialData.router}
         location={initialData.location}
         widgetLimitReached={false}

+ 13 - 4
tests/js/spec/views/dashboardsV2/detail.spec.jsx

@@ -184,7 +184,6 @@ describe('Dashboards > Detail', function () {
 
   describe('custom dashboards', function () {
     let wrapper, initialData, widgets, mockVisit;
-    const openLibraryModal = jest.spyOn(modals, 'openDashboardWidgetLibraryModal');
     const openEditModal = jest.spyOn(modals, 'openAddDashboardWidgetModal');
 
     beforeEach(function () {
@@ -517,7 +516,12 @@ describe('Dashboards > Detail', function () {
       wrapper.find('Controls Button[data-test-id="dashboard-edit"]').simulate('click');
       wrapper.update();
       wrapper.find('AddButton[data-test-id="widget-add"]').simulate('click');
-      expect(openLibraryModal).toHaveBeenCalledTimes(1);
+      expect(openEditModal).toHaveBeenCalledTimes(1);
+      expect(openEditModal).toHaveBeenCalledWith(
+        expect.objectContaining({
+          source: types.DashboardWidgetSource.LIBRARY,
+        })
+      );
     });
 
     it('hides add widget option', async function () {
@@ -627,7 +631,7 @@ describe('Dashboards > Detail', function () {
       );
     });
 
-    it('can add library widgets', async function () {
+    it('opens add widget to custom  modal', async function () {
       types.MAX_WIDGETS = 10;
 
       initialData = initializeOrg({
@@ -662,7 +666,12 @@ describe('Dashboards > Detail', function () {
         .find('Controls Button[data-test-id="add-widget-library"]')
         .simulate('click');
 
-      expect(openLibraryModal).toHaveBeenCalledTimes(1);
+      expect(openEditModal).toHaveBeenCalledTimes(1);
+      expect(openEditModal).toHaveBeenCalledWith(
+        expect.objectContaining({
+          source: types.DashboardWidgetSource.LIBRARY,
+        })
+      );
     });
 
     it('disables add library widgets when max widgets reached', async function () {

+ 11 - 9
tests/js/spec/views/dashboardsV2/gridLayout/dashboard.spec.tsx

@@ -40,15 +40,16 @@ describe('Dashboards > Dashboard', () => {
     });
   });
   it('dashboard adds new widget if component is mounted with newWidget prop', async () => {
-    const mock = jest.fn();
+    const mockHandleAddCustomWidget = jest.fn();
     const wrapper = mountWithTheme(
       <Dashboard
         paramDashboardId="1"
         dashboard={mockDashboard}
         organization={initialData.organization}
         isEditing={false}
-        onUpdate={mock}
-        handleAddLibraryWidgets={mock}
+        onUpdate={() => undefined}
+        handleAddLibraryWidgets={() => undefined}
+        handleAddCustomWidget={mockHandleAddCustomWidget}
         onSetWidgetToBeUpdated={() => undefined}
         router={initialData.router}
         location={initialData.location}
@@ -61,19 +62,20 @@ describe('Dashboards > Dashboard', () => {
     );
     await tick();
     wrapper.update();
-    expect(mock).toHaveBeenCalled();
+    expect(mockHandleAddCustomWidget).toHaveBeenCalled();
   });
 
   it('dashboard adds new widget if component updated with newWidget prop', async () => {
-    const mock = jest.fn();
+    const mockHandleAddCustomWidget = jest.fn();
     const wrapper = mountWithTheme(
       <Dashboard
         paramDashboardId="1"
         dashboard={mockDashboard}
         organization={initialData.organization}
         isEditing={false}
-        onUpdate={mock}
-        handleAddLibraryWidgets={mock}
+        onUpdate={() => undefined}
+        handleAddLibraryWidgets={() => undefined}
+        handleAddCustomWidget={mockHandleAddCustomWidget}
         onSetWidgetToBeUpdated={() => undefined}
         router={initialData.router}
         location={initialData.location}
@@ -83,10 +85,10 @@ describe('Dashboards > Dashboard', () => {
       />,
       initialData.routerContext
     );
-    expect(mock).not.toHaveBeenCalled();
+    expect(mockHandleAddCustomWidget).not.toHaveBeenCalled();
     wrapper.setProps({newWidget});
     await tick();
     wrapper.update();
-    expect(mock).toHaveBeenCalled();
+    expect(mockHandleAddCustomWidget).toHaveBeenCalled();
   });
 });

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

@@ -126,7 +126,6 @@ describe('Dashboards > Detail', function () {
   describe('custom dashboards', function () {
     let wrapper, initialData, widgets, mockVisit;
 
-    const openLibraryModal = jest.spyOn(modals, 'openDashboardWidgetLibraryModal');
     const openEditModal = jest.spyOn(modals, 'openAddDashboardWidgetModal');
     beforeEach(function () {
       initialData = initializeOrg({organization});
@@ -391,7 +390,12 @@ describe('Dashboards > Detail', function () {
       wrapper.find('Controls Button[data-test-id="dashboard-edit"]').simulate('click');
       wrapper.update();
       wrapper.find('AddButton[data-test-id="widget-add"]').simulate('click');
-      expect(openLibraryModal).toHaveBeenCalledTimes(1);
+      expect(openEditModal).toHaveBeenCalledTimes(1);
+      expect(openEditModal).toHaveBeenCalledWith(
+        expect.objectContaining({
+          source: types.DashboardWidgetSource.LIBRARY,
+        })
+      );
     });
 
     it('hides add widget option', async function () {