Use null as tag color by default (fallback to theme color) and stricter tag validation (#5398)

* Use null as tag color by default (fallback to theme color) and stricter tag validation

* Set ColorPicker's defaultValue props

* Set default tag color in first position

* Make ColorSwatchPicker configurable

Easier to change colors and how they are presented

packages/component-library/src/ColorPicker.tsx

@@ -33,64 +33,108 @@ function ColorSwatch(props: ColorSwatchProps) {
 }
 
 // colors from https://materialui.co/colors
-const colorsets = [
-  ['#D32F2F', '#C2185B', '#7B1FA2', '#512DA8', '#303F9F'],
-  ['#1976D2', '#0288D1', '#0097A7', '#00796B', '#388E3C'],
-  ['#689F38', '#AFB42B', '#FBC02D', '#FFA000', '#F57C00'],
-  ['#E64A19', '#5D4037', '#616161', '#455A64', '#690CB0'],
+const DEFAULT_COLOR_SET = [
+  '#690CB0',
+  '#D32F2F',
+  '#C2185B',
+  '#7B1FA2',
+  '#512DA8',
+  '#303F9F',
+  '#1976D2',
+  '#0288D1',
+  '#0097A7',
+  '#00796B',
+  '#388E3C',
+  '#689F38',
+  '#AFB42B',
+  '#FBC02D',
+  '#FFA000',
+  '#F57C00',
+  '#E64A19',
+  '#5D4037',
+  '#616161',
+  '#455A64',
 ];
 
-function ColorSwatchPicker() {
-  return (
-    <>
-      {colorsets.map((colors, idx) => (
-        <AriaColorSwatchPicker
-          key={`colorset-${idx}`}
-          style={{
-            display: 'flex',
-            gap: '8px',
-            flexWrap: 'wrap',
-          }}
-        >
-          {colors.map(color => (
-            <ColorSwatchPickerItem
-              key={color}
-              color={color}
-              className={css({
-                position: 'relative',
-                outline: 'none',
-                borderRadius: '4px',
-                width: 'fit-content',
-                forcedColorAdjust: 'none',
-                cursor: 'pointer',
+interface ColorSwatchPickerProps {
+  columns?: number;
+  colorset?: string[];
+}
 
-                '&[data-selected]::after': {
-                  // eslint-disable-next-line actual/typography
-                  content: '""',
-                  position: 'absolute',
-                  inset: 0,
-                  border: '2px solid black',
-                  outline: '2px solid white',
-                  outlineOffset: '-4px',
-                  borderRadius: 'inherit',
-                },
-              })}
-            >
-              <ColorSwatch />
-            </ColorSwatchPickerItem>
-          ))}
-        </AriaColorSwatchPicker>
-      ))}
-    </>
-  );
+function ColorSwatchPicker({
+  columns = 5,
+  colorset = DEFAULT_COLOR_SET,
+}: ColorSwatchPickerProps) {
+  const pickers = [];
+
+  for (let l = 0; l < colorset.length / columns; l++) {
+    const pickerItems = [];
+
+    for (let c = 0; c < columns; c++) {
+      const color = colorset[columns * l + c];
+      if (!color) {
+        break;
+      }
+
+      pickerItems.push(
+        <ColorSwatchPickerItem
+          key={color}
+          color={color}
+          className={css({
+            position: 'relative',
+            outline: 'none',
+            borderRadius: '4px',
+            width: 'fit-content',
+            forcedColorAdjust: 'none',
+            cursor: 'pointer',
+
+            '&[data-selected]::after': {
+              // eslint-disable-next-line actual/typography
+              content: '""',
+              position: 'absolute',
+              inset: 0,
+              border: '2px solid black',
+              outline: '2px solid white',
+              outlineOffset: '-4px',
+              borderRadius: 'inherit',
+            },
+          })}
+        >
+          <ColorSwatch />
+        </ColorSwatchPickerItem>,
+      );
+    }
+
+    pickers.push(
+      <AriaColorSwatchPicker
+        key={`colorset-${l}`}
+        style={{
+          display: 'flex',
+          gap: '8px',
+          flexWrap: 'wrap',
+        }}
+      >
+        {pickerItems}
+      </AriaColorSwatchPicker>,
+    );
+  }
+
+  return pickers;
 }
 const isColor = (value: string) => /^#[0-9a-fA-F]{6}$/.test(value);
 
 interface ColorPickerProps extends AriaColorPickerProps {
   children?: ReactNode;
+  columns?: number;
+  colorset?: string[];
 }
 
-export function ColorPicker({ children, ...props }: ColorPickerProps) {
+export function ColorPicker({
+  children,
+  columns,
+  colorset,
+  ...props
+}: ColorPickerProps) {
   const onInput = (value: string) => {
     if (!isColor(value)) {
       return;
@@ -103,7 +147,7 @@ export function ColorPicker({ children, ...props }: ColorPickerProps) {
   };
 
   return (
-    <AriaColorPicker {...props}>
+    <AriaColorPicker defaultValue={props.defaultValue ?? '#690CB0'} {...props}>
       <DialogTrigger>
         {children}
         <Popover>
@@ -120,7 +164,7 @@ export function ColorPicker({ children, ...props }: ColorPickerProps) {
               overflow: 'auto',
             }}
           >
-            <ColorSwatchPicker />
+            <ColorSwatchPicker columns={columns} colorset={colorset} />
             <ColorField
               onInput={({ target: { value } }: ChangeEvent<HTMLInputElement>) =>
                 onInput(value)

packages/desktop-client/src/components/tags/TagCreationRow.tsx

@@ -31,14 +31,12 @@ type TagCreationRowProps = {
   onClose: () => void;
 };
 
-const isTagValid = (tag: string) => tag.match(/^([^#\s]+)$/);
-
 export const TagCreationRow = ({ onClose, tags }: TagCreationRowProps) => {
   const { t } = useTranslation();
   const dispatch = useDispatch();
   const [tag, setTag] = useState('');
   const [description, setDescription] = useState('');
-  const [color, setColor] = useState(theme.noteTagDefault);
+  const [color, setColor] = useState<string | null>(null);
   const tagInput = useRef<HTMLInputElement>(null);
   const getTagCSS = useTagCSS();
 
@@ -59,14 +57,23 @@ export const TagCreationRow = ({ onClose, tags }: TagCreationRowProps) => {
   useProperFocus(cancelButtonRef, tableNavigator.focusedField === 'cancel');
 
   const resetInputs = () => {
-    setColor(theme.noteTagDefault);
+    setColor(null);
     setTag('');
     setDescription('');
     tableNavigator.onEdit('new-tag', 'tag');
   };
 
+  const isTagValid = () => {
+    return (
+      /^[^#\s]+$/.test(tag) && // accept any char except whitespaces and '#'
+      !tagNames.includes(tag) && // does not exists already
+      // color is null (default color) or is a 6 char hex color
+      (color === null || /^#[0-9a-fA-F]{6}$/.test(color))
+    );
+  };
+
   const onAddTag = () => {
-    if (!isTagValid(tag) || !color.trim() || tagNames.includes(tag)) {
+    if (!isTagValid()) {
       return;
     }
 
@@ -157,7 +164,7 @@ export const TagCreationRow = ({ onClose, tags }: TagCreationRowProps) => {
       >
         <Trans>Choose Color:</Trans>
         <ColorPicker
-          value={color}
+          value={color ?? undefined}
           onChange={color => setColor(color.toString('hex'))}
         >
           <Button
@@ -189,7 +196,7 @@ export const TagCreationRow = ({ onClose, tags }: TagCreationRowProps) => {
             style={{ padding: '4px 10px' }}
             onPress={onAddTag}
             data-testid="add-button"
-            isDisabled={!isTagValid(tag) || tagNames.includes(tag)}
+            isDisabled={!isTagValid()}
             ref={addButtonRef}
           >
             <Trans>Add</Trans>

packages/desktop-client/src/style/tags.ts

@@ -57,7 +57,10 @@ export function useTagCSS() {
   const [theme] = useTheme();
 
   return useCallback(
-    (tag: string, options: { color?: string; compact?: boolean } = {}) => {
+    (
+      tag: string,
+      options: { color?: string | null; compact?: boolean } = {},
+    ) => {
       const [color, backgroundColor, backgroundColorHovered] = getTagCSSColors(
         theme,
         // fallback strategy: options color > tag color > default color > theme color (undefined)