fix: Add icons to menu items (#2373)

Co-authored-by: Tom Moor <tom.moor@gmail.com>
2021-08-14 02:51:25 +05:30
parent 1db31eed41
commit 2c52a8cb8b
13 changed files with 242 additions and 162 deletions
--- a/app/components/ContextMenu/MenuItem.js
+++ b/app/components/ContextMenu/MenuItem.js
@ -4,6 +4,7 @@ import * as React from "react";
 import { MenuItem as BaseMenuItem } from "reakit/Menu";
 import styled from "styled-components";
 import breakpoint from "styled-components-breakpoint";
+import MenuIconWrapper from "../MenuIconWrapper";

 type Props = {|
  onClick?: (SyntheticEvent<>) => void | Promise<void>,
@ -16,6 +17,7 @@ type Props = {|
  as?: string | React.ComponentType<*>,
  hide?: () => void,
  level?: number,
+  icon?: React.Node,
 |};

 const MenuItem = ({
@ -25,6 +27,7 @@ const MenuItem = ({
  disabled,
  as,
  hide,
+  icon,
  ...rest
 }: Props) => {
  const handleClick = React.useCallback(
@ -71,6 +74,7 @@ const MenuItem = ({
              &nbsp;
            </>
          )}
+          {icon && <MenuIconWrapper>{icon}</MenuIconWrapper>}
          {children}
        </MenuAnchor>
      )}
@ -130,8 +134,8 @@ export const MenuAnchor = styled.a`
  `};

  ${breakpoint("tablet")`
-    padding: ${(props) => (props.$toggleable ? "4px 12px" : "6px 12px")};
-    font-size: 15px;
+    padding: 4px 12px;
+    font-size: 14px;
  `};
 `;

--- a/app/components/ContextMenu/Template.js
+++ b/app/components/ContextMenu/Template.js
@ -9,6 +9,8 @@ import {
  MenuItem as BaseMenuItem,
 } from "reakit/Menu";
 import styled from "styled-components";
+import Flex from "components/Flex";
+import MenuIconWrapper from "components/MenuIconWrapper";
 import Header from "./Header";
 import MenuItem, { MenuAnchor } from "./MenuItem";
 import Separator from "./Separator";
@ -67,7 +69,15 @@ export function filterTemplateItems(items: TMenuItem[]): TMenuItem[] {
 }

 function Template({ items, ...menu }: Props): React.Node {
-  return filterTemplateItems(items).map((item, index) => {
+  const filteredTemplates = filterTemplateItems(items);
+  const iconIsPresentInAnyMenuItem = filteredTemplates.find(
+    (item) => !!item.icon
+  );
+
+  return filteredTemplates.map((item, index) => {
+    if (iconIsPresentInAnyMenuItem)
+      item.icon = item.icon ? item.icon : <MenuIconWrapper />;
+
    if (item.to) {
      return (
        <MenuItem
@ -76,6 +86,7 @@ function Template({ items, ...menu }: Props): React.Node {
          key={index}
          disabled={item.disabled}
          selected={item.selected}
+          icon={item.icon}
          {...menu}
        >
          {item.title}
@ -92,6 +103,7 @@ function Template({ items, ...menu }: Props): React.Node {
          selected={item.selected}
          level={item.level}
          target={item.href.startsWith("#") ? undefined : "_blank"}
+          icon={item.icon}
          {...menu}
        >
          {item.title}
@ -107,6 +119,7 @@ function Template({ items, ...menu }: Props): React.Node {
          disabled={item.disabled}
          selected={item.selected}
          key={index}
+          icon={item.icon}
          {...menu}
        >
          {item.title}
@ -120,7 +133,7 @@ function Template({ items, ...menu }: Props): React.Node {
          key={index}
          as={Submenu}
          templateItems={item.items}
-          title={item.title}
+          title={<Title title={item.title} icon={item.icon} />}
          {...menu}
        />
      );
@ -139,4 +152,13 @@ function Template({ items, ...menu }: Props): React.Node {
  });
 }

+function Title({ title, icon }) {
+  return (
+    <Flex align="center">
+      {icon && <MenuIconWrapper>{icon}</MenuIconWrapper>}
+      {title}
+    </Flex>
+  );
+}
+
 export default React.memo<Props>(Template);
--- a/app/components/MenuIconWrapper.js
+++ b/app/components/MenuIconWrapper.js
@ -0,0 +1,11 @@
+// @flow
+import styled from "styled-components";
+
+const MenuIconWrapper = styled.span`
+  width: 24px;
+  height: 24px;
+  margin-right: 6px;
+  margin-left: -4px;
+`;
+
+export default MenuIconWrapper;
--- a/app/menus/AccountMenu.js
+++ b/app/menus/AccountMenu.js
@ -1,23 +1,19 @@
 // @flow
 import { observer } from "mobx-react";
-import { SunIcon, MoonIcon } from "outline-icons";
+import { MoonIcon, SunIcon } from "outline-icons";
 import * as React from "react";
 import { useTranslation } from "react-i18next";
-import { Link } from "react-router-dom";
-import { useMenuState, MenuButton } from "reakit/Menu";
-import styled from "styled-components";
+import { MenuButton, useMenuState } from "reakit/Menu";
 import {
-  developers,
  changelog,
+  developers,
  githubIssuesUrl,
  mailToUrl,
  settings,
 } from "shared/utils/routeHelpers";
 import KeyboardShortcuts from "scenes/KeyboardShortcuts";
 import ContextMenu from "components/ContextMenu";
-import MenuItem, { MenuAnchor } from "components/ContextMenu/MenuItem";
-import Separator from "components/ContextMenu/Separator";
-import Flex from "components/Flex";
+import Template from "components/ContextMenu/Template";
 import Guide from "components/Guide";
 import useBoolean from "hooks/useBoolean";
 import usePrevious from "hooks/usePrevious";
@ -27,50 +23,6 @@ type Props = {|
  children: (props: any) => React.Node,
 |};

-const AppearanceMenu = React.forwardRef((props, ref) => {
-  const { ui } = useStores();
-  const { t } = useTranslation();
-  const menu = useMenuState();
-
-  return (
-    <>
-      <MenuButton ref={ref} {...menu} {...props} onClick={menu.show}>
-        {(props) => (
-          <MenuAnchor {...props}>
-            <ChangeTheme justify="space-between">
-              {t("Appearance")}
-              {ui.resolvedTheme === "light" ? <SunIcon /> : <MoonIcon />}
-            </ChangeTheme>
-          </MenuAnchor>
-        )}
-      </MenuButton>
-      <ContextMenu {...menu} aria-label={t("Appearance")}>
-        <MenuItem
-          {...menu}
-          onClick={() => ui.setTheme("system")}
-          selected={ui.theme === "system"}
-        >
-          {t("System")}
-        </MenuItem>
-        <MenuItem
-          {...menu}
-          onClick={() => ui.setTheme("light")}
-          selected={ui.theme === "light"}
-        >
-          {t("Light")}
-        </MenuItem>
-        <MenuItem
-          {...menu}
-          onClick={() => ui.setTheme("dark")}
-          selected={ui.theme === "dark"}
-        >
-          {t("Dark")}
-        </MenuItem>
-      </ContextMenu>
-    </>
-  );
-});
-
 function AccountMenu(props: Props) {
  const menu = useMenuState({
    unstable_offset: [8, 0],
@ -92,6 +44,67 @@ function AccountMenu(props: Props) {
    }
  }, [menu, ui.theme, previousTheme]);

+  const items = React.useMemo(
+    () => [
+      {
+        title: t("Settings"),
+        to: settings(),
+      },
+      {
+        title: t("Keyboard shortcuts"),
+        onClick: handleKeyboardShortcutsOpen,
+      },
+      {
+        title: t("API documentation"),
+        href: developers(),
+      },
+      {
+        type: "separator",
+      },
+      {
+        title: t("Changelog"),
+        href: changelog(),
+      },
+      {
+        title: t("Send us feedback"),
+        href: mailToUrl(),
+      },
+      {
+        title: t("Report a bug"),
+        href: githubIssuesUrl(),
+      },
+      {
+        title: t("Appearance"),
+        icon: ui.resolvedTheme === "light" ? <SunIcon /> : <MoonIcon />,
+        items: [
+          {
+            title: t("System"),
+            onClick: () => ui.setTheme("system"),
+            selected: ui.theme === "system",
+          },
+          {
+            title: t("Light"),
+            onClick: () => ui.setTheme("light"),
+            selected: ui.theme === "light",
+          },
+          {
+            title: t("Dark"),
+            onClick: () => ui.setTheme("dark"),
+            selected: ui.theme === "dark",
+          },
+        ],
+      },
+      {
+        type: "separator",
+      },
+      {
+        title: t("Log out"),
+        onClick: auth.logout,
+      },
+    ],
+    [auth.logout, handleKeyboardShortcutsOpen, t, ui]
+  );
+
  return (
    <>
      <Guide
@ -103,38 +116,10 @@ function AccountMenu(props: Props) {
      </Guide>
      <MenuButton {...menu}>{props.children}</MenuButton>
      <ContextMenu {...menu} aria-label={t("Account")}>
-        <MenuItem {...menu} as={Link} to={settings()}>
-          {t("Settings")}
-        </MenuItem>
-        <MenuItem {...menu} onClick={handleKeyboardShortcutsOpen}>
-          {t("Keyboard shortcuts")}
-        </MenuItem>
-        <MenuItem {...menu} href={developers()} target="_blank">
-          {t("API documentation")}
-        </MenuItem>
-        <Separator {...menu} />
-        <MenuItem {...menu} href={changelog()} target="_blank">
-          {t("Changelog")}
-        </MenuItem>
-        <MenuItem {...menu} href={mailToUrl()} target="_blank">
-          {t("Send us feedback")}
-        </MenuItem>
-        <MenuItem {...menu} href={githubIssuesUrl()} target="_blank">
-          {t("Report a bug")}
-        </MenuItem>
-        <Separator {...menu} />
-        <MenuItem {...menu} as={AppearanceMenu} />
-        <Separator {...menu} />
-        <MenuItem {...menu} onClick={auth.logout}>
-          {t("Log out")}
-        </MenuItem>
+        <Template {...menu} items={items} />
      </ContextMenu>
    </>
  );
 }

-const ChangeTheme = styled(Flex)`
-  width: 100%;
-`;
-
 export default observer(AccountMenu);
--- a/app/menus/CollectionMenu.js
+++ b/app/menus/CollectionMenu.js
@ -1,5 +1,13 @@
 // @flow
 import { observer } from "mobx-react";
+import {
+  NewDocumentIcon,
+  EditIcon,
+  TrashIcon,
+  ImportIcon,
+  ExportIcon,
+  PadlockIcon,
+} from "outline-icons";
 import * as React from "react";
 import { useTranslation } from "react-i18next";
 import { useHistory } from "react-router-dom";
@ -12,7 +20,7 @@ import CollectionExport from "scenes/CollectionExport";
 import CollectionPermissions from "scenes/CollectionPermissions";
 import ContextMenu from "components/ContextMenu";
 import OverflowMenuButton from "components/ContextMenu/OverflowMenuButton";
-import Template, { filterTemplateItems } from "components/ContextMenu/Template";
+import Template from "components/ContextMenu/Template";
 import Modal from "components/Modal";
 import useStores from "hooks/useStores";
 import useToasts from "hooks/useToasts";
@ -113,17 +121,18 @@ function CollectionMenu({

  const can = policies.abilities(collection.id);
  const items = React.useMemo(
-    () =>
-      filterTemplateItems([
+    () => [
      {
        title: t("New document"),
        visible: can.update,
        onClick: handleNewDocument,
+        icon: <NewDocumentIcon />,
      },
      {
        title: t("Import document"),
        visible: can.update,
        onClick: handleImportDocument,
+        icon: <ImportIcon />,
      },
      {
        type: "separator",
@ -132,16 +141,19 @@ function CollectionMenu({
        title: `${t("Edit")}…`,
        visible: can.update,
        onClick: () => setShowCollectionEdit(true),
+        icon: <EditIcon />,
      },
      {
        title: `${t("Permissions")}…`,
        visible: can.update,
        onClick: () => setShowCollectionPermissions(true),
+        icon: <PadlockIcon />,
      },
      {
        title: `${t("Export")}…`,
        visible: !!(collection && can.export),
        onClick: () => setShowCollectionExport(true),
+        icon: <ExportIcon />,
      },
      {
        type: "separator",
@ -150,8 +162,9 @@ function CollectionMenu({
        title: `${t("Delete")}…`,
        visible: !!(collection && can.delete),
        onClick: () => setShowCollectionDelete(true),
+        icon: <TrashIcon />,
      },
-      ]),
+    ],
    [can, collection, handleNewDocument, handleImportDocument, t]
  );

--- a/app/menus/DocumentMenu.js
+++ b/app/menus/DocumentMenu.js
@ -1,5 +1,25 @@
 // @flow
 import { observer } from "mobx-react";
+import {
+  EditIcon,
+  PinIcon,
+  StarredIcon,
+  UnstarredIcon,
+  DuplicateIcon,
+  ArchiveIcon,
+  TrashIcon,
+  MoveIcon,
+  HistoryIcon,
+  UnpublishIcon,
+  ShapesIcon,
+  PrintIcon,
+  ImportIcon,
+  NewDocumentIcon,
+  DownloadIcon,
+  BuildingBlocksIcon,
+  RestoreIcon,
+  CrossIcon,
+} from "outline-icons";
 import * as React from "react";
 import { useTranslation } from "react-i18next";
 import { useHistory } from "react-router-dom";
@ -248,6 +268,7 @@ function DocumentMenu({
              title: t("Restore"),
              visible: (!!collection && can.restore) || can.unarchive,
              onClick: handleRestore,
+              icon: <RestoreIcon />,
            },
            {
              title: t("Restore"),
@ -258,6 +279,7 @@ function DocumentMenu({
                position: "relative",
                top: -40,
              },
+              icon: <RestoreIcon />,
              hover: true,
              items: [
                {
@ -271,86 +293,102 @@ function DocumentMenu({
              title: t("Unpin"),
              onClick: document.unpin,
              visible: !!(showPin && document.pinned && can.unpin),
+              icon: <PinIcon />,
            },
            {
              title: t("Pin to collection"),
              onClick: document.pin,
              visible: !!(showPin && !document.pinned && can.pin),
+              icon: <PinIcon />,
            },
            {
              title: t("Unstar"),
              onClick: handleUnstar,
              visible: document.isStarred && !!can.unstar,
+              icon: <UnstarredIcon />,
            },
            {
              title: t("Star"),
              onClick: handleStar,
              visible: !document.isStarred && !!can.star,
-            },
-            {
-              title: t("Enable embeds"),
-              onClick: document.enableEmbeds,
-              visible: !!showToggleEmbeds && document.embedsDisabled,
-            },
-            {
-              title: t("Disable embeds"),
-              onClick: document.disableEmbeds,
-              visible: !!showToggleEmbeds && !document.embedsDisabled,
+              icon: <StarredIcon />,
            },
            {
              type: "separator",
            },
+            {
+              title: t("Edit"),
+              to: editDocumentUrl(document),
+              visible: !!can.update,
+              icon: <EditIcon />,
+            },
            {
              title: t("New nested document"),
              to: newDocumentUrl(document.collectionId, {
                parentDocumentId: document.id,
              }),
              visible: !!can.createChildDocument,
+              icon: <NewDocumentIcon />,
            },
            {
              title: t("Import document"),
              visible: can.createChildDocument,
              onClick: handleImportDocument,
+              icon: <ImportIcon />,
            },
            {
              title: `${t("Create template")}…`,
              onClick: () => setShowTemplateModal(true),
              visible: !!can.update && !document.isTemplate,
-            },
-            {
-              title: t("Edit"),
-              to: editDocumentUrl(document),
-              visible: !!can.update,
+              icon: <ShapesIcon />,
            },
            {
              title: t("Duplicate"),
              onClick: handleDuplicate,
              visible: !!can.update,
+              icon: <DuplicateIcon />,
            },
            {
              title: t("Unpublish"),
              onClick: handleUnpublish,
              visible: !!can.unpublish,
+              icon: <UnpublishIcon />,
            },
            {
              title: t("Archive"),
              onClick: handleArchive,
              visible: !!can.archive,
+              icon: <ArchiveIcon />,
            },
            {
              title: `${t("Delete")}…`,
              onClick: () => setShowDeleteModal(true),
              visible: !!can.delete,
+              icon: <TrashIcon />,
            },
            {
              title: `${t("Permanently delete")}…`,
              onClick: () => setShowPermanentDeleteModal(true),
              visible: can.permanentDelete,
+              icon: <CrossIcon />,
            },
            {
              title: `${t("Move")}…`,
              onClick: () => setShowMoveModal(true),
              visible: !!can.move,
+              icon: <MoveIcon />,
+            },
+            {
+              title: t("Enable embeds"),
+              onClick: document.enableEmbeds,
+              visible: !!showToggleEmbeds && document.embedsDisabled,
+              icon: <BuildingBlocksIcon />,
+            },
+            {
+              title: t("Disable embeds"),
+              onClick: document.disableEmbeds,
+              visible: !!showToggleEmbeds && !document.embedsDisabled,
+              icon: <BuildingBlocksIcon />,
            },
            {
              type: "separator",
@ -361,16 +399,19 @@ function DocumentMenu({
                ? documentUrl(document)
                : documentHistoryUrl(document),
              visible: canViewHistory,
+              icon: <HistoryIcon />,
            },
            {
              title: t("Download"),
              onClick: document.download,
              visible: !!can.download,
+              icon: <DownloadIcon />,
            },
            {
              title: t("Print"),
              onClick: handlePrint,
              visible: !!showPrint,
+              icon: <PrintIcon />,
            },
          ]}
        />
--- a/app/menus/NewDocumentMenu.js
+++ b/app/menus/NewDocumentMenu.js
@ -11,7 +11,6 @@ import CollectionIcon from "components/CollectionIcon";
 import ContextMenu from "components/ContextMenu";
 import Header from "components/ContextMenu/Header";
 import Template from "components/ContextMenu/Template";
-import Flex from "components/Flex";
 import useCurrentTeam from "hooks/useCurrentTeam";
 import useStores from "hooks/useStores";
 import { newDocumentUrl } from "utils/routeHelpers";
@ -31,12 +30,8 @@ function NewDocumentMenu() {
        if (can.update) {
          filtered.push({
            to: newDocumentUrl(collection.id),
-            title: (
-              <Flex align="center">
-                <CollectionIcon collection={collection} />
-                <CollectionName>{collection.name}</CollectionName>
-              </Flex>
-            ),
+            title: <CollectionName>{collection.name}</CollectionName>,
+            icon: <CollectionIcon collection={collection} />,
          });
        }
        return filtered;
--- a/app/menus/NewTemplateMenu.js
+++ b/app/menus/NewTemplateMenu.js
@ -3,14 +3,13 @@ import { observer } from "mobx-react";
 import { PlusIcon } from "outline-icons";
 import * as React from "react";
 import { useTranslation } from "react-i18next";
-import { useMenuState, MenuButton } from "reakit/Menu";
+import { MenuButton, useMenuState } from "reakit/Menu";
 import styled from "styled-components";
 import Button from "components/Button";
 import CollectionIcon from "components/CollectionIcon";
 import ContextMenu from "components/ContextMenu";
 import Header from "components/ContextMenu/Header";
 import Template from "components/ContextMenu/Template";
-import Flex from "components/Flex";
 import useCurrentTeam from "hooks/useCurrentTeam";
 import useStores from "hooks/useStores";
 import { newDocumentUrl } from "utils/routeHelpers";
@ -29,12 +28,8 @@ function NewTemplateMenu() {
        if (can.update) {
          filtered.push({
            to: newDocumentUrl(collection.id, { template: true }),
-            title: (
-              <Flex align="center">
-                <CollectionIcon collection={collection} />
-                <CollectionName>{collection.name}</CollectionName>
-              </Flex>
-            ),
+            title: <CollectionName>{collection.name}</CollectionName>,
+            icon: <CollectionIcon collection={collection} />,
          });
        }
        return filtered;
--- a/app/menus/RevisionMenu.js
+++ b/app/menus/RevisionMenu.js
@ -1,5 +1,6 @@
 // @flow
 import { observer } from "mobx-react";
+import { RestoreIcon, LinkIcon } from "outline-icons";
 import * as React from "react";
 import { useTranslation } from "react-i18next";
 import { useHistory } from "react-router-dom";
@ -10,6 +11,7 @@ import MenuItem from "components/ContextMenu/MenuItem";
 import OverflowMenuButton from "components/ContextMenu/OverflowMenuButton";
 import Separator from "components/ContextMenu/Separator";
 import CopyToClipboard from "components/CopyToClipboard";
+import MenuIconWrapper from "components/MenuIconWrapper";
 import useToasts from "hooks/useToasts";
 import { documentHistoryUrl } from "utils/routeHelpers";

@ -54,11 +56,19 @@ function RevisionMenu({ document, revisionId, className }: Props) {
      />
      <ContextMenu {...menu} aria-label={t("Revision options")}>
        <MenuItem {...menu} onClick={handleRestore}>
+          <MenuIconWrapper>
+            <RestoreIcon />
+          </MenuIconWrapper>
          {t("Restore version")}
        </MenuItem>
        <Separator />
        <CopyToClipboard text={url} onCopy={handleCopy}>
-          <MenuItem {...menu}>{t("Copy link")}</MenuItem>
+          <MenuItem {...menu}>
+            <MenuIconWrapper>
+              <LinkIcon />
+            </MenuIconWrapper>
+            {t("Copy link")}
+          </MenuItem>
        </CopyToClipboard>
      </ContextMenu>
    </>
--- a/app/menus/TemplatesMenu.js
+++ b/app/menus/TemplatesMenu.js
@ -37,9 +37,9 @@ function TemplatesMenu({ document }: Props) {
    <MenuItem
      key={template.id}
      onClick={() => document.updateFromTemplate(template)}
+      icon={<DocumentIcon />}
      {...menu}
    >
-      <DocumentIcon />
      <TemplateItem>
        <strong>{template.titleWithDefault}</strong>
        <br />
--- a/app/types/index.js
+++ b/app/types/index.js
@ -66,6 +66,7 @@ export type MenuItem =
      visible?: boolean,
      selected?: boolean,
      disabled?: boolean,
+      icon?: React.Node,
    |}
  | {|
      title: React.Node,
@ -73,6 +74,7 @@ export type MenuItem =
      visible?: boolean,
      selected?: boolean,
      disabled?: boolean,
+      icon?: React.Node,
    |}
  | {|
      title: React.Node,
@ -81,6 +83,7 @@ export type MenuItem =
      selected?: boolean,
      disabled?: boolean,
      level?: number,
+      icon?: React.Node,
    |}
  | {|
      title: React.Node,
@ -89,6 +92,7 @@ export type MenuItem =
      style?: Object,
      hover?: boolean,
      items: MenuItem[],
+      icon?: React.Node,
    |}
  | {|
      type: "separator",
--- a/shared/i18n/locales/en_US/translation.json
+++ b/shared/i18n/locales/en_US/translation.json
@ -167,14 +167,14 @@
  "Previous page": "Previous page",
  "Next page": "Next page",
  "Could not import file": "Could not import file",
-  "Appearance": "Appearance",
-  "System": "System",
-  "Light": "Light",
-  "Dark": "Dark",
  "API documentation": "API documentation",
  "Changelog": "Changelog",
  "Send us feedback": "Send us feedback",
  "Report a bug": "Report a bug",
+  "Appearance": "Appearance",
+  "System": "System",
+  "Light": "Light",
+  "Dark": "Dark",
  "Log out": "Log out",
  "Show path to document": "Show path to document",
  "Path to document": "Path to document",
--- a/shared/theme.js
+++ b/shared/theme.js
@ -10,7 +10,7 @@ const colors = {

  slate: "#9BA6B2",
  slateLight: "#DAE1E9",
-  slateDark: "#4E5C6E",
+  slateDark: "#394351",

  smoke: "#F4F7FA",
  smokeLight: "#F9FBFC",