From 1a5ab1e2aa4c2ff36272919dbd6dd7a06a39f9d5 Mon Sep 17 00:00:00 2001 From: hlomzik Date: Tue, 1 Oct 2024 21:48:04 +0100 Subject: [PATCH 01/10] feat: LEAP-1492: More flexible Custom buttons - extend the structure of custom buttons definition to allow different modes - custom buttons now can replace just some buttons, leaving the rest untouched - start to simplify work with all buttons to use standard button description + additional wrappers for main actions, so any button action will go through mandatory checks --- .../src/components/BottomBar/Controls.tsx | 104 ++++++++++++------ .../src/components/BottomBar/buttons.tsx | 44 ++------ web/libs/editor/src/stores/AppStore.js | 6 +- web/libs/editor/src/stores/CustomButton.ts | 11 +- web/libs/editor/src/stores/types.d.ts | 3 +- 5 files changed, 92 insertions(+), 76 deletions(-) diff --git a/web/libs/editor/src/components/BottomBar/Controls.tsx b/web/libs/editor/src/components/BottomBar/Controls.tsx index 4360b775fde1..d6b682b8707e 100644 --- a/web/libs/editor/src/components/BottomBar/Controls.tsx +++ b/web/libs/editor/src/components/BottomBar/Controls.tsx @@ -12,40 +12,44 @@ import { useCallback, useState } from "react"; import { IconBan, LsChevron } from "../../assets/icons"; import { Button } from "../../common/Button/Button"; import { Dropdown } from "../../common/Dropdown/Dropdown"; -import type { CustomButton } from "../../stores/CustomButton"; +import { CustomButton } from "../../stores/CustomButton"; import { Block, cn, Elem } from "../../utils/bem"; import { FF_REVIEWER_FLOW, isFF } from "../../utils/feature-flags"; import { isDefined } from "../../utils/utilities"; -import { AcceptButton, ButtonTooltip, controlsInjector, RejectButton, SkipButton, UnskipButton } from "./buttons"; +import { AcceptButton, ButtonTooltip, controlsInjector, RejectButtonDefinition, SkipButton, UnskipButton } from "./buttons"; import "./Controls.scss"; -type CustomControlProps = { - button: Instance; +type CustomButtonType = Instance; +// @todo should be Instance["customButtons"] but it doesn't fit to itself +type CustomButtonsField = Map>; +type ControlButtonProps = { + button: CustomButtonType; disabled: boolean; - onClick?: (name: string) => void; + onClick: (e: React.MouseEvent) => void; }; +/** If given one element, wrap it in an array */ +function toArray(arg: undefined | T | (T | undefined)[]): T[] { + return (Array.isArray(arg) ? arg : [arg]).filter(v => v !== undefined); +} + /** * Custom action button component, rendering buttons from store.customButtons */ -const CustomControl = observer(({ button, disabled, onClick }: CustomControlProps) => { +const ControlButton = observer(({ button, disabled, onClick }: ControlButtonProps) => { const look = button.disabled || disabled ? "disabled" : button.look; - const [waiting, setWaiting] = useState(false); - const clickHandler = useCallback(async () => { - if (!onClick) return; - setWaiting(true); - await onClick?.(button.name); - setWaiting(false); - }, []); + // @todo do we need waiting? all buttons should utilize isSubmitting + // const [waiting, setWaiting] = useState(false); + return ( @@ -60,14 +64,16 @@ export const Controls = controlsInjector<{ annotation: MSTAnnotation }>( const historySelected = isDefined(store.annotationStore.selectedHistory); const { userGenerate, sentUserGenerate, versions, results, editable: annotationEditable } = annotation; const dropdownTrigger = cn("dropdown").elem("trigger").toClassName(); + const customButtons: CustomButtonsField = store.customButtons; const buttons = []; const [isInProgress, setIsInProgress] = useState(false); const disabled = !annotationEditable || store.isSubmitting || historySelected || isInProgress; const submitDisabled = store.hasInterface("annotations:deny-empty") && results.length === 0; - const buttonHandler = useCallback( - async (e: React.MouseEvent, callback: () => any, tooltipMessage: string) => { + /** Check all things related to comments and then call the action if all is good */ + const handleActionWithComments = useCallback( + async (e: React.MouseEvent, callback: () => any, errorMessage: string) => { const { addedCommentThisSession, currentComment, commentFormSubmit } = store.commentStore; if (isInProgress) return; @@ -84,7 +90,7 @@ export const Controls = controlsInjector<{ annotation: MSTAnnotation }>( await commentFormSubmit(); callback(); } else { - store.commentStore.setTooltipMessage(tooltipMessage); + store.commentStore.setTooltipMessage(errorMessage); } setIsInProgress(false); }, @@ -98,29 +104,65 @@ export const Controls = controlsInjector<{ annotation: MSTAnnotation }>( ], ); - // custom buttons replace all the internal buttons, but they can be reused if `name` is one of the internal buttons - if (store.customButtons?.length) { - for (const customButton of store.customButtons ?? []) { + const buttonsBefore = customButtons.get("_before"); + const buttonsReplacement = customButtons.get("_replace"); + const firstToRender = buttonsReplacement ?? buttonsBefore; + + // either we render _before buttons and then the rest, or we render only _replace buttons + if (firstToRender) { + const allButtons = Array.isArray(firstToRender) ? firstToRender : [firstToRender]; + for (const customButton of allButtons) { // @todo make a list of all internal buttons and use them here to mix custom buttons with internal ones - if (customButton.name === "accept") { - buttons.push(); + // string buttons is a way to render internal buttons + if (typeof customButton === "string") { + if (customButton === "accept") { + // just an example of internal button usage + // @todo move buttons to separate components + buttons.push(); + } } else { buttons.push( - store.handleCustomButton?.(customButton.name)} />, ); } } + } + + if (buttonsReplacement) { + // do nothing as all custom buttons are rendered already and we don't need internal buttons } else if (isReview) { - const onRejectWithComment = (e: React.MouseEvent, action: () => any) => { - buttonHandler(e, action, "Please enter a comment before rejecting"); - }; + const customRejectButtons = toArray(customButtons.get("reject")); + const hasCustomReject = customRejectButtons.length > 0; + const originalRejectButton = RejectButtonDefinition; + const rejectButtons: CustomButtonType[] = hasCustomReject + // @todo implement reuse of internal buttons later (they are set as strings) + ? customRejectButtons.filter(button => typeof button !== "string") + : [originalRejectButton]; + + rejectButtons.forEach(button => { + const action = hasCustomReject + ? () => store.handleCustomButton?.(button.name) + : () => store.rejectAnnotation({}); + + const onReject = async (e: React.MouseEvent) => { + const selected = store.annotationStore?.selected; + + if (store.hasInterface("comments:reject")) { + handleActionWithComments(e, action, "Please enter a comment before rejecting"); + } else { + selected?.submissionInProgress(); + await store.commentStore.commentFormSubmit(); + action(); + } + } - buttons.push(); + buttons.push(); + }); buttons.push(); } else if (annotation.skipped) { buttons.push( @@ -132,7 +174,7 @@ export const Controls = controlsInjector<{ annotation: MSTAnnotation }>( } else { if (store.hasInterface("skip")) { const onSkipWithComment = (e: React.MouseEvent, action: () => any) => { - buttonHandler(e, action, "Please enter a comment before skipping"); + handleActionWithComments(e, action, "Please enter a comment before skipping"); }; buttons.push(); diff --git a/web/libs/editor/src/components/BottomBar/buttons.tsx b/web/libs/editor/src/components/BottomBar/buttons.tsx index f950eb0ce6a2..7e7ceefc6bcc 100644 --- a/web/libs/editor/src/components/BottomBar/buttons.tsx +++ b/web/libs/editor/src/components/BottomBar/buttons.tsx @@ -72,43 +72,17 @@ export const AcceptButton = memo( }), ); -type RejectButtonProps = { - disabled: boolean; - store: MSTStore; - /** - * Handler wrapper for reject with required comment, - * conditions are checked in wrapper and if all good the `action` is called. - **/ - onRejectWithComment: (event: React.MouseEvent, action: () => any) => void; +export const RejectButtonDefinition = { + id: "reject", + name: "reject", + title: "Reject", + look: undefined, + ariaLabel: "reject-annotation", + tooltip: "Reject annotation: [ Ctrl+Space ]", + // @todo we need this for types compatibility, but better to fix CustomButtonType + disabled: false, }; -export const RejectButton = memo( - observer(({ disabled, store, onRejectWithComment }: RejectButtonProps) => { - return ( - - - - ); - }), -); - type SkipButtonProps = { disabled: boolean; store: MSTStore; diff --git a/web/libs/editor/src/stores/AppStore.js b/web/libs/editor/src/stores/AppStore.js index de7eb4ad2485..bb258ca0f303 100644 --- a/web/libs/editor/src/stores/AppStore.js +++ b/web/libs/editor/src/stores/AppStore.js @@ -161,7 +161,11 @@ export default types queuePosition: types.optional(types.number, 0), - customButtons: types.array(CustomButton, []), + customButtons: types.map(types.union( + types.string, + CustomButton, + types.array(types.union(types.string, CustomButton)), + )), }) .preProcessSnapshot((sn) => { // This should only be handled if the sn.user value is an object, and converted to a reference id for other diff --git a/web/libs/editor/src/stores/CustomButton.ts b/web/libs/editor/src/stores/CustomButton.ts index 5bc26fbe4a93..5f288335c558 100644 --- a/web/libs/editor/src/stores/CustomButton.ts +++ b/web/libs/editor/src/stores/CustomButton.ts @@ -1,4 +1,4 @@ -import { applySnapshot, getSnapshot, types } from "mobx-state-tree"; +import { types } from "mobx-state-tree"; import { guidGenerator } from "../utils/unique"; /** @@ -10,16 +10,11 @@ export const CustomButton = types .model("CustomButton", { id: types.optional(types.identifier, guidGenerator), name: types.string, - title: types.maybe(types.string), + title: types.string, look: types.maybe( types.enumeration(["primary", "danger", "destructive", "alt", "outlined", "active", "disabled"] as const), ), tooltip: types.maybe(types.string), ariaLabel: types.maybe(types.string), disabled: types.maybe(types.boolean), - }) - .actions((self) => ({ - updateProps(newProps: Partial) { - applySnapshot(self, Object.assign({}, getSnapshot(self), newProps)); - }, - })); + }); diff --git a/web/libs/editor/src/stores/types.d.ts b/web/libs/editor/src/stores/types.d.ts index 515ef35542e0..d40a7b01948b 100644 --- a/web/libs/editor/src/stores/types.d.ts +++ b/web/libs/editor/src/stores/types.d.ts @@ -29,7 +29,8 @@ type MSTCommentStore = { }; type MSTStore = { - customButtons: Instance[]; + // @todo we can't import CustomButton store here and use it type :( + customButtons: any; settings: Record; isSubmitting: boolean; // @todo WHAT IS THIS? From 8583b62029580bfb816a926799cb3129e68386ed Mon Sep 17 00:00:00 2001 From: hlomzik Date: Wed, 2 Oct 2024 13:51:49 +0100 Subject: [PATCH 02/10] Pass all properties to LSF coming from external sources DM should pass all the options down to LSF, so it will be controllable. --- web/libs/datamanager/src/sdk/lsf-sdk.js | 28 +++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/web/libs/datamanager/src/sdk/lsf-sdk.js b/web/libs/datamanager/src/sdk/lsf-sdk.js index 173592c7c420..0ee88ee6ab73 100644 --- a/web/libs/datamanager/src/sdk/lsf-sdk.js +++ b/web/libs/datamanager/src/sdk/lsf-sdk.js @@ -88,15 +88,29 @@ export class LSFWrapper { * @param {LSFOptions} options */ constructor(dm, element, options) { + // we need to pass the rest of the options to LSF below + const { + task, + preload, + isLabelStream, + annotation, + interfacesModifier, + isInteractivePreannotations, + user, + keymap, + messages, + ...restOptions + } = options; + this.datamanager = dm; this.store = dm.store; this.root = element; - this.task = options.task; - this.preload = options.preload; - this.labelStream = options.isLabelStream ?? false; - this.initialAnnotation = options.annotation; - this.interfacesModifier = options.interfacesModifier; - this.isInteractivePreannotations = options.isInteractivePreannotations ?? false; + this.task = task; + this.preload = preload; + this.labelStream = isLabelStream ?? false; + this.initialAnnotation = annotation; + this.interfacesModifier = interfacesModifier; + this.isInteractivePreannotations = isInteractivePreannotations ?? false; let interfaces = [...DEFAULT_INTERFACES]; @@ -192,6 +206,8 @@ export class LSFWrapper { onSelectAnnotation: this.onSelectAnnotation, onNextTask: this.onNextTask, onPrevTask: this.onPrevTask, + + ...restOptions, }; this.initLabelStudio(lsfProperties); From 239f9c0d08a048e084f28d3316191e687119c32d Mon Sep 17 00:00:00 2001 From: hlomzik Date: Thu, 3 Oct 2024 09:06:10 +0100 Subject: [PATCH 03/10] Remove commented out `waiting`, it's not usable right now Button's handler is executed too fast, so `waiting` doesn't wait for the end of actual request. --- web/libs/editor/src/components/BottomBar/Controls.tsx | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/web/libs/editor/src/components/BottomBar/Controls.tsx b/web/libs/editor/src/components/BottomBar/Controls.tsx index d6b682b8707e..549f5c0d01fa 100644 --- a/web/libs/editor/src/components/BottomBar/Controls.tsx +++ b/web/libs/editor/src/components/BottomBar/Controls.tsx @@ -39,17 +39,14 @@ function toArray(arg: undefined | T | (T | undefined)[]): T[] { */ const ControlButton = observer(({ button, disabled, onClick }: ControlButtonProps) => { const look = button.disabled || disabled ? "disabled" : button.look; - // @todo do we need waiting? all buttons should utilize isSubmitting - // const [waiting, setWaiting] = useState(false); return ( From e73332a4dad78c96cb2662925ba04982428d15b5 Mon Sep 17 00:00:00 2001 From: hlomzik Date: Thu, 3 Oct 2024 14:33:28 +0100 Subject: [PATCH 04/10] Fix crash with new comment format `trim is not a function` because new comments are objects, not strings --- web/libs/editor/src/components/BottomBar/Controls.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/web/libs/editor/src/components/BottomBar/Controls.tsx b/web/libs/editor/src/components/BottomBar/Controls.tsx index 549f5c0d01fa..f7d6244c3be0 100644 --- a/web/libs/editor/src/components/BottomBar/Controls.tsx +++ b/web/libs/editor/src/components/BottomBar/Controls.tsx @@ -72,6 +72,9 @@ export const Controls = controlsInjector<{ annotation: MSTAnnotation }>( const handleActionWithComments = useCallback( async (e: React.MouseEvent, callback: () => any, errorMessage: string) => { const { addedCommentThisSession, currentComment, commentFormSubmit } = store.commentStore; + const comment = currentComment[annotation.id]; + // accept both old and new comment formats + const commentText = (comment?.text ?? comment)?.trim(); if (isInProgress) return; setIsInProgress(true); @@ -81,7 +84,7 @@ export const Controls = controlsInjector<{ annotation: MSTAnnotation }>( if (addedCommentThisSession) { selected?.submissionInProgress(); callback(); - } else if (currentComment[annotation.id]?.trim()) { + } else if (commentText) { e.preventDefault(); selected?.submissionInProgress(); await commentFormSubmit(); From d80ece56b6247340e56571d91b423d8aa1f808b8 Mon Sep 17 00:00:00 2001 From: hlomzik Date: Thu, 3 Oct 2024 14:34:14 +0100 Subject: [PATCH 05/10] Fix init with custom buttons in old format It was just an array and it's a map now. Auto convert array to _replace. --- web/libs/editor/src/stores/AppStore.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/web/libs/editor/src/stores/AppStore.js b/web/libs/editor/src/stores/AppStore.js index bb258ca0f303..1b6c9d42c5aa 100644 --- a/web/libs/editor/src/stores/AppStore.js +++ b/web/libs/editor/src/stores/AppStore.js @@ -182,6 +182,11 @@ export default types : [currentUser]; } } + // fix for old version of custom buttons which were just an array + // @todo remove after a short time + if (Array.isArray(sn.customButtons)) { + sn.customButtons = { _replace: sn.customButtons }; + } return { ...sn, _autoAnnotation: localStorage.getItem("autoAnnotation") === "true", From 728e1156d5cf3fb447be6dae0dc289d4e13233e3 Mon Sep 17 00:00:00 2001 From: hlomzik Date: Thu, 3 Oct 2024 14:46:27 +0100 Subject: [PATCH 06/10] reuse `toArray()` Co-authored-by: Sergey --- web/libs/editor/src/components/BottomBar/Controls.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/libs/editor/src/components/BottomBar/Controls.tsx b/web/libs/editor/src/components/BottomBar/Controls.tsx index f7d6244c3be0..14e521f9ce02 100644 --- a/web/libs/editor/src/components/BottomBar/Controls.tsx +++ b/web/libs/editor/src/components/BottomBar/Controls.tsx @@ -110,7 +110,7 @@ export const Controls = controlsInjector<{ annotation: MSTAnnotation }>( // either we render _before buttons and then the rest, or we render only _replace buttons if (firstToRender) { - const allButtons = Array.isArray(firstToRender) ? firstToRender : [firstToRender]; + const allButtons = toArray(firstToRender); for (const customButton of allButtons) { // @todo make a list of all internal buttons and use them here to mix custom buttons with internal ones // string buttons is a way to render internal buttons From ebf497ca70339d8405bb2efc7b1a496b7a8f65c6 Mon Sep 17 00:00:00 2001 From: hlomzik Date: Mon, 7 Oct 2024 17:23:54 +0100 Subject: [PATCH 07/10] Add stub controlButtons to fix Controls.test.tsx --- .../editor/src/components/BottomBar/__tests__/Controls.test.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/web/libs/editor/src/components/BottomBar/__tests__/Controls.test.tsx b/web/libs/editor/src/components/BottomBar/__tests__/Controls.test.tsx index 9af6e9099d45..e90b13170d69 100644 --- a/web/libs/editor/src/components/BottomBar/__tests__/Controls.test.tsx +++ b/web/libs/editor/src/components/BottomBar/__tests__/Controls.test.tsx @@ -25,6 +25,7 @@ const mockStore = { }, }, }, + customButtons: new Map(), }; const mockHistory = { From c3a7ee652f04049049b439874cec0525362bf92d Mon Sep 17 00:00:00 2001 From: hlomzik Date: Mon, 7 Oct 2024 19:52:49 +0100 Subject: [PATCH 08/10] Fix linting --- .../src/components/BottomBar/Controls.tsx | 28 ++++++++++--------- web/libs/editor/src/stores/AppStore.js | 8 ++---- web/libs/editor/src/stores/CustomButton.ts | 23 ++++++++------- 3 files changed, 29 insertions(+), 30 deletions(-) diff --git a/web/libs/editor/src/components/BottomBar/Controls.tsx b/web/libs/editor/src/components/BottomBar/Controls.tsx index 14e521f9ce02..4069466fed3a 100644 --- a/web/libs/editor/src/components/BottomBar/Controls.tsx +++ b/web/libs/editor/src/components/BottomBar/Controls.tsx @@ -12,11 +12,18 @@ import { useCallback, useState } from "react"; import { IconBan, LsChevron } from "../../assets/icons"; import { Button } from "../../common/Button/Button"; import { Dropdown } from "../../common/Dropdown/Dropdown"; -import { CustomButton } from "../../stores/CustomButton"; +import type { CustomButton } from "../../stores/CustomButton"; import { Block, cn, Elem } from "../../utils/bem"; import { FF_REVIEWER_FLOW, isFF } from "../../utils/feature-flags"; import { isDefined } from "../../utils/utilities"; -import { AcceptButton, ButtonTooltip, controlsInjector, RejectButtonDefinition, SkipButton, UnskipButton } from "./buttons"; +import { + AcceptButton, + ButtonTooltip, + controlsInjector, + RejectButtonDefinition, + SkipButton, + UnskipButton, +} from "./buttons"; import "./Controls.scss"; @@ -31,7 +38,7 @@ type ControlButtonProps = { /** If given one element, wrap it in an array */ function toArray(arg: undefined | T | (T | undefined)[]): T[] { - return (Array.isArray(arg) ? arg : [arg]).filter(v => v !== undefined); + return (Array.isArray(arg) ? arg : [arg]).filter((v) => v !== undefined); } /** @@ -42,12 +49,7 @@ const ControlButton = observer(({ button, disabled, onClick }: ControlButtonProp return ( - @@ -139,12 +141,12 @@ export const Controls = controlsInjector<{ annotation: MSTAnnotation }>( const customRejectButtons = toArray(customButtons.get("reject")); const hasCustomReject = customRejectButtons.length > 0; const originalRejectButton = RejectButtonDefinition; + // @todo implement reuse of internal buttons later (they are set as strings) const rejectButtons: CustomButtonType[] = hasCustomReject - // @todo implement reuse of internal buttons later (they are set as strings) - ? customRejectButtons.filter(button => typeof button !== "string") + ? customRejectButtons.filter((button) => typeof button !== "string") : [originalRejectButton]; - rejectButtons.forEach(button => { + rejectButtons.forEach((button) => { const action = hasCustomReject ? () => store.handleCustomButton?.(button.name) : () => store.rejectAnnotation({}); @@ -159,7 +161,7 @@ export const Controls = controlsInjector<{ annotation: MSTAnnotation }>( await store.commentStore.commentFormSubmit(); action(); } - } + }; buttons.push(); }); diff --git a/web/libs/editor/src/stores/AppStore.js b/web/libs/editor/src/stores/AppStore.js index 1b6c9d42c5aa..6465dcc19fbe 100644 --- a/web/libs/editor/src/stores/AppStore.js +++ b/web/libs/editor/src/stores/AppStore.js @@ -161,11 +161,9 @@ export default types queuePosition: types.optional(types.number, 0), - customButtons: types.map(types.union( - types.string, - CustomButton, - types.array(types.union(types.string, CustomButton)), - )), + customButtons: types.map( + types.union(types.string, CustomButton, types.array(types.union(types.string, CustomButton))), + ), }) .preProcessSnapshot((sn) => { // This should only be handled if the sn.user value is an object, and converted to a reference id for other diff --git a/web/libs/editor/src/stores/CustomButton.ts b/web/libs/editor/src/stores/CustomButton.ts index 5f288335c558..92d4bdc1facc 100644 --- a/web/libs/editor/src/stores/CustomButton.ts +++ b/web/libs/editor/src/stores/CustomButton.ts @@ -6,15 +6,14 @@ import { guidGenerator } from "../utils/unique"; * The only required property is `name`. If the `name` is one of the predefined buttons, it will be rendered as such. * @see CustomControl in BottomBar/Controls */ -export const CustomButton = types - .model("CustomButton", { - id: types.optional(types.identifier, guidGenerator), - name: types.string, - title: types.string, - look: types.maybe( - types.enumeration(["primary", "danger", "destructive", "alt", "outlined", "active", "disabled"] as const), - ), - tooltip: types.maybe(types.string), - ariaLabel: types.maybe(types.string), - disabled: types.maybe(types.boolean), - }); +export const CustomButton = types.model("CustomButton", { + id: types.optional(types.identifier, guidGenerator), + name: types.string, + title: types.string, + look: types.maybe( + types.enumeration(["primary", "danger", "destructive", "alt", "outlined", "active", "disabled"] as const), + ), + tooltip: types.maybe(types.string), + ariaLabel: types.maybe(types.string), + disabled: types.maybe(types.boolean), +}); From 0541168a0d52cdac003dd2f065007b67f6b9c045 Mon Sep 17 00:00:00 2001 From: hlomzik Date: Mon, 7 Oct 2024 20:02:57 +0100 Subject: [PATCH 09/10] Move `toArray()` to utilities We already have `wrapArray()` but it has broken types and a little different semantic --- .../src/components/BottomBar/Controls.tsx | 7 +------ .../src/utils/__tests__/utilities.test.js | 20 ++++++++++++++++++- web/libs/editor/src/utils/utilities.ts | 10 ++++++++++ 3 files changed, 30 insertions(+), 7 deletions(-) diff --git a/web/libs/editor/src/components/BottomBar/Controls.tsx b/web/libs/editor/src/components/BottomBar/Controls.tsx index 4069466fed3a..90020af29ff2 100644 --- a/web/libs/editor/src/components/BottomBar/Controls.tsx +++ b/web/libs/editor/src/components/BottomBar/Controls.tsx @@ -15,7 +15,7 @@ import { Dropdown } from "../../common/Dropdown/Dropdown"; import type { CustomButton } from "../../stores/CustomButton"; import { Block, cn, Elem } from "../../utils/bem"; import { FF_REVIEWER_FLOW, isFF } from "../../utils/feature-flags"; -import { isDefined } from "../../utils/utilities"; +import { isDefined, toArray } from "../../utils/utilities"; import { AcceptButton, ButtonTooltip, @@ -36,11 +36,6 @@ type ControlButtonProps = { onClick: (e: React.MouseEvent) => void; }; -/** If given one element, wrap it in an array */ -function toArray(arg: undefined | T | (T | undefined)[]): T[] { - return (Array.isArray(arg) ? arg : [arg]).filter((v) => v !== undefined); -} - /** * Custom action button component, rendering buttons from store.customButtons */ diff --git a/web/libs/editor/src/utils/__tests__/utilities.test.js b/web/libs/editor/src/utils/__tests__/utilities.test.js index e912af51352c..81518e101ac8 100644 --- a/web/libs/editor/src/utils/__tests__/utilities.test.js +++ b/web/libs/editor/src/utils/__tests__/utilities.test.js @@ -1,5 +1,5 @@ /* global it, describe, expect, test */ -import { emailFromCreatedBy, getUrl, isString, isStringEmpty, isStringJSON, toTimeString } from "../utilities"; +import { emailFromCreatedBy, toArray, getUrl, isString, isStringEmpty, isStringJSON, toTimeString } from "../utilities"; describe("Helper function emailFromCreatedBy", () => { expect(emailFromCreatedBy("abc@def.com, 12")).toBe("abc@def.com"); @@ -14,6 +14,24 @@ describe("Helper function emailFromCreatedBy", () => { expect(emailFromCreatedBy("ab.c+12@def.com.pt")).toBe("ab.c+12@def.com.pt"); }); +describe("Helper function toArray, converting any value to array, skipping undefined values", () => { + test("Empty", () => { + expect(toArray()).toEqual([]); + }); + + test("Single value", () => { + expect(toArray("value")).toEqual(["value"]); + }); + + test("Zero", () => { + expect(toArray(0)).toEqual([0]); + }); + + test("Array", () => { + expect(toArray(["value"])).toEqual(["value"]); + }); +}); + /** * isString */ diff --git a/web/libs/editor/src/utils/utilities.ts b/web/libs/editor/src/utils/utilities.ts index 315379343ab4..6db402d5e417 100644 --- a/web/libs/editor/src/utils/utilities.ts +++ b/web/libs/editor/src/utils/utilities.ts @@ -144,6 +144,16 @@ export function wrapArray(value: any[]) { return ([] as any[]).concat(...[value]); } +/** + * If given one element, wrap it in an array. Removes missing items. Returns empty array for undefined. + * @template T + * @param {T | T[]} arg + * @returns {T[]} + **/ +export function toArray(arg: undefined | T | (T | undefined)[]): T[] { + return (Array.isArray(arg) ? arg : [arg]).filter((v) => v !== undefined); +} + export function delay(ms = 0) { return new Promise((resolve) => setTimeout(resolve, ms)); } From 531120bef867ed7812ccaa51a6919fb5312b1f68 Mon Sep 17 00:00:00 2001 From: hlomzik Date: Mon, 7 Oct 2024 20:06:54 +0100 Subject: [PATCH 10/10] Add comments and types for string keys in custom buttons --- web/libs/editor/src/components/BottomBar/Controls.tsx | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/web/libs/editor/src/components/BottomBar/Controls.tsx b/web/libs/editor/src/components/BottomBar/Controls.tsx index 90020af29ff2..664bf39b54f0 100644 --- a/web/libs/editor/src/components/BottomBar/Controls.tsx +++ b/web/libs/editor/src/components/BottomBar/Controls.tsx @@ -28,8 +28,15 @@ import { import "./Controls.scss"; type CustomButtonType = Instance; +// these buttons can be reused inside custom buttons or can be replaces with custom buttons +type SupportedInternalButtons = "accept" | "reject"; +// special places for custom buttons — before, after or instead of internal buttons +type SpecialPlaces = "_before" | "_after" | "_replace"; // @todo should be Instance["customButtons"] but it doesn't fit to itself -type CustomButtonsField = Map>; +type CustomButtonsField = Map< + SpecialPlaces | SupportedInternalButtons, + CustomButtonType | SupportedInternalButtons | Array +>; type ControlButtonProps = { button: CustomButtonType; disabled: boolean;