From 8f3d135caac393cf1aeec5e2a0831e6efc012f73 Mon Sep 17 00:00:00 2001
From: nikec <43032218+niikeec@users.noreply.github.com>
Date: Wed, 29 Mar 2023 12:54:02 +0200
Subject: [PATCH] [Desktop] Fix menu colors and arrow highlighting (#639)

* [Desktop] Context Menu cleanup

* [Desktop] Update dropdown with new menu changes

* [UI] Change dark menu line color

* [Desktop] remove menu animate prop and make it a default

* Remove duplicate classes
---
 .../Layout/Sidebar/LibrariesDropdown.tsx      |  5 +-
 packages/ui/src/ContextMenu.tsx               | 88 +++++++++--------
 packages/ui/src/DropdownMenu.tsx              | 99 ++++++-------------
 packages/ui/style/colors.scss                 |  6 +-
 4 files changed, 79 insertions(+), 119 deletions(-)

diff --git a/interface/app/$libraryId/Layout/Sidebar/LibrariesDropdown.tsx b/interface/app/$libraryId/Layout/Sidebar/LibrariesDropdown.tsx
index 855f8e61b..153308630 100644
--- a/interface/app/$libraryId/Layout/Sidebar/LibrariesDropdown.tsx
+++ b/interface/app/$libraryId/Layout/Sidebar/LibrariesDropdown.tsx
@@ -27,9 +27,8 @@ export default () => {
 			}
 			// we override the sidebar dropdown item's hover styles
 			// because the dark style clashes with the sidebar
-			className="dark:bg-sidebar-box dark:border-sidebar-line dark:divide-menu-selected/30 mt-1 shadow-none"
+			className="dark:bg-sidebar-box dark:border-sidebar-line dark:divide-menu-selected/30 data-[side=bottom]:slide-in-from-top-2 mt-1 shadow-none"
 			alignToTrigger
-			animate
 		>
 			{libraries.data?.map((lib) => (
 				<DropdownMenu.Item
@@ -40,7 +39,7 @@ export default () => {
 					{lib.config.name}
 				</DropdownMenu.Item>
 			))}
-			<DropdownMenu.Separator className="mx-0 my-0.5" />
+			<DropdownMenu.Separator className="mx-0" />
 			<DropdownMenu.Item
 				label="	New Library"
 				icon={Plus}
diff --git a/packages/ui/src/ContextMenu.tsx b/packages/ui/src/ContextMenu.tsx
index cf50db5f4..d6f4c1dd7 100644
--- a/packages/ui/src/ContextMenu.tsx
+++ b/packages/ui/src/ContextMenu.tsx
@@ -8,21 +8,21 @@ interface ContextMenuProps extends RadixCM.MenuContentProps {
 	trigger: React.ReactNode;
 }
 
-export const contextMenuClasses = clsx(
-	'z-50 flex flex-col',
-	'my-2 min-w-[8rem] px-1 py-0.5',
-	'text-menu-ink text-left text-sm',
+export const contextMenuClassNames = clsx(
+	'z-50 max-h-[calc(100vh-20px)] overflow-y-auto',
+	'my-2 min-w-[12rem] max-w-[16rem] px-1 py-0.5',
 	'bg-menu cool-shadow',
 	'border-menu-line border',
-	'cursor-default select-none rounded-md'
+	'cursor-default select-none rounded-md',
+	'animate-in fade-in'
 );
 
-const Root = ({ trigger, children, className, ...props }: PropsWithChildren<ContextMenuProps>) => {
+const Root = ({ trigger, children, className, ...props }: ContextMenuProps) => {
 	return (
 		<RadixCM.Root>
 			<RadixCM.Trigger asChild>{trigger}</RadixCM.Trigger>
 			<RadixCM.Portal>
-				<RadixCM.Content {...props} className={clsx(contextMenuClasses, className)}>
+				<RadixCM.Content className={clsx(contextMenuClassNames, className)} {...props}>
 					{children}
 				</RadixCM.Content>
 			</RadixCM.Portal>
@@ -30,16 +30,12 @@ const Root = ({ trigger, children, className, ...props }: PropsWithChildren<Cont
 	);
 };
 
-export const contextMenuSeparatorClassNames =
-	'border-b-menu-line pointer-events-none mx-2 my-1 border-0 border-b';
+export const contextMenuSeparatorClassNames = 'border-b-menu-line my-0.5 border-b';
 
 const Separator = (props: { className?: string }) => (
 	<RadixCM.Separator className={clsx(contextMenuSeparatorClassNames, props.className)} />
 );
 
-export const contextSubMenuTriggerClassNames =
-	"[&[data-state='open']_div]:bg-accent text-menu-ink py-[3px]  focus:outline-none [&[data-state='open']_div]:text-white";
-
 const SubMenu = ({
 	label,
 	icon,
@@ -48,33 +44,37 @@ const SubMenu = ({
 }: RadixCM.MenuSubContentProps & ContextMenuItemProps) => {
 	return (
 		<RadixCM.Sub>
-			<RadixCM.SubTrigger className={contextSubMenuTriggerClassNames}>
-				<DivItem rightArrow {...{ label, icon }} />
+			<RadixCM.SubTrigger className={contextMenuItemClassNames}>
+				<ContextMenuDivItem rightArrow {...{ label, icon }} />
 			</RadixCM.SubTrigger>
 			<RadixCM.Portal>
 				<Suspense fallback={null}>
-					<RadixCM.SubContent {...props} className={clsx(contextMenuClasses, '-mt-2', className)} />
+					<RadixCM.SubContent
+						className={clsx(contextMenuClassNames, '-mt-2', className)}
+						{...props}
+					/>
 				</Suspense>
 			</RadixCM.Portal>
 		</RadixCM.Sub>
 	);
 };
 
-export const contextMenuItemStyles = cva(
+const contextMenuItemStyles = cva(
 	[
-		'flex flex-1 flex-row items-center justify-start overflow-hidden',
-		'space-x-2 px-2 py-[3px]',
-		'cursor-default rounded',
-		'focus:outline-none'
+		'flex h-[26px] items-center space-x-2 overflow-hidden rounded px-2',
+		'text-ink text-sm',
+		'group-radix-highlighted:text-white dark:group-radix-highlighted:text-ink',
+		'group-radix-disabled:text-ink/50 group-radix-disabled:pointer-events-none',
+		'group-radix-state-open:bg-accent group-radix-state-open:text-white dark:group-radix-state-open:text-ink'
 	],
 	{
 		variants: {
 			variant: {
-				default: 'hover:bg-accent focus:bg-accent hover:text-white',
+				default: 'group-radix-highlighted:bg-accent',
 				danger: [
 					'text-red-600 dark:text-red-400',
-					'hover:text-white focus:text-white',
-					'hover:bg-red-500 focus:bg-red-500'
+					'group-radix-highlighted:text-white',
+					'group-radix-highlighted:bg-red-500'
 				]
 			}
 		},
@@ -92,6 +92,8 @@ export interface ContextMenuItemProps extends VariantProps<typeof contextMenuIte
 	keybind?: string;
 }
 
+export const contextMenuItemClassNames = 'group py-0.5 outline-none';
+
 const Item = ({
 	icon,
 	label,
@@ -99,46 +101,46 @@ const Item = ({
 	children,
 	keybind,
 	variant,
+	iconProps,
 	...props
 }: ContextMenuItemProps & RadixCM.MenuItemProps) => {
 	return (
-		<RadixCM.Item {...props} className="">
-			<div className={contextMenuItemStyles({ variant })}>
-				{children ? children : <ItemInternals {...{ icon, label, rightArrow, keybind }} />}
-			</div>
+		<RadixCM.Item className={contextMenuItemClassNames} {...props}>
+			<ContextMenuDivItem {...{ icon, iconProps, label, rightArrow, keybind, variant, children }} />
 		</RadixCM.Item>
 	);
 };
 
-const DivItem = ({ variant, ...props }: ContextMenuItemProps) => (
-	<div className={contextMenuItemStyles({ variant })}>
-		<ItemInternals {...props} />
+export const ContextMenuDivItem = ({
+	variant,
+	children,
+	className,
+	...props
+}: PropsWithChildren<ContextMenuItemProps & { className?: string }>) => (
+	<div className={contextMenuItemStyles({ variant, className })}>
+		{children || <ItemInternals {...props} />}
 	</div>
 );
 
-export const ItemInternals = ({
-	icon,
-	label,
-	rightArrow,
-	keybind,
-	iconProps
-}: ContextMenuItemProps) => {
+const ItemInternals = ({ icon, label, rightArrow, keybind, iconProps }: ContextMenuItemProps) => {
 	const ItemIcon = icon;
+
 	return (
 		<>
 			{ItemIcon && <ItemIcon size={18} {...iconProps} />}
-			{label && <p>{label}</p>}
+			{label && <span className="flex-1 truncate">{label}</span>}
 
 			{keybind && (
-				<span className="flex-end text-menu-faint absolute right-3 text-xs font-medium group-hover:text-white">
+				<span className="text-menu-faint group-radix-highlighted:text-white text-xs font-medium">
 					{keybind}
 				</span>
 			)}
 			{rightArrow && (
-				<>
-					<div className="flex-1" />
-					<CaretRight weight="fill" size={12} alt="" className="text-menu-faint" />
-				</>
+				<CaretRight
+					weight="fill"
+					size={12}
+					className="text-menu-faint group-radix-highlighted:text-white group-radix-state-open:text-white"
+				/>
 			)}
 		</>
 	);
diff --git a/packages/ui/src/DropdownMenu.tsx b/packages/ui/src/DropdownMenu.tsx
index 9340f51f8..f049b9f8b 100644
--- a/packages/ui/src/DropdownMenu.tsx
+++ b/packages/ui/src/DropdownMenu.tsx
@@ -3,21 +3,17 @@ import clsx from 'clsx';
 import React, { PropsWithChildren, Suspense, useCallback, useRef, useState } from 'react';
 import { Link } from 'react-router-dom';
 import {
+	ContextMenuDivItem,
 	ContextMenuItemProps,
-	ItemInternals,
-	contextMenuClasses,
-	contextMenuItemStyles,
-	contextMenuSeparatorClassNames,
-	contextSubMenuTriggerClassNames
+	contextMenuClassNames,
+	contextMenuItemClassNames,
+	contextMenuSeparatorClassNames
 } from './ContextMenu';
 
-interface DropdownMenuProps
-	extends RadixDM.MenuContentProps,
-		Pick<RadixDM.DropdownMenuProps, 'onOpenChange'> {
+interface DropdownMenuProps extends RadixDM.MenuContentProps {
 	trigger: React.ReactNode;
 	triggerClassName?: string;
 	alignToTrigger?: boolean;
-	animate?: boolean;
 }
 
 const Root = ({
@@ -27,8 +23,6 @@ const Root = ({
 	asChild = true,
 	triggerClassName,
 	alignToTrigger,
-	onOpenChange,
-	animate,
 	...props
 }: PropsWithChildren<DropdownMenuProps>) => {
 	const [width, setWidth] = useState<number>();
@@ -38,28 +32,19 @@ const Root = ({
 	}, []);
 
 	return (
-		<RadixDM.Root modal={false} onOpenChange={onOpenChange}>
+		<RadixDM.Root>
 			<RadixDM.Trigger ref={measureRef} asChild={asChild} className={triggerClassName}>
 				{trigger}
 			</RadixDM.Trigger>
 			<RadixDM.Portal>
-				<div>
-					<div className="fixed inset-0"></div>
-					<RadixDM.Content
-						className={clsx(
-							contextMenuClasses,
-							animate && 'animate-in fade-in data-[side=bottom]:slide-in-from-top-2',
-							'w-44',
-							className
-						)}
-						align="start"
-						collisionPadding={5}
-						style={{ width }}
-						{...props}
-					>
-						{children}
-					</RadixDM.Content>
-				</div>
+				<RadixDM.Content
+					className={clsx(contextMenuClassNames, width && 'min-w-0', className)}
+					align="start"
+					style={{ width }}
+					{...props}
+				>
+					{children}
+				</RadixDM.Content>
 			</RadixDM.Portal>
 		</RadixDM.Root>
 	);
@@ -77,29 +62,19 @@ const SubMenu = ({
 }: RadixDM.MenuSubContentProps & ContextMenuItemProps) => {
 	return (
 		<RadixDM.Sub>
-			<RadixDM.SubTrigger className={contextSubMenuTriggerClassNames}>
-				<div
-					className={contextMenuItemStyles({
-						class: 'group-radix-state-open:bg-trinary/50 group-radix-state-open:text-primary'
-					})}
-				>
-					<ItemInternals rightArrow {...{ label, icon }} />
-				</div>
+			<RadixDM.SubTrigger className={contextMenuItemClassNames}>
+				<ContextMenuDivItem rightArrow {...{ label, icon }} />
 			</RadixDM.SubTrigger>
 			<RadixDM.Portal>
 				<Suspense fallback={null}>
-					<RadixDM.SubContent
-						className={clsx(contextMenuClasses, className)}
-						collisionPadding={5}
-						{...props}
-					/>
+					<RadixDM.SubContent className={clsx(contextMenuClassNames, className)} {...props} />
 				</Suspense>
 			</RadixDM.Portal>
 		</RadixDM.Sub>
 	);
 };
 
-interface DropdownItemProps extends ContextMenuItemProps, RadixDM.MenuItemProps {
+interface DropdownItemProps extends ContextMenuItemProps {
 	to?: string;
 	selected?: boolean;
 }
@@ -116,39 +91,23 @@ const Item = ({
 	selected,
 	to,
 	...props
-}: DropdownItemProps) => {
+}: DropdownItemProps & RadixDM.MenuItemProps) => {
 	const ref = useRef<HTMLDivElement>(null);
 
 	return (
-		<RadixDM.Item
-			className={clsx(
-				'text-menu-ink group cursor-default select-none py-0.5 focus:outline-none active:opacity-80',
-				className
-			)}
-			ref={ref}
-			{...props}
-		>
+		<RadixDM.Item ref={ref} className={clsx(contextMenuItemClassNames, className)} {...props}>
 			{to ? (
-				<Link
-					to={to}
-					className={contextMenuItemStyles({
-						variant,
-						className: clsx(selected && 'bg-accent')
-					})}
-					onClick={() => ref.current?.click()}
-				>
-					{children ? (
-						<span className="truncate">{children}</span>
-					) : (
-						<ItemInternals {...{ icon, iconProps, label, rightArrow, keybind }} />
-					)}
+				<Link to={to} onClick={() => ref.current?.click()}>
+					<ContextMenuDivItem
+						className={clsx(selected && 'bg-accent')}
+						{...{ icon, iconProps, label, rightArrow, keybind, variant, children }}
+					/>
 				</Link>
 			) : (
-				<div
-					className={contextMenuItemStyles({ variant, className: clsx(selected && 'bg-accent') })}
-				>
-					{children || <ItemInternals {...{ icon, iconProps, label, rightArrow, keybind }} />}
-				</div>
+				<ContextMenuDivItem
+					className={clsx(selected && 'bg-accent')}
+					{...{ icon, iconProps, label, rightArrow, keybind, variant, children }}
+				/>
 			)}
 		</RadixDM.Item>
 	);
diff --git a/packages/ui/style/colors.scss b/packages/ui/style/colors.scss
index 5c83a35de..24e548534 100644
--- a/packages/ui/style/colors.scss
+++ b/packages/ui/style/colors.scss
@@ -37,7 +37,7 @@
 	--color-app-frame: var(--dark-hue), 15%, 25%;
 	// menu
 	--color-menu: var(--dark-hue), 25%, 5%;
-	--color-menu-line: var(--dark-hue), 15%, 7%;
+	--color-menu-line: var(--dark-hue), 15%, 11%;
 	--color-menu-ink: var(--dark-hue), 5%, 100%;
 	--color-menu-faint: var(--dark-hue), 5%, 80%;
 	--color-menu-hover: var(--dark-hue), 15%, 30%;
@@ -81,8 +81,8 @@
 		--color-app-shade: var(--light-hue), 15%, 50%;
 		--color-app-frame: 0, 0%, 100%;
 		// menu
-		--color-menu: var(--light-hue), 16%, 0%;
-		--color-menu-line: var(--light-hue), 5%, 18%;
+		--color-menu: var(--light-hue), 5%, 100%;
+		--color-menu-line: var(--light-hue), 5%, 95%;
 		--color-menu-ink: var(--light-hue), 5%, 100%;
 		--color-menu-faint: var(--light-hue), 5%, 80%;
 		--color-menu-hover: var(--light-hue), 15%, 20%;