hide fill icons when fill color transparent (#2414)

Co-authored-by: Panayiotis Lipiridis <lipiridis@gmail.com>

src/components/Actions.tsx

@@ -12,7 +12,7 @@ import {
 import { t } from "../i18n";
 import { SHAPES } from "../shapes";
 import { ToolButton } from "./ToolButton";
-import { capitalizeString, setCursorForShape } from "../utils";
+import { capitalizeString, isTransparent, setCursorForShape } from "../utils";
 import Stack from "./Stack";
 import useIsMobile from "../is-mobile";
 import { getNonDeletedElements } from "../element";
@@ -34,20 +34,22 @@ export const SelectedShapeActions = ({
   );
   const isEditing = Boolean(appState.editingElement);
   const isMobile = useIsMobile();
-
   const isRTL = document.documentElement.getAttribute("dir") === "rtl";
 
+  const showFillIcons =
+    hasBackground(elementType) ||
+    targetElements.some(
+      (element) =>
+        hasBackground(element.type) && !isTransparent(element.backgroundColor),
+    );
+  const showChangeBackgroundIcons =
+    hasBackground(elementType) ||
+    targetElements.some((element) => hasBackground(element.type));
   return (
     <div className="panelColumn">
       {renderAction("changeStrokeColor")}
-      {(hasBackground(elementType) ||
+      {showChangeBackgroundIcons && renderAction("changeBackgroundColor")}
-        targetElements.some((element) => hasBackground(element.type))) && (
+      {showFillIcons && renderAction("changeFillStyle")}
-        <>
-          {renderAction("changeBackgroundColor")}
-
-          {renderAction("changeFillStyle")}
-        </>
-      )}
 
       {(hasStroke(elementType) ||
         targetElements.some((element) => hasStroke(element.type))) && (

src/tests/__snapshots__/regressionTests.test.tsx.snap

@@ -25765,6 +25765,238 @@ exports[`regression tests shift-click to multiselect, then drag: [end of test] n
 
 exports[`regression tests shift-click to multiselect, then drag: [end of test] number of renders 1`] = `17`;
 
+exports[`regression tests should show fill icons when element has non transparent background: [end of test] appState 1`] = `
+Object {
+  "appearance": "light",
+  "collaborators": Map {},
+  "currentItemBackgroundColor": "#fa5252",
+  "currentItemFillStyle": "hachure",
+  "currentItemFontFamily": 1,
+  "currentItemFontSize": 20,
+  "currentItemLinearStrokeSharpness": "round",
+  "currentItemOpacity": 100,
+  "currentItemRoughness": 1,
+  "currentItemStrokeColor": "#000000",
+  "currentItemStrokeSharpness": "sharp",
+  "currentItemStrokeStyle": "solid",
+  "currentItemStrokeWidth": 1,
+  "currentItemTextAlign": "left",
+  "cursorButton": "up",
+  "cursorX": 0,
+  "cursorY": 0,
+  "draggingElement": null,
+  "editingElement": null,
+  "editingGroupId": null,
+  "editingLinearElement": null,
+  "elementLocked": false,
+  "elementType": "selection",
+  "errorMessage": null,
+  "exportBackground": true,
+  "exportEmbedScene": false,
+  "fileHandle": null,
+  "gridSize": null,
+  "height": 768,
+  "isBindingEnabled": true,
+  "isCollaborating": false,
+  "isLibraryOpen": false,
+  "isLoading": false,
+  "isResizing": false,
+  "isRotating": false,
+  "lastPointerDownWith": "mouse",
+  "multiElement": null,
+  "name": "Untitled-201933152653",
+  "offsetLeft": 0,
+  "offsetTop": 0,
+  "openMenu": null,
+  "previousSelectedElementIds": Object {
+    "id0": true,
+  },
+  "resizingElement": null,
+  "scrollX": 0,
+  "scrollY": 0,
+  "scrolledOutside": false,
+  "selectedElementIds": Object {
+    "id0": true,
+    "id1": true,
+  },
+  "selectedGroupIds": Object {},
+  "selectionElement": null,
+  "shouldAddWatermark": false,
+  "shouldCacheIgnoreZoom": false,
+  "showShortcutsDialog": false,
+  "startBoundElement": null,
+  "suggestedBindings": Array [],
+  "username": "",
+  "viewBackgroundColor": "#ffffff",
+  "width": 1024,
+  "zenModeEnabled": false,
+  "zoom": Object {
+    "translation": Object {
+      "x": 0,
+      "y": 0,
+    },
+    "value": 1,
+  },
+}
+`;
+
+exports[`regression tests should show fill icons when element has non transparent background: [end of test] element 0 1`] = `
+Object {
+  "angle": 0,
+  "backgroundColor": "#fa5252",
+  "boundElementIds": null,
+  "fillStyle": "hachure",
+  "groupIds": Array [],
+  "height": 10,
+  "id": "id0",
+  "isDeleted": false,
+  "opacity": 100,
+  "roughness": 1,
+  "seed": 337897,
+  "strokeColor": "#000000",
+  "strokeSharpness": "sharp",
+  "strokeStyle": "solid",
+  "strokeWidth": 1,
+  "type": "rectangle",
+  "version": 5,
+  "versionNonce": 401146281,
+  "width": 10,
+  "x": 0,
+  "y": 0,
+}
+`;
+
+exports[`regression tests should show fill icons when element has non transparent background: [end of test] history 1`] = `
+Object {
+  "recording": false,
+  "redoStack": Array [],
+  "stateHistory": Array [
+    Object {
+      "appState": Object {
+        "editingGroupId": null,
+        "editingLinearElement": null,
+        "name": "Untitled-201933152653",
+        "selectedElementIds": Object {},
+        "viewBackgroundColor": "#ffffff",
+      },
+      "elements": Array [],
+    },
+    Object {
+      "appState": Object {
+        "editingGroupId": null,
+        "editingLinearElement": null,
+        "name": "Untitled-201933152653",
+        "selectedElementIds": Object {
+          "id0": true,
+        },
+        "viewBackgroundColor": "#ffffff",
+      },
+      "elements": Array [
+        Object {
+          "angle": 0,
+          "backgroundColor": "transparent",
+          "boundElementIds": null,
+          "fillStyle": "hachure",
+          "groupIds": Array [],
+          "height": 10,
+          "id": "id0",
+          "isDeleted": false,
+          "opacity": 100,
+          "roughness": 1,
+          "seed": 337897,
+          "strokeColor": "#000000",
+          "strokeSharpness": "sharp",
+          "strokeStyle": "solid",
+          "strokeWidth": 1,
+          "type": "rectangle",
+          "version": 2,
+          "versionNonce": 1278240551,
+          "width": 10,
+          "x": 0,
+          "y": 0,
+        },
+      ],
+    },
+    Object {
+      "appState": Object {
+        "editingGroupId": null,
+        "editingLinearElement": null,
+        "name": "Untitled-201933152653",
+        "selectedElementIds": Object {
+          "id0": true,
+        },
+        "viewBackgroundColor": "#ffffff",
+      },
+      "elements": Array [
+        Object {
+          "angle": 0,
+          "backgroundColor": "transparent",
+          "boundElementIds": null,
+          "fillStyle": "hachure",
+          "groupIds": Array [],
+          "height": 10,
+          "id": "id0",
+          "isDeleted": false,
+          "opacity": 100,
+          "roughness": 1,
+          "seed": 337897,
+          "strokeColor": "#000000",
+          "strokeSharpness": "sharp",
+          "strokeStyle": "solid",
+          "strokeWidth": 1,
+          "type": "rectangle",
+          "version": 3,
+          "versionNonce": 449462985,
+          "width": 10,
+          "x": 0,
+          "y": 0,
+        },
+      ],
+    },
+    Object {
+      "appState": Object {
+        "editingGroupId": null,
+        "editingLinearElement": null,
+        "name": "Untitled-201933152653",
+        "selectedElementIds": Object {
+          "id0": true,
+        },
+        "viewBackgroundColor": "#ffffff",
+      },
+      "elements": Array [
+        Object {
+          "angle": 0,
+          "backgroundColor": "#fa5252",
+          "boundElementIds": null,
+          "fillStyle": "hachure",
+          "groupIds": Array [],
+          "height": 10,
+          "id": "id0",
+          "isDeleted": false,
+          "opacity": 100,
+          "roughness": 1,
+          "seed": 337897,
+          "strokeColor": "#000000",
+          "strokeSharpness": "sharp",
+          "strokeStyle": "solid",
+          "strokeWidth": 1,
+          "type": "rectangle",
+          "version": 5,
+          "versionNonce": 401146281,
+          "width": 10,
+          "x": 0,
+          "y": 0,
+        },
+      ],
+    },
+  ],
+}
+`;
+
+exports[`regression tests should show fill icons when element has non transparent background: [end of test] number of elements 1`] = `1`;
+
+exports[`regression tests should show fill icons when element has non transparent background: [end of test] number of renders 1`] = `10`;
+
 exports[`regression tests shows 'Group selection' in context menu for multiple selected elements: [end of test] appState 1`] = `
 Object {
   "appearance": "light",

src/tests/regressionTests.test.tsx

@@ -512,19 +512,19 @@ describe("regression tests", () => {
   it("rerenders UI on language change", async () => {
     // select rectangle tool to show properties menu
     UI.clickTool("rectangle");
-    // english lang should display `hachure` label
+    // english lang should display `thin` label
-    expect(screen.queryByTitle(/hachure/i)).not.toBeNull();
+    expect(screen.queryByTitle(/thin/i)).not.toBeNull();
     fireEvent.change(document.querySelector(".dropdown-select__language")!, {
       target: { value: "de-DE" },
     });
-    // switching to german, `hachure` label should no longer exist
+    // switching to german, `thin` label should no longer exist
-    await waitFor(() => expect(screen.queryByTitle(/hachure/i)).toBeNull());
+    await waitFor(() => expect(screen.queryByTitle(/thin/i)).toBeNull());
     // reset language
     fireEvent.change(document.querySelector(".dropdown-select__language")!, {
       target: { value: "en" },
     });
     // switching back to English
-    await waitFor(() => expect(screen.queryByTitle(/hachure/i)).not.toBeNull());
+    await waitFor(() => expect(screen.queryByTitle(/thin/i)).not.toBeNull());
   });
 
   it("make a group and duplicate it", () => {
@@ -1546,6 +1546,21 @@ describe("regression tests", () => {
     });
     assertSelectedElements(rect3);
   });
+
+  it("should show fill icons when element has non transparent background", () => {
+    UI.clickTool("rectangle");
+    expect(screen.queryByText(/fill/i)).not.toBeNull();
+    mouse.down();
+    mouse.up(10, 10);
+    expect(screen.queryByText(/fill/i)).toBeNull();
+
+    clickLabeledElement("Background");
+    clickLabeledElement("#fa5252");
+    // select rectangle
+    mouse.reset();
+    mouse.click();
+    expect(screen.queryByText(/fill/i)).not.toBeNull();
+  });
 });
 
 it(

src/tests/utils.test.ts

@@ -0,0 +1,13 @@
+import * as utils from "../utils";
+
+describe("Test isTransparent", () => {
+  it("should return true when color is rgb transparent", () => {
+    expect(utils.isTransparent("#ff00")).toEqual(true);
+    expect(utils.isTransparent("#fff00000")).toEqual(true);
+    expect(utils.isTransparent("transparent")).toEqual(true);
+  });
+
+  it("should return false when color is not transparent", () => {
+    expect(utils.isTransparent("#ced4da")).toEqual(false);
+  });
+});

src/utils.ts

@@ -1,10 +1,11 @@
-import { Zoom } from "./types";
+import colors from "./colors";
 import {
   CURSOR_TYPE,
   FONT_FAMILY,
   WINDOWS_EMOJI_FALLBACK_FONT,
 } from "./constants";
 import { FontFamily, FontString } from "./element/types";
+import { Zoom } from "./types";
 
 export const SVG_NS = "http://www.w3.org/2000/svg";
 
@@ -292,3 +293,13 @@ export const findLastIndex = <T>(
   }
   return -1;
 };
+
+export const isTransparent = (color: string) => {
+  const isRGBTransparent = color.length === 5 && color.substr(4, 1) === "0";
+  const isRRGGBBTransparent = color.length === 9 && color.substr(7, 2) === "00";
+  return (
+    isRGBTransparent ||
+    isRRGGBBTransparent ||
+    color === colors.elementBackground[0]
+  );
+};