From 2c52a8cb8bfc8d97ce34c1ef5d1df9082d5c2e74 Mon Sep 17 00:00:00 2001 From: Saumya Pandey Date: Sat, 14 Aug 2021 02:51:25 +0530 Subject: [PATCH] fix: Add icons to menu items (#2373) Co-authored-by: Tom Moor --- app/components/ContextMenu/MenuItem.js | 8 +- app/components/ContextMenu/Template.js | 26 +++- app/components/MenuIconWrapper.js | 11 ++ app/menus/AccountMenu.js | 147 +++++++++------------ app/menus/CollectionMenu.js | 93 +++++++------ app/menus/DocumentMenu.js | 71 +++++++--- app/menus/NewDocumentMenu.js | 9 +- app/menus/NewTemplateMenu.js | 11 +- app/menus/RevisionMenu.js | 12 +- app/menus/TemplatesMenu.js | 2 +- app/types/index.js | 4 + shared/i18n/locales/en_US/translation.json | 8 +- shared/theme.js | 2 +- 13 files changed, 242 insertions(+), 162 deletions(-) create mode 100644 app/components/MenuIconWrapper.js diff --git a/app/components/ContextMenu/MenuItem.js b/app/components/ContextMenu/MenuItem.js index 3448103e..357c3e68 100644 --- 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, @@ -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 = ({   )} + {icon && {icon}} {children} )} @@ -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; `}; `; diff --git a/app/components/ContextMenu/Template.js b/app/components/ContextMenu/Template.js index a1f8c511..9da4c97f 100644 --- 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 : ; + if (item.to) { return ( {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={} {...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); diff --git a/app/components/MenuIconWrapper.js b/app/components/MenuIconWrapper.js new file mode 100644 index 00000000..03b1cca1 --- /dev/null +++ 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; diff --git a/app/menus/AccountMenu.js b/app/menus/AccountMenu.js index 822a39d1..1a879218 100644 --- 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); diff --git a/app/menus/CollectionMenu.js b/app/menus/CollectionMenu.js index b6cb5676..f2135503 100644 --- 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,45 +121,50 @@ function CollectionMenu({ const can = policies.abilities(collection.id); const items = React.useMemo( - () => - filterTemplateItems([ - { - title: t("New document"), - visible: can.update, - onClick: handleNewDocument, - }, - { - title: t("Import document"), - visible: can.update, - onClick: handleImportDocument, - }, - { - type: "separator", - }, - { - title: `${t("Edit")}…`, - visible: can.update, - onClick: () => setShowCollectionEdit(true), - }, - { - title: `${t("Permissions")}…`, - visible: can.update, - onClick: () => setShowCollectionPermissions(true), - }, - { - title: `${t("Export")}…`, - visible: !!(collection && can.export), - onClick: () => setShowCollectionExport(true), - }, - { - type: "separator", - }, - { - title: `${t("Delete")}…`, - visible: !!(collection && can.delete), - onClick: () => setShowCollectionDelete(true), - }, - ]), + () => [ + { + title: t("New document"), + visible: can.update, + onClick: handleNewDocument, + icon: <NewDocumentIcon />, + }, + { + title: t("Import document"), + visible: can.update, + onClick: handleImportDocument, + icon: <ImportIcon />, + }, + { + type: "separator", + }, + { + 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", + }, + { + title: `${t("Delete")}…`, + visible: !!(collection && can.delete), + onClick: () => setShowCollectionDelete(true), + icon: <TrashIcon />, + }, + ], [can, collection, handleNewDocument, handleImportDocument, t] ); diff --git a/app/menus/DocumentMenu.js b/app/menus/DocumentMenu.js index 528941ae..8b6fc59b 100644 --- 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 />, }, ]} /> diff --git a/app/menus/NewDocumentMenu.js b/app/menus/NewDocumentMenu.js index fbc34363..1b17e6d8 100644 --- 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; diff --git a/app/menus/NewTemplateMenu.js b/app/menus/NewTemplateMenu.js index c3aab464..bbc0f39c 100644 --- 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; diff --git a/app/menus/RevisionMenu.js b/app/menus/RevisionMenu.js index 1da8e43f..5e680321 100644 --- 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> </> diff --git a/app/menus/TemplatesMenu.js b/app/menus/TemplatesMenu.js index 163fcefe..311530fc 100644 --- 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 /> diff --git a/app/types/index.js b/app/types/index.js index afa96865..518e0969 100644 --- 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", diff --git a/shared/i18n/locales/en_US/translation.json b/shared/i18n/locales/en_US/translation.json index 5071471e..c1d63440 100644 --- 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", diff --git a/shared/theme.js b/shared/theme.js index 011c6634..56f51e87 100644 --- 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",