From 62dbe3acf5a2f124422504abf55ab71eec196640 Mon Sep 17 00:00:00 2001
From: Julian Dominguez-Schatz <julian.dominguezschatz@gmail.com>
Date: Sun, 8 Sep 2024 13:16:50 -0400
Subject: [PATCH] Refine `Menu`/`Select` types to allow broader types for the
`value`/`name` attribute (#3391)
* Refine `Menu`/`Select` types to allow arbitrary value types
* Add release notes
---
.../src/components/common/Menu.tsx | 52 +++++++----
.../src/components/common/Select.tsx | 12 ++-
.../src/components/reports/ReportSidebar.tsx | 11 +--
.../src/components/reports/SaveReportMenu.tsx | 92 +++++++++----------
upcoming-release-notes/3391.md | 6 ++
5 files changed, 93 insertions(+), 80 deletions(-)
create mode 100644 upcoming-release-notes/3391.md
diff --git a/packages/desktop-client/src/components/common/Menu.tsx b/packages/desktop-client/src/components/common/Menu.tsx
index 55372c02c..e83bf2b6a 100644
--- a/packages/desktop-client/src/components/common/Menu.tsx
+++ b/packages/desktop-client/src/components/common/Menu.tsx
@@ -1,5 +1,4 @@
import {
- type ReactElement,
type ReactNode,
useEffect,
useRef,
@@ -15,8 +14,9 @@ import { Toggle } from './Toggle';
import { View } from './View';
const MenuLine: unique symbol = Symbol('menu-line');
+const MenuLabel: unique symbol = Symbol('menu-label');
Menu.line = MenuLine;
-Menu.label = Symbol('menu-label');
+Menu.label = MenuLabel;
type KeybindingProps = {
keyName: ReactNode;
@@ -30,9 +30,9 @@ function Keybinding({ keyName }: KeybindingProps) {
);
}
-export type MenuItem = {
- type?: string | symbol;
- name: string;
+type MenuItemObject<NameType, Type extends string | symbol = string> = {
+ type?: Type;
+ name: NameType;
disabled?: boolean;
icon?: ComponentType<SVGProps<SVGSVGElement>>;
iconSize?: number;
@@ -42,17 +42,28 @@ export type MenuItem = {
tooltip?: string;
};
-type MenuProps<T extends MenuItem = MenuItem> = {
+export type MenuItem<NameType = string> =
+ | MenuItemObject<NameType>
+ | MenuItemObject<string, typeof Menu.label>
+ | typeof Menu.line;
+
+function isLabel<T>(
+ item: MenuItemObject<T> | MenuItemObject<string, typeof Menu.label>,
+): item is MenuItemObject<string, typeof Menu.label> {
+ return item.type === Menu.label;
+}
+
+type MenuProps<NameType> = {
header?: ReactNode;
footer?: ReactNode;
- items: Array<T | typeof Menu.line>;
- onMenuSelect?: (itemName: T['name']) => void;
+ items: Array<MenuItem<NameType>>;
+ onMenuSelect?: (itemName: NameType) => void;
style?: CSSProperties;
className?: string;
- getItemStyle?: (item: T) => CSSProperties;
+ getItemStyle?: (item: MenuItemObject<NameType>) => CSSProperties;
};
-export function Menu<T extends MenuItem>({
+export function Menu<const NameType = string>({
header,
footer,
items: allItems,
@@ -60,7 +71,7 @@ export function Menu<T extends MenuItem>({
style,
className,
getItemStyle,
-}: MenuProps<T>) {
+}: MenuProps<NameType>) {
const elRef = useRef<HTMLDivElement>(null);
const items = allItems.filter(x => x);
const [hoveredIndex, setHoveredIndex] = useState<number | null>(null);
@@ -99,7 +110,7 @@ export function Menu<T extends MenuItem>({
case 'Enter':
e.preventDefault();
const item = items[hoveredIndex || 0];
- if (hoveredIndex !== null && item !== Menu.line) {
+ if (hoveredIndex !== null && item !== Menu.line && !isLabel(item)) {
onMenuSelect?.(item.name);
}
break;
@@ -129,7 +140,7 @@ export function Menu<T extends MenuItem>({
<View style={{ borderTop: '1px solid ' + theme.menuBorder }} />
</View>
);
- } else if (item.type === Menu.label) {
+ } else if (isLabel(item)) {
return (
<Text
key={idx}
@@ -152,7 +163,7 @@ export function Menu<T extends MenuItem>({
return (
<View
role="button"
- key={item.name}
+ key={String(item.name)}
style={{
cursor: 'default',
padding: 10,
@@ -166,14 +177,18 @@ export function Menu<T extends MenuItem>({
backgroundColor: theme.menuItemBackgroundHover,
color: theme.menuItemTextHover,
}),
- ...getItemStyle?.(item),
+ ...(!isLabel(item) && getItemStyle?.(item)),
}}
onPointerEnter={() => setHoveredIndex(idx)}
onPointerLeave={() => setHoveredIndex(null)}
onPointerUp={e => {
e.stopPropagation();
- if (!item.disabled && item.toggle === undefined) {
+ if (
+ !item.disabled &&
+ item.toggle === undefined &&
+ !isLabel(item)
+ ) {
onMenuSelect?.(item.name);
}
}}
@@ -193,17 +208,18 @@ export function Menu<T extends MenuItem>({
</>
) : (
<>
- <label htmlFor={item.name} title={item.tooltip}>
+ <label htmlFor={String(item.name)} title={item.tooltip}>
{item.text}
</label>
<View style={{ flex: 1 }} />
<Toggle
- id={item.name}
+ id={String(item.name)}
checked={item.toggle}
onColor={theme.pageTextPositive}
style={{ marginLeft: 5 }}
onToggle={() =>
!item.disabled &&
+ !isLabel(item) &&
item.toggle !== undefined &&
onMenuSelect?.(item.name)
}
diff --git a/packages/desktop-client/src/components/common/Select.tsx b/packages/desktop-client/src/components/common/Select.tsx
index 38a5be455..c2fae346e 100644
--- a/packages/desktop-client/src/components/common/Select.tsx
+++ b/packages/desktop-client/src/components/common/Select.tsx
@@ -8,15 +8,17 @@ import { Menu } from './Menu';
import { Popover } from './Popover';
import { View } from './View';
-function isValueOption<Value extends string>(
- option: [Value, string] | typeof Menu.line,
+function isValueOption<Value>(
+ option: readonly [Value, string] | typeof Menu.line,
): option is [Value, string] {
return option !== Menu.line;
}
-type SelectProps<Value extends string> = {
+export type SelectOption<Value = string> = [Value, string] | typeof Menu.line;
+
+type SelectProps<Value> = {
bare?: boolean;
- options: Array<[Value, string] | typeof Menu.line>;
+ options: Array<readonly [Value, string] | typeof Menu.line>;
value: Value;
defaultLabel?: string;
onChange?: (newValue: Value) => void;
@@ -38,7 +40,7 @@ type SelectProps<Value extends string> = {
* // <Select options={[['1', 'Option 1'], ['2', 'Option 2']]} value="1" onChange={handleOnChange} />
* // <Select options={[['1', 'Option 1'], ['2', 'Option 2']]} value="3" defaultLabel="Select an option" onChange={handleOnChange} />
*/
-export function Select<Value extends string>({
+export function Select<const Value = string>({
bare,
options,
value,
diff --git a/packages/desktop-client/src/components/reports/ReportSidebar.tsx b/packages/desktop-client/src/components/reports/ReportSidebar.tsx
index 3bf4fdcf3..0424fba5b 100644
--- a/packages/desktop-client/src/components/reports/ReportSidebar.tsx
+++ b/packages/desktop-client/src/components/reports/ReportSidebar.tsx
@@ -1,4 +1,4 @@
-import React, { useMemo, useRef, useState, type ComponentProps } from 'react';
+import React, { useMemo, useRef, useState } from 'react';
import * as monthUtils from 'loot-core/src/shared/months';
import { type CategoryEntity } from 'loot-core/types/models/category';
@@ -12,7 +12,7 @@ import { Information } from '../alerts';
import { Button } from '../common/Button2';
import { Menu } from '../common/Menu';
import { Popover } from '../common/Popover';
-import { Select } from '../common/Select';
+import { Select, type SelectOption } from '../common/Select';
import { Text } from '../common/Text';
import { Tooltip } from '../common/Tooltip';
import { View } from '../common/View';
@@ -139,10 +139,9 @@ export function ReportSidebar({
};
const rangeOptions = useMemo(() => {
- const options: ComponentProps<typeof Select>['options'] =
- ReportOptions.dateRange
- .filter(f => f[customReportItems.interval as keyof dateRangeProps])
- .map(option => [option.description, option.description]);
+ const options: SelectOption[] = ReportOptions.dateRange
+ .filter(f => f[customReportItems.interval as keyof dateRangeProps])
+ .map(option => [option.description, option.description]);
// Append separator if necessary
if (dateRangeLine > 0) {
diff --git a/packages/desktop-client/src/components/reports/SaveReportMenu.tsx b/packages/desktop-client/src/components/reports/SaveReportMenu.tsx
index d6eb390f0..ad1409e82 100644
--- a/packages/desktop-client/src/components/reports/SaveReportMenu.tsx
+++ b/packages/desktop-client/src/components/reports/SaveReportMenu.tsx
@@ -1,6 +1,6 @@
-import React, { type ComponentPropsWithoutRef } from 'react';
+import React from 'react';
-import { Menu } from '../common/Menu';
+import { Menu, type MenuItem } from '../common/Menu';
export function SaveReportMenu({
onMenuSelect,
@@ -11,65 +11,55 @@ export function SaveReportMenu({
savedStatus: string;
listReports: number;
}) {
- const savedMenu: ComponentPropsWithoutRef<typeof Menu> =
+ const savedMenu: MenuItem[] =
savedStatus === 'saved'
- ? {
- items: [
- { name: 'rename-report', text: 'Rename' },
- { name: 'delete-report', text: 'Delete' },
- Menu.line,
- ],
- }
- : {
- items: [],
- };
+ ? [
+ { name: 'rename-report', text: 'Rename' },
+ { name: 'delete-report', text: 'Delete' },
+ Menu.line,
+ ]
+ : [];
- const modifiedMenu: ComponentPropsWithoutRef<typeof Menu> =
+ const modifiedMenu: MenuItem[] =
savedStatus === 'modified'
- ? {
- items: [
- { name: 'rename-report', text: 'Rename' },
- {
- name: 'update-report',
- text: 'Update report',
- },
- {
- name: 'reload-report',
- text: 'Revert changes',
- },
- { name: 'delete-report', text: 'Delete' },
- Menu.line,
- ],
- }
- : {
- items: [],
- };
+ ? [
+ { name: 'rename-report', text: 'Rename' },
+ {
+ name: 'update-report',
+ text: 'Update report',
+ },
+ {
+ name: 'reload-report',
+ text: 'Revert changes',
+ },
+ { name: 'delete-report', text: 'Delete' },
+ Menu.line,
+ ]
+ : [];
- const unsavedMenu: ComponentPropsWithoutRef<typeof Menu> = {
- items: [
- {
- name: 'save-report',
- text: 'Save new report',
- },
- {
- name: 'reset-report',
- text: 'Reset to default',
- },
- Menu.line,
- {
- name: 'choose-report',
- text: 'Choose Report',
- disabled: listReports > 0 ? false : true,
- },
- ],
- };
+ const unsavedMenu: MenuItem[] = [
+ {
+ name: 'save-report',
+ text: 'Save new report',
+ },
+ {
+ name: 'reset-report',
+ text: 'Reset to default',
+ },
+ Menu.line,
+ {
+ name: 'choose-report',
+ text: 'Choose Report',
+ disabled: listReports > 0 ? false : true,
+ },
+ ];
return (
<Menu
onMenuSelect={item => {
onMenuSelect(item);
}}
- items={[...savedMenu.items, ...modifiedMenu.items, ...unsavedMenu.items]}
+ items={[...savedMenu, ...modifiedMenu, ...unsavedMenu]}
/>
);
}
diff --git a/upcoming-release-notes/3391.md b/upcoming-release-notes/3391.md
new file mode 100644
index 000000000..a09102eac
--- /dev/null
+++ b/upcoming-release-notes/3391.md
@@ -0,0 +1,6 @@
+---
+category: Maintenance
+authors: [jfdoming]
+---
+
+Refine Menu/Select types to allow broader types for the value/name attribute
--
GitLab