fix: Actions panel ux improvement (#6850)

Co-authored-by: dwelle <luzar.david@gmail.com>
This commit is contained in:
Viczián András 2023-10-24 20:36:13 +02:00 committed by GitHub
parent afea0df141
commit 71ad3c5356
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 279 additions and 261 deletions

View File

@ -0,0 +1,167 @@
import { Excalidraw } from "../packages/excalidraw/index";
import { queryByTestId } from "@testing-library/react";
import { render } from "../tests/test-utils";
import { UI } from "../tests/helpers/ui";
import { API } from "../tests/helpers/api";
import { COLOR_PALETTE, DEFAULT_ELEMENT_BACKGROUND_PICKS } from "../colors";
import { FONT_FAMILY, STROKE_WIDTH } from "../constants";
const { h } = window;
describe("element locking", () => {
beforeEach(async () => {
await render(<Excalidraw />);
});
describe("properties when tool selected", () => {
it("should show active background top picks", () => {
UI.clickTool("rectangle");
const color = DEFAULT_ELEMENT_BACKGROUND_PICKS[1];
// just in case we change it in the future
expect(color).not.toBe(COLOR_PALETTE.transparent);
h.setState({
currentItemBackgroundColor: color,
});
const activeColor = queryByTestId(
document.body,
`color-top-pick-${color}`,
);
expect(activeColor).toHaveClass("active");
});
it("should show fill style when background non-transparent", () => {
UI.clickTool("rectangle");
const color = DEFAULT_ELEMENT_BACKGROUND_PICKS[1];
// just in case we change it in the future
expect(color).not.toBe(COLOR_PALETTE.transparent);
h.setState({
currentItemBackgroundColor: color,
currentItemFillStyle: "hachure",
});
const hachureFillButton = queryByTestId(document.body, `fill-hachure`);
expect(hachureFillButton).toHaveClass("active");
h.setState({
currentItemFillStyle: "solid",
});
const solidFillStyle = queryByTestId(document.body, `fill-solid`);
expect(solidFillStyle).toHaveClass("active");
});
it("should not show fill style when background transparent", () => {
UI.clickTool("rectangle");
h.setState({
currentItemBackgroundColor: COLOR_PALETTE.transparent,
currentItemFillStyle: "hachure",
});
const hachureFillButton = queryByTestId(document.body, `fill-hachure`);
expect(hachureFillButton).toBe(null);
});
it("should show horizontal text align for text tool", () => {
UI.clickTool("text");
h.setState({
currentItemTextAlign: "right",
});
const centerTextAlign = queryByTestId(document.body, `align-right`);
expect(centerTextAlign).toBeChecked();
});
});
describe("properties when elements selected", () => {
it("should show active styles when single element selected", () => {
const rect = API.createElement({
type: "rectangle",
backgroundColor: "red",
fillStyle: "cross-hatch",
});
h.elements = [rect];
API.setSelectedElements([rect]);
const crossHatchButton = queryByTestId(document.body, `fill-cross-hatch`);
expect(crossHatchButton).toHaveClass("active");
});
it("should not show fill style selected element's background is transparent", () => {
const rect = API.createElement({
type: "rectangle",
backgroundColor: COLOR_PALETTE.transparent,
fillStyle: "cross-hatch",
});
h.elements = [rect];
API.setSelectedElements([rect]);
const crossHatchButton = queryByTestId(document.body, `fill-cross-hatch`);
expect(crossHatchButton).toBe(null);
});
it("should highlight common stroke width of selected elements", () => {
const rect1 = API.createElement({
type: "rectangle",
strokeWidth: STROKE_WIDTH.thin,
});
const rect2 = API.createElement({
type: "rectangle",
strokeWidth: STROKE_WIDTH.thin,
});
h.elements = [rect1, rect2];
API.setSelectedElements([rect1, rect2]);
const thinStrokeWidthButton = queryByTestId(
document.body,
`strokeWidth-thin`,
);
expect(thinStrokeWidthButton).toBeChecked();
});
it("should not highlight any stroke width button if no common style", () => {
const rect1 = API.createElement({
type: "rectangle",
strokeWidth: STROKE_WIDTH.thin,
});
const rect2 = API.createElement({
type: "rectangle",
strokeWidth: STROKE_WIDTH.bold,
});
h.elements = [rect1, rect2];
API.setSelectedElements([rect1, rect2]);
expect(queryByTestId(document.body, `strokeWidth-thin`)).not.toBe(null);
expect(
queryByTestId(document.body, `strokeWidth-thin`),
).not.toBeChecked();
expect(
queryByTestId(document.body, `strokeWidth-bold`),
).not.toBeChecked();
expect(
queryByTestId(document.body, `strokeWidth-extraBold`),
).not.toBeChecked();
});
it("should show properties of different element types when selected", () => {
const rect = API.createElement({
type: "rectangle",
strokeWidth: STROKE_WIDTH.bold,
});
const text = API.createElement({
type: "text",
fontFamily: FONT_FAMILY.Cascadia,
});
h.elements = [rect, text];
API.setSelectedElements([rect, text]);
expect(queryByTestId(document.body, `strokeWidth-bold`)).toBeChecked();
expect(queryByTestId(document.body, `font-family-code`)).toBeChecked();
});
});
});

View File

@ -1,4 +1,4 @@
import { AppState } from "../../src/types";
import { AppState, Primitive } from "../../src/types";
import {
DEFAULT_ELEMENT_BACKGROUND_COLOR_PALETTE,
DEFAULT_ELEMENT_BACKGROUND_PICKS,
@ -51,6 +51,7 @@ import {
DEFAULT_FONT_SIZE,
FONT_FAMILY,
ROUNDNESS,
STROKE_WIDTH,
VERTICAL_ALIGN,
} from "../constants";
import {
@ -82,7 +83,6 @@ import { getLanguage, t } from "../i18n";
import { KEYS } from "../keys";
import { randomInteger } from "../random";
import {
canChangeRoundness,
canHaveArrowheads,
getCommonAttributeOfSelectedElements,
getSelectedElements,
@ -118,25 +118,44 @@ export const changeProperty = (
});
};
export const getFormValue = function <T>(
export const getFormValue = function <T extends Primitive>(
elements: readonly ExcalidrawElement[],
appState: AppState,
getAttribute: (element: ExcalidrawElement) => T,
defaultValue: T,
isRelevantElement: true | ((element: ExcalidrawElement) => boolean),
defaultValue: T | ((isSomeElementSelected: boolean) => T),
): T {
const editingElement = appState.editingElement;
const nonDeletedElements = getNonDeletedElements(elements);
return (
(editingElement && getAttribute(editingElement)) ??
(isSomeElementSelected(nonDeletedElements, appState)
? getCommonAttributeOfSelectedElements(
nonDeletedElements,
let ret: T | null = null;
if (editingElement) {
ret = getAttribute(editingElement);
}
if (!ret) {
const hasSelection = isSomeElementSelected(nonDeletedElements, appState);
if (hasSelection) {
ret =
getCommonAttributeOfSelectedElements(
isRelevantElement === true
? nonDeletedElements
: nonDeletedElements.filter((el) => isRelevantElement(el)),
appState,
getAttribute,
)
: defaultValue) ??
defaultValue
);
) ??
(typeof defaultValue === "function"
? defaultValue(true)
: defaultValue);
} else {
ret =
typeof defaultValue === "function" ? defaultValue(false) : defaultValue;
}
}
return ret;
};
const offsetElementAfterFontResize = (
@ -247,6 +266,7 @@ export const actionChangeStrokeColor = register({
elements,
appState,
(element) => element.strokeColor,
true,
appState.currentItemStrokeColor,
)}
onChange={(color) => updateData({ currentItemStrokeColor: color })}
@ -289,6 +309,7 @@ export const actionChangeBackgroundColor = register({
elements,
appState,
(element) => element.backgroundColor,
true,
appState.currentItemBackgroundColor,
)}
onChange={(color) => updateData({ currentItemBackgroundColor: color })}
@ -338,23 +359,28 @@ export const actionChangeFillStyle = register({
} (${getShortcutKey("Alt-Click")})`,
icon: allElementsZigZag ? FillZigZagIcon : FillHachureIcon,
active: allElementsZigZag ? true : undefined,
testId: `fill-hachure`,
},
{
value: "cross-hatch",
text: t("labels.crossHatch"),
icon: FillCrossHatchIcon,
testId: `fill-cross-hatch`,
},
{
value: "solid",
text: t("labels.solid"),
icon: FillSolidIcon,
testId: `fill-solid`,
},
]}
value={getFormValue(
elements,
appState,
(element) => element.fillStyle,
appState.currentItemFillStyle,
(element) => element.hasOwnProperty("fillStyle"),
(hasSelection) =>
hasSelection ? null : appState.currentItemFillStyle,
)}
onClick={(value, event) => {
const nextValue =
@ -393,26 +419,31 @@ export const actionChangeStrokeWidth = register({
group="stroke-width"
options={[
{
value: 1,
value: STROKE_WIDTH.thin,
text: t("labels.thin"),
icon: StrokeWidthBaseIcon,
testId: "strokeWidth-thin",
},
{
value: 2,
value: STROKE_WIDTH.bold,
text: t("labels.bold"),
icon: StrokeWidthBoldIcon,
testId: "strokeWidth-bold",
},
{
value: 4,
value: STROKE_WIDTH.extraBold,
text: t("labels.extraBold"),
icon: StrokeWidthExtraBoldIcon,
testId: "strokeWidth-extraBold",
},
]}
value={getFormValue(
elements,
appState,
(element) => element.strokeWidth,
appState.currentItemStrokeWidth,
(element) => element.hasOwnProperty("strokeWidth"),
(hasSelection) =>
hasSelection ? null : appState.currentItemStrokeWidth,
)}
onChange={(value) => updateData(value)}
/>
@ -461,7 +492,9 @@ export const actionChangeSloppiness = register({
elements,
appState,
(element) => element.roughness,
appState.currentItemRoughness,
(element) => element.hasOwnProperty("roughness"),
(hasSelection) =>
hasSelection ? null : appState.currentItemRoughness,
)}
onChange={(value) => updateData(value)}
/>
@ -509,7 +542,9 @@ export const actionChangeStrokeStyle = register({
elements,
appState,
(element) => element.strokeStyle,
appState.currentItemStrokeStyle,
(element) => element.hasOwnProperty("strokeStyle"),
(hasSelection) =>
hasSelection ? null : appState.currentItemStrokeStyle,
)}
onChange={(value) => updateData(value)}
/>
@ -549,6 +584,7 @@ export const actionChangeOpacity = register({
elements,
appState,
(element) => element.opacity,
true,
appState.currentItemOpacity,
) ?? undefined
}
@ -607,7 +643,12 @@ export const actionChangeFontSize = register({
}
return null;
},
appState.currentItemFontSize || DEFAULT_FONT_SIZE,
(element) =>
isTextElement(element) || getBoundTextElement(element) !== null,
(hasSelection) =>
hasSelection
? null
: appState.currentItemFontSize || DEFAULT_FONT_SIZE,
)}
onChange={(value) => updateData(value)}
/>
@ -692,21 +733,25 @@ export const actionChangeFontFamily = register({
value: FontFamilyValues;
text: string;
icon: JSX.Element;
testId: string;
}[] = [
{
value: FONT_FAMILY.Virgil,
text: t("labels.handDrawn"),
icon: FreedrawIcon,
testId: "font-family-virgil",
},
{
value: FONT_FAMILY.Helvetica,
text: t("labels.normal"),
icon: FontFamilyNormalIcon,
testId: "font-family-normal",
},
{
value: FONT_FAMILY.Cascadia,
text: t("labels.code"),
icon: FontFamilyCodeIcon,
testId: "font-family-code",
},
];
@ -729,7 +774,12 @@ export const actionChangeFontFamily = register({
}
return null;
},
appState.currentItemFontFamily || DEFAULT_FONT_FAMILY,
(element) =>
isTextElement(element) || getBoundTextElement(element) !== null,
(hasSelection) =>
hasSelection
? null
: appState.currentItemFontFamily || DEFAULT_FONT_FAMILY,
)}
onChange={(value) => updateData(value)}
/>
@ -806,7 +856,10 @@ export const actionChangeTextAlign = register({
}
return null;
},
appState.currentItemTextAlign,
(element) =>
isTextElement(element) || getBoundTextElement(element) !== null,
(hasSelection) =>
hasSelection ? null : appState.currentItemTextAlign,
)}
onChange={(value) => updateData(value)}
/>
@ -882,7 +935,9 @@ export const actionChangeVerticalAlign = register({
}
return null;
},
VERTICAL_ALIGN.MIDDLE,
(element) =>
isTextElement(element) || getBoundTextElement(element) !== null,
(hasSelection) => (hasSelection ? null : VERTICAL_ALIGN.MIDDLE),
)}
onChange={(value) => updateData(value)}
/>
@ -947,9 +1002,9 @@ export const actionChangeRoundness = register({
appState,
(element) =>
hasLegacyRoundness ? null : element.roundness ? "round" : "sharp",
(canChangeRoundness(appState.activeTool.type) &&
appState.currentItemRoundness) ||
null,
(element) => element.hasOwnProperty("roundness"),
(hasSelection) =>
hasSelection ? null : appState.currentItemRoundness,
)}
onChange={(value) => updateData(value)}
/>
@ -1043,6 +1098,7 @@ export const actionChangeArrowhead = register({
isLinearElement(element) && canHaveArrowheads(element.type)
? element.startArrowhead
: appState.currentItemStartArrowhead,
true,
appState.currentItemStartArrowhead,
)}
onChange={(value) => updateData({ position: "start", type: value })}
@ -1089,6 +1145,7 @@ export const actionChangeArrowhead = register({
isLinearElement(element) && canHaveArrowheads(element.type)
? element.endArrowhead
: appState.currentItemEndArrowhead,
true,
appState.currentItemEndArrowhead,
)}
onChange={(value) => updateData({ position: "end", type: value })}

View File

@ -11,7 +11,6 @@ import {
hasBackground,
hasStrokeStyle,
hasStrokeWidth,
hasText,
} from "../scene";
import { SHAPES } from "../shapes";
import { AppClassProperties, UIAppState, Zoom } from "../types";
@ -20,7 +19,7 @@ import Stack from "./Stack";
import { ToolButton } from "./ToolButton";
import { hasStrokeColor } from "../scene/comparisons";
import { trackEvent } from "../analytics";
import { hasBoundTextElement } from "../element/typeChecks";
import { hasBoundTextElement, isTextElement } from "../element/typeChecks";
import clsx from "clsx";
import { actionToggleZenMode } from "../actions";
import { Tooltip } from "./Tooltip";
@ -66,7 +65,8 @@ export const SelectedShapeActions = ({
const isRTL = document.documentElement.getAttribute("dir") === "rtl";
const showFillIcons =
hasBackground(appState.activeTool.type) ||
(hasBackground(appState.activeTool.type) &&
!isTransparent(appState.currentItemBackgroundColor)) ||
targetElements.some(
(element) =>
hasBackground(element.type) && !isTransparent(element.backgroundColor),
@ -123,14 +123,15 @@ export const SelectedShapeActions = ({
<>{renderAction("changeRoundness")}</>
)}
{(hasText(appState.activeTool.type) ||
targetElements.some((element) => hasText(element.type))) && (
{(appState.activeTool.type === "text" ||
targetElements.some(isTextElement)) && (
<>
{renderAction("changeFontSize")}
{renderAction("changeFontFamily")}
{suppportsHorizontalAlign(targetElements) &&
{(appState.activeTool.type === "text" ||
suppportsHorizontalAlign(targetElements)) &&
renderAction("changeTextAlign")}
</>
)}

View File

@ -55,6 +55,7 @@ export const TopPicks = ({
type="button"
title={color}
onClick={() => onChange(color)}
data-testid={`color-top-pick-${color}`}
>
<div className="color-picker__button-outline" />
</button>

View File

@ -302,6 +302,12 @@ export const ROUGHNESS = {
cartoonist: 2,
} as const;
export const STROKE_WIDTH = {
thin: 1,
bold: 2,
extraBold: 4,
} as const;
export const DEFAULT_ELEMENT_PROPS: {
strokeColor: ExcalidrawElement["strokeColor"];
backgroundColor: ExcalidrawElement["backgroundColor"];

View File

@ -39,8 +39,6 @@ export const canChangeRoundness = (type: string) =>
type === "line" ||
type === "diamond";
export const hasText = (type: string) => type === "text";
export const canHaveArrowheads = (type: string) => type === "arrow";
export const getElementAtPosition = (

View File

@ -14,7 +14,6 @@ export {
canHaveArrowheads,
canChangeRoundness,
getElementAtPosition,
hasText,
getElementsAtPosition,
} from "./comparisons";
export { getNormalizedZoom } from "./zoom";

View File

@ -14255,217 +14255,6 @@ exports[`regression tests > should group elements and ungroup them > [end of tes
exports[`regression tests > should group elements and ungroup them > [end of test] number of renders 1`] = `21`;
exports[`regression tests > should show fill icons when element has non transparent background > [end of test] appState 1`] = `
{
"activeEmbeddable": null,
"activeTool": {
"customType": null,
"lastActiveTool": null,
"locked": false,
"type": "selection",
},
"collaborators": Map {},
"contextMenu": null,
"currentChartType": "bar",
"currentItemBackgroundColor": "#ffc9c9",
"currentItemEndArrowhead": "arrow",
"currentItemFillStyle": "solid",
"currentItemFontFamily": 1,
"currentItemFontSize": 20,
"currentItemOpacity": 100,
"currentItemRoughness": 1,
"currentItemRoundness": "round",
"currentItemStartArrowhead": null,
"currentItemStrokeColor": "#1e1e1e",
"currentItemStrokeStyle": "solid",
"currentItemStrokeWidth": 2,
"currentItemTextAlign": "left",
"cursorButton": "up",
"defaultSidebarDockedPreference": false,
"draggingElement": null,
"editingElement": null,
"editingFrame": null,
"editingGroupId": null,
"editingLinearElement": null,
"elementsToHighlight": null,
"errorMessage": null,
"exportBackground": true,
"exportEmbedScene": false,
"exportScale": 1,
"exportWithDarkMode": false,
"fileHandle": null,
"frameRendering": {
"clip": true,
"enabled": true,
"name": true,
"outline": true,
},
"frameToHighlight": null,
"gridSize": null,
"height": 768,
"isBindingEnabled": true,
"isLoading": false,
"isResizing": false,
"isRotating": false,
"lastPointerDownWith": "mouse",
"multiElement": null,
"name": "Untitled-201933152653",
"objectsSnapModeEnabled": false,
"offsetLeft": 0,
"offsetTop": 0,
"openDialog": null,
"openMenu": null,
"openPopup": "elementBackground",
"openSidebar": null,
"originSnapOffset": null,
"pasteDialog": {
"data": null,
"shown": false,
},
"penDetected": false,
"penMode": false,
"pendingImageElementId": null,
"previousSelectedElementIds": {},
"resizingElement": null,
"scrollX": 0,
"scrollY": 0,
"scrolledOutside": false,
"selectedElementIds": {
"id0": true,
},
"selectedElementsAreBeingDragged": false,
"selectedGroupIds": {},
"selectedLinearElement": null,
"selectionElement": null,
"shouldCacheIgnoreZoom": false,
"showHyperlinkPopup": false,
"showStats": false,
"showWelcomeScreen": true,
"snapLines": [],
"startBoundElement": null,
"suggestedBindings": [],
"theme": "light",
"toast": null,
"viewBackgroundColor": "#ffffff",
"viewModeEnabled": false,
"width": 1024,
"zenModeEnabled": false,
"zoom": {
"value": 1,
},
}
`;
exports[`regression tests > should show fill icons when element has non transparent background > [end of test] history 1`] = `
{
"recording": false,
"redoStack": [],
"stateHistory": [
{
"appState": {
"editingGroupId": null,
"editingLinearElement": null,
"name": "Untitled-201933152653",
"selectedElementIds": {},
"selectedGroupIds": {},
"viewBackgroundColor": "#ffffff",
},
"elements": [],
},
{
"appState": {
"editingGroupId": null,
"editingLinearElement": null,
"name": "Untitled-201933152653",
"selectedElementIds": {
"id0": true,
},
"selectedGroupIds": {},
"viewBackgroundColor": "#ffffff",
},
"elements": [
{
"angle": 0,
"backgroundColor": "transparent",
"boundElements": null,
"fillStyle": "solid",
"frameId": null,
"groupIds": [],
"height": 10,
"id": "id0",
"isDeleted": false,
"link": null,
"locked": false,
"opacity": 100,
"roughness": 1,
"roundness": {
"type": 3,
},
"seed": 1278240551,
"strokeColor": "#1e1e1e",
"strokeStyle": "solid",
"strokeWidth": 2,
"type": "rectangle",
"updated": 1,
"version": 2,
"versionNonce": 453191,
"width": 10,
"x": 0,
"y": 0,
},
],
},
{
"appState": {
"editingGroupId": null,
"editingLinearElement": null,
"name": "Untitled-201933152653",
"selectedElementIds": {
"id0": true,
},
"selectedGroupIds": {},
"viewBackgroundColor": "#ffffff",
},
"elements": [
{
"angle": 0,
"backgroundColor": "#ffc9c9",
"boundElements": null,
"fillStyle": "solid",
"frameId": null,
"groupIds": [],
"height": 10,
"id": "id0",
"isDeleted": false,
"link": null,
"locked": false,
"opacity": 100,
"roughness": 1,
"roundness": {
"type": 3,
},
"seed": 1278240551,
"strokeColor": "#1e1e1e",
"strokeStyle": "solid",
"strokeWidth": 2,
"type": "rectangle",
"updated": 1,
"version": 3,
"versionNonce": 2019559783,
"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`] = `0`;
exports[`regression tests > should show fill icons when element has non transparent background > [end of test] number of renders 1`] = `9`;
exports[`regression tests > single-clicking on a subgroup of a selected group should not alter selection > [end of test] appState 1`] = `
{
"activeEmbeddable": null,

View File

@ -535,6 +535,7 @@ exports[`<Excalidraw/> > Test UIOptions prop > Test canvasActions > should rende
>
<button
class="color-picker__button active"
data-testid="color-top-pick-#ffffff"
style="--swatch-color: #ffffff;"
title="#ffffff"
type="button"
@ -545,6 +546,7 @@ exports[`<Excalidraw/> > Test UIOptions prop > Test canvasActions > should rende
</button>
<button
class="color-picker__button"
data-testid="color-top-pick-#f8f9fa"
style="--swatch-color: #f8f9fa;"
title="#f8f9fa"
type="button"
@ -555,6 +557,7 @@ exports[`<Excalidraw/> > Test UIOptions prop > Test canvasActions > should rende
</button>
<button
class="color-picker__button"
data-testid="color-top-pick-#f5faff"
style="--swatch-color: #f5faff;"
title="#f5faff"
type="button"
@ -565,6 +568,7 @@ exports[`<Excalidraw/> > Test UIOptions prop > Test canvasActions > should rende
</button>
<button
class="color-picker__button"
data-testid="color-top-pick-#fffce8"
style="--swatch-color: #fffce8;"
title="#fffce8"
type="button"
@ -575,6 +579,7 @@ exports[`<Excalidraw/> > Test UIOptions prop > Test canvasActions > should rende
</button>
<button
class="color-picker__button"
data-testid="color-top-pick-#fdf8f6"
style="--swatch-color: #fdf8f6;"
title="#fdf8f6"
type="button"

View File

@ -1089,20 +1089,6 @@ describe("regression tests", () => {
});
assertSelectedElements(rect3);
});
it("should show fill icons when element has non transparent background", async () => {
UI.clickTool("rectangle");
expect(screen.queryByText(/fill/i)).not.toBeNull();
mouse.down();
mouse.up(10, 10);
expect(screen.queryByText(/fill/i)).toBeNull();
togglePopover("Background");
UI.clickOnTestId("color-red");
// select rectangle
mouse.reset();
mouse.click();
expect(screen.queryByText(/fill/i)).not.toBeNull();
});
});
it(

View File

@ -695,3 +695,12 @@ export type KeyboardModifiersObject = {
altKey: boolean;
metaKey: boolean;
};
export type Primitive =
| number
| string
| boolean
| bigint
| symbol
| null
| undefined;