Browse Source

feat(ui): Add `priority` prop to DropdownMenuItemV2 (#31693)

* ref(ui): Use DropdownMenuV2 for dashboard widget dropdown

* fix(test): Update tests related to dashboard widget dropdowns

Necessary given the new implementation of DropdownMenuV2.

* fix(acceptance): Update acceptance tests for Dashboards

* feat(ui): Add priority prop to DropdownMenuItemV2

To optionally use a different item color (danger = red, primary = purple).

* ref(ui): Use danger priority for delete actions in DropdownMenuV2

* fix(tsc): Add missing MenuItemProps type

* ref(ui): Limit query card dropdown height

* style: Use type Priority instead of 'primary' | 'danger'

* style(lint): Auto commit lint changes

Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com>
Vu Luong 3 years ago
parent
commit
83c547a6e8

+ 63 - 17
static/app/components/dropdownMenuItemV2.tsx

@@ -10,17 +10,14 @@ import {Node} from '@react-types/shared';
 import {IconChevron} from 'sentry/icons';
 import overflowEllipsis from 'sentry/styles/overflowEllipsis';
 import space from 'sentry/styles/space';
+import {Theme} from 'sentry/utils/theme';
 
+type Priority = 'primary' | 'danger';
 export type MenuItemProps = {
   /**
    * Item key. Must be unique across the entire menu, including sub-menus.
    */
   key: string;
-  /**
-   * Item label. Should prefereably be a string. If not, make sure that
-   * there are appropriate aria-labels.
-   */
-  label: React.ReactNode;
   /**
    * Sub-items that are nested inside this item. By default, sub-items are
    * rendered collectively as menu sections inside the current menu. If
@@ -37,6 +34,11 @@ export type MenuItemProps = {
    * when `children` is also defined.
    */
   isSubmenu?: boolean;
+  /**
+   * Item label. Should prefereably be a string. If not, make sure that
+   * there are appropriate aria-labels.
+   */
+  label?: React.ReactNode;
   /*
    * Items to be added to the left of the label
    */
@@ -52,6 +54,11 @@ export type MenuItemProps = {
    * item's key is passed as an argument.
    */
   onAction?: (key: MenuItemProps['key']) => void;
+  /**
+   * Accented text and background (on hover) colors. Primary = purple, and
+   * danger = red.
+   */
+  priority?: Priority;
   /**
    * Whether to show a line divider below this menu item
    */
@@ -194,6 +201,7 @@ const MenuItem = withRouter(
       keyboardProps
     );
     const {
+      priority,
       details,
       leadingItems,
       leadingItemsSpanFullHeight,
@@ -208,11 +216,13 @@ const MenuItem = withRouter(
         ref={ref}
         as={renderAs}
         isDisabled={isDisabled}
+        priority={priority}
         data-test-id={item.key}
+        {...(item.to && {'data-test-href': item.to})}
         {...props}
         {...(isSubmenuTrigger && {role: 'menuitemradio'})}
       >
-        <InnerWrap isFocused={isFocused}>
+        <InnerWrap isFocused={isFocused} priority={priority}>
           {leadingItems && (
             <LeadingItems
               isDisabled={isDisabled}
@@ -223,10 +233,18 @@ const MenuItem = withRouter(
           )}
           <ContentWrap isFocused={isFocused} showDividers={showDividers}>
             <LabelWrap>
-              <Label isDisabled={isDisabled} {...labelProps} aria-hidden="true">
+              <Label {...labelProps} aria-hidden="true">
                 {label}
               </Label>
-              {details && <Details {...descriptionProps}>{details}</Details>}
+              {details && (
+                <Details
+                  isDisabled={isDisabled}
+                  priority={priority}
+                  {...descriptionProps}
+                >
+                  {details}
+                </Details>
+              )}
             </LabelWrap>
             {(trailingItems || isSubmenuTrigger) && (
               <TrailingItems
@@ -247,7 +265,11 @@ const MenuItem = withRouter(
 );
 export default MenuItem;
 
-const MenuItemWrap = styled('li')<{isDisabled?: boolean}>`
+const MenuItemWrap = styled('li')<{
+  isDisabled?: boolean;
+  isFocused?: boolean;
+  priority?: Priority;
+}>`
   position: static;
   list-style-type: none;
   margin: 0;
@@ -255,8 +277,14 @@ const MenuItemWrap = styled('li')<{isDisabled?: boolean}>`
   cursor: pointer;
 
   color: ${p => p.theme.textColor};
-
-  ${p => p.isDisabled && `cursor: initial;`}
+  ${p => p.priority === 'primary' && `color: ${p.theme.activeText};`}
+  ${p => p.priority === 'danger' && `color: ${p.theme.errorText};`}
+  ${p =>
+    p.isDisabled &&
+    `
+    color: ${p.theme.subText};
+    cursor: initial;
+  `}
 
   &:focus {
     outline: none;
@@ -266,14 +294,30 @@ const MenuItemWrap = styled('li')<{isDisabled?: boolean}>`
   }
 `;
 
-const InnerWrap = styled('div')<{isFocused: boolean}>`
+const getHoverBackground = (theme: Theme, priority?: Priority) => {
+  let hoverBackground: string;
+  switch (priority) {
+    case 'primary':
+      hoverBackground = theme.purple100;
+      break;
+    case 'danger':
+      hoverBackground = theme.red100;
+      break;
+    default:
+      hoverBackground = theme.hover;
+  }
+
+  return `background: ${hoverBackground}; z-index: 1;`;
+};
+
+const InnerWrap = styled('div')<{isFocused: boolean; priority?: Priority}>`
   display: flex;
   position: relative;
   padding: 0 ${space(1)};
   border-radius: ${p => p.theme.borderRadius};
   box-sizing: border-box;
 
-  ${p => p.isFocused && `background: ${p.theme.hover}; z-index: 1;`}
+  ${p => p.isFocused && getHoverBackground(p.theme, p.priority)}
 `;
 
 const LeadingItems = styled('div')<{isDisabled?: boolean; spanFullHeight?: boolean}>`
@@ -319,21 +363,23 @@ const LabelWrap = styled('div')`
   width: 100%;
 `;
 
-const Label = styled('p')<{isDisabled?: boolean}>`
+const Label = styled('p')`
   margin-bottom: 0;
   line-height: 1.4;
   white-space: nowrap;
   ${overflowEllipsis}
-
-  ${p => p.isDisabled && `color: ${p.theme.subText};`}
 `;
 
-const Details = styled('p')`
+const Details = styled('p')<{isDisabled: boolean; priority?: Priority}>`
   font-size: ${p => p.theme.fontSizeSmall};
   color: ${p => p.theme.subText};
   line-height: 1.2;
   margin-bottom: 0;
   ${overflowEllipsis}
+
+  ${p => p.priority === 'primary' && `color: ${p.theme.activeText};`}
+  ${p => p.priority === 'danger' && `color: ${p.theme.errorText};`}
+  ${p => p.isDisabled && `color: ${p.theme.subText};`}
 `;
 
 const TrailingItems = styled('div')<{isDisabled?: boolean; spanFullHeight?: boolean}>`

+ 3 - 0
static/app/utils/theme.tsx

@@ -200,11 +200,13 @@ const generateAliases = (colors: BaseColors) => ({
    * A color that denotes a "success", or something good
    */
   success: colors.green300,
+  successText: colors.green400,
 
   /**
    * A color that denotes an error, or something that is wrong
    */
   error: colors.red300,
+  errorText: colors.red400,
 
   /**
    * A color that indicates something is disabled where user can not interact or use
@@ -223,6 +225,7 @@ const generateAliases = (colors: BaseColors) => ({
    */
   active: colors.purple300,
   activeHover: colors.purple400,
+  activeText: colors.purple400,
 
   /**
    * Indicates that something has "focus", which is different than "active" state as it is more temporal

+ 3 - 1
static/app/views/dashboardsV2/manage/dashboardList.tsx

@@ -13,6 +13,7 @@ import {Client} from 'sentry/api';
 import Button from 'sentry/components/button';
 import {openConfirmModal} from 'sentry/components/confirm';
 import DropdownMenuControlV2 from 'sentry/components/dropdownMenuControlV2';
+import {MenuItemProps} from 'sentry/components/dropdownMenuItemV2';
 import EmptyStateWarning from 'sentry/components/emptyStateWarning';
 import Pagination from 'sentry/components/pagination';
 import TimeSince from 'sentry/components/timeSince';
@@ -83,7 +84,7 @@ function DashboardList({
   }
 
   function renderDropdownMenu(dashboard: DashboardListItem) {
-    const menuItems = [
+    const menuItems: MenuItemProps[] = [
       {
         key: 'dashboard-duplicate',
         label: t('Duplicate'),
@@ -94,6 +95,7 @@ function DashboardList({
         key: 'dashboard-delete',
         label: t('Delete'),
         leadingItems: <IconDelete />,
+        priority: 'danger',
         onAction: () => {
           openConfirmModal({
             message: t('Are you sure you want to delete this dashboard?'),

+ 1 - 0
static/app/views/dashboardsV2/widgetCard/widgetCardContextMenu.tsx

@@ -202,6 +202,7 @@ function WidgetCardContextMenu({
       key: 'delete-widget',
       label: t('Delete Widget'),
       leadingItems: <IconDelete />,
+      priority: 'danger',
       onAction: () => {
         openConfirmModal({
           message: t('Are you sure you want to delete this widget?'),

+ 2 - 1
static/app/views/eventsV2/queryList.tsx

@@ -273,7 +273,7 @@ class QueryList extends React.Component<Props> {
       const dateStatus = <TimeSince date={savedQuery.dateUpdated} />;
       const referrer = `api.discover.${eventView.getDisplayMode()}-chart`;
 
-      const menuItems = (canAddToDashboard: boolean) => [
+      const menuItems = (canAddToDashboard: boolean): MenuItemProps[] => [
         ...(canAddToDashboard
           ? [
               {
@@ -295,6 +295,7 @@ class QueryList extends React.Component<Props> {
           key: 'delete',
           label: t('Delete Query'),
           leadingItems: <IconDelete />,
+          priority: 'danger',
           onAction: () => this.handleDeleteQuery(eventView),
         },
       ];